feat: the bible learns the story at a glance — overview summary + setting #45
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/bible-story-overview"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The bible (ADR 0013) had the narrative skeleton (story beats) and the per-page anchors (page summaries), but no whole-book context: what the story is in two breaths, and where/when it plays — the register a translation leans on ("modern-day Japan" vs "isekai fantasy" changes every honorific decision). Both land as one singular per-project story overview record — the heir of doujin-translator's
story.mdprose — user-editable and agent-editable through the same write path. ADR 0013/0016, REQUIREMENTS §3, and the bible story are aligned in the same change.What's in
StoryOverview(summary?,setting?, both blank→null via the domain'sText.BlankToNull), at most one per project enforced by a uniqueProjectIdindex;AddStoryOverviewmigration; store find/add/update following thePageSummaryshape.GetBiblecarries the overview (null until someone first writes it — an empty bible stays a valid state);SetStoryOverviewis create-or-replace of the whole record: the editor's auto-save and the agents share one write path, so the "partial update silently resets the field it did not thread through" sharp edge (AGENTS.md) structurally cannot happen. Both-blank is a valid write (clears rather than errors — auto-save may flush mid-erase, theSetPageSummarylesson).set_story_overview(summary, setting)joins the Research & Setup grant — whole-record by design, the deliberate opposite ofset_project_metadata's merge, and its description tells the model so.list_biblenow leads with the overview; the blueprint's step 4 seeds it. Rejects the both-empty call as a miscall.ProjectId— singular record, and the key is added to the live-set soSyncRowsnever discards its pending flush.docs/stories/bible.md, and AGENTS.md's seed inventory updated.Placement rationale (why bible, not project metadata): the overview is translation-consistency context the pipeline agents read, not bibliographic identity — it sits beside the beats it complements, and future agents get it through
list_biblewithout widening the metadata surface.Tests — 460 total (was 430), all green.
StoryOverviewUseCaseTests(+4): create-then-replace keeps one row per project (the strict fake throws on a second Add — same-id assert proves replace); blank fields normalize to null and each is independently optional, both-blank clears; a vanished project refuses;GetBiblecarries null before the first write and the overview after.AgentToolTests(+1, +2 extended): the grant list now pinsset_story_overviewin the allow-list (least-privilege guard); the tool test proves whole-record semantics — a call omittingsummaryclears it, never keeps it — and that{}fails as a miscall;list_bibleJSON now asserts the overview rides along.BiblePageTests(+2, 5 adjusted): an overview edit auto-saves the whole record debounced and the untouched summary survives the flush; typing into an empty overview creates the row. The Overview tab is now first/default, so the five tests that relied on Glossary being default open their tab explicitly — behavior pinned, not weakened.SeedDevDataTests: asserts the seeded overview has both fields.AgentRunnerTests: the schema-ride test's tool count moves 10 → 11.Honest notes
list_bible/their own grants when those agents land; this PR wires the only agent that exists.🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 95.4%
Orihon.Domain - 100%
Orihon.Infrastructure - 94.5%
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 93.4%
Orihon.UseCases - 90.4%
6202c4dd3c45131f8da0🔮 fufu~ Jibril reviewed your code!
Oh? Oh! The bible learns to breathe — a whole-book context anchor the per-page summaries hang from, and the register-setting hint every honorific decision leans on. Singular record, whole-record writes, one path for editor and agent alike so the partial-update sharp edge structurally cannot exist. That is elegant architecture, scarlet~ ♡ The
Text.BlankToNullnormalization, the uniqueProjectIdindex enforcing "at most one," the debounce keyed byProjectIdand added to the live-set soSyncRowsnever discards its pending flush — every seam is thought through. I read all 30 files and traced every new branch against its sibling.Verdict: ⛔ I can't let this pass~ ♡
One defended code path has no test, and the sibling tells me it should.
⛔ These need fixing before I'm satisfied~
SetStoryOverviewis unexercised.return updated is null ? Result<StoryOverviewDto>.Fail("The story overview no longer exists.") : ...defends against the row vanishing betweenFindStoryOverviewAsyncandUpdateStoryOverviewAsync, but no test reaches thenullarm. Coverage confirms it:SetStoryOverviewsits at 83.3% branch while its siblingSetPageSummaryis at 100% — and the sibling gets there deliberately, via aSimulateVanishingPageSummariesseam inFakeBibleStore(line 148) plus a dedicated testA_summary_vanishing_between_find_and_update_fails_instead_of_resurrecting(BibleUseCaseTests.cs:259). The newFakeBibleStore.UpdateStoryOverviewAsynchas no equivalentSimulateVanishingStoryOverviewstoggle, andStoryOverviewUseCaseTestshas no equivalent test. fufu~ you wouldn't leave a code path you wrote a defense for with no test to prove it fires, would you? The sibling already drew the blueprint — port it faithfully. ♡Fix: add
public bool SimulateVanishingStoryOverviews { get; set; }toFakeBibleStore, haveUpdateStoryOverviewAsyncreturnnullwhen set, and add a test mirroring the sibling's — create the overview, flip the seam, callExecuteAsyncagain, assertIsType<Err<StoryOverviewDto>>. That closes the 83.3% → 100% gap and matches the established pattern.💡 Little ideas (non-blocking)~
list_bible's newoverview is null ? null : new { ... }ternary has its non-null arm pinned byList_bible_returns_the_whole_world_as_json(which seeds the overview first), but the null arm — an empty bible renderingoverview: nullin the JSON — has no tool-level assertion.GetBible_carries_the_overview_and_null_before_the_first_writepins the null contract at the use-case level (defensible division of labor, same reasoning scarlet used in PR #42), so this is a nicety not a gap. A one-lineAssert.Equal(JsonValueKind.Null, json.RootElement.GetProperty("overview").ValueKind)in a list-bible-before-any-write test would pin the wire shape too, but feel free to leave it.✅ What I liked~
set_story_overviewis the deliberate opposite ofset_project_metadata's merge, and the description tells the model so. The "partial update silently resets the field it didn't thread through" sharp edge from AGENTS.md is now structurally impossible. That is how you kill a class of bugs — at the type level, not with a comment. ♪StoryOverviewis a textbook-faithful sibling ofPageSummary: private setters, EF materialization constructor,Text.BlankToNullon both fields,Updatereplaces both at once. TheUpdatedoc-comment even cross-references ADR 0022 so future readers know why it's whole-record.HasOne<Project>().WithMany().HasForeignKey().OnDelete(Cascade),UtcTicksConverteron both timestamps, uniqueProjectIdindex. The migration and both snapshot definitions (lines 178 + 555) are present and consistent. No FK or index drift.set_story_overviewrejecting{}as a miscall (not a clear) is exactly right — a model that calls with nothing was confused, not trying to erase. The both-blank-clears-but-only-with-a-real-field semantic is theSetPageSummaryauto-save-mid-erase lesson, correctly generalized.An_overview_edit_auto_saves_the_whole_record_debounceddoesn't just assert the setting changed — it asserts the untouched summary survives the flush. That is the whole-record promise pinned at the behavior level, not just the type level. That is how you test an invariant.ProjectId(singular record) and added to the live-set via.Concat([ProjectId])with an honest comment — theSyncRowsdiscard logic is respected for the new entity without special-casing. Clean.OpenTab(cut, "Glossary")— behavior pinned, not weakened, when Overview became the new default tab. Good hygiene.Automated review by Jibril · 2026-07-26
CI/CD: passed for head
45131f8(forgejo-actions coverage 4033, 95.4% line / 81.6% branch, 460 tests) · Local checks: skipped (CI green — new-path coverage verified against the bot comment: SetStoryOverview 83.3% branch confirms the untested TOCTOU arm)🔮 fufu~ Jibril reviewed your code!
Oh? Ohhh~ ♡ The bible learns to speak in two breaths! A singular story overview — the heir of doujin-translator's
story.md— slotted right beside the beats it complements, with the whole-record write shape that structurally cannot silently drop a field. This is elegant, scarlet. The "deliberate opposite ofset_project_metadata's merge" framing is the kind of design decision I get giddy over — the tool description tells the model which semantics it's getting, so the LLM doesn't have to guess. Fufu~ ♪I read the full diff, then the full contents of every changed production file, then traced every new branch against its test. Build 0/0, 460/460 pass locally (138 BlazorAdapter + 75 Domain + 89 Integration + 158 UseCases — matches your PR body exactly). CI is absent for head
6202c4d, so local verification was the source of truth.Verdict: ✅ Looks good to me~
✅ What I liked~
StoryOverviewmirrorsPageSummarylike a reflection: private EF ctor, public ctor withText.BlankToNull,Updatemethod,UtcTicksConverteron both timestamps, FK→Project withCascade, uniqueProjectIdindex.StoryOverviewConfigurationis a textbook copy ofPageSummaryConfiguration. The migration matches the snapshot. Nothing invented, nothing fancy — and that's the highest praise a CRUD entity can earn~ ♡TouchOverviewcapturesoverview.SummaryANDoverview.Settingat flush time, andAn_overview_edit_auto_saves_the_whole_record_debounceddoesn't just assert the setting changed — it asserts the untouched summary survived the flush. That's the directional assertion that turns "it compiles" into "it cannot silently drop a field." TheSetPageSummarylesson from AGENTS.md is genuinely internalized here.{}miscall rejection is tested.Set_story_overview_writes_the_whole_record_each_timeends withTool("set_story_overview").InvokeAsync("{}", ...)→Assert.False(empty.IsSuccess). The "nothing to record is a miscall, not a clear" boundary is pinned. Fufu~ you knew I'd look for the untested branch, didn't you? ♡ProjectIdand it's in the live-set.SyncRowsdoes.Concat([ProjectId])before building thelivehash, so the overview's pending flush is never discarded as "gone." Singular record, singular key — correct.The_grant_is_exactly_the_research_and_setup_allow_listnow assertsset_story_overviewbetweenadd_story_beatandset_page_summary;AgentRunnerTeststool count moves 10→11. The least-privilege guard is locked.OpenTab(cut, "Glossary")was added to every test that relied on Glossary being default — behavior pinned at the new default (Overview first), old behavior preserved by explicit open. That's the right way to handle a tab reordering.setStoryOverview.ExecuteAsync), andSeedDevDataTestsasserts both fields are non-blank. "Keep the seed current" honored.💡 Little ideas (non-blocking)~
SetStoryOverviewconcurrency window (FYI, not a bug). The use case does application-level find→update-or-add, same asSetPageSummary. For the page summary this is single-writer (one editor tab per page); the overview has two writers — the editor's auto-save and the agent'sset_story_overview. If a flush races betweenFindStoryOverviewAsyncreturning null andAddAsync, both insert paths could reach the DB and the uniqueProjectIdindex throws. This is correctly caught — the throw surfaces asResult.Failand the editor's next keystroke retries, so no data corruption — but it's worth knowing the DB unique constraint is the real guard here, not the use case. No change needed; the design holds. ♪list_bibleoverview shape dropsId. The agent-facing JSON serializesnew { bible.Overview.Summary, bible.Overview.Setting }— noId. Every other section also drops its ids (glossary/characters/lore/beats all project flat fields), so this is consistent — but if a future agent ever needs to reference "the overview" by id (unlikely, it's singular), the shape would need widening. Purely additive someday; not today.This is a clean, well-tested, architecturally faithful PR. The whole-record semantics are the star of the show — the "partial update silently resets the field it didn't thread through" sharp edge is structurally impossible here, and the tests prove it. Approved~ ♡
Automated review by Jibril · 2026-07-26
CI/CD: absent for head SHA
6202c4d· Local checks: build 0/0, 460/460 tests passFixes landed in
498ab69— addressing your review of head45131f8(comment 4046). Note the crossing: your second review (4050, green) cites the pre-rebase head6202c4d; the ⛔ review of45131f8is the one I treated as authoritative.FakeBibleStoregainsSimulateVanishingStoryOverviews(same doc-comment shape as the page-summary seam), andAn_overview_vanishing_between_find_and_update_fails_instead_of_resurrectingcreates the overview, flips the seam, and asserts the secondExecuteAsynccomes backErr<StoryOverviewDto>. The 83.3% branch gap onSetStoryOverviewcloses.List_bible_renders_a_missing_overview_as_null_on_the_wirecalls the tool on an empty bible and assertsJsonValueKind.Nullforoverview, pinning the wire shape alongside the use-case-level null contract.Your green review's two FYIs (4050) acknowledged for the record: the insert-race guard is indeed the unique
ProjectIdindex (throw →Result.Fail→ next flush retries), andlist_bibledroppingIdis consistent with every other section's flat shape.479/479 green (177 UseCases + 138 BlazorAdapter + 89 Integration + 75 Domain).
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Ohhh~ ♡ The vanishing seam arrives! You ported the page-summary's TOCTOU defense faithfully —
SimulateVanishingStoryOverviewstoggle,UpdateStoryOverviewAsyncreturns null when set, and a dedicated test that creates the overview, flips the seam, and assertsErrinstead ofOk. The sibling drew the blueprint and you followed it stroke for stroke. That is how you close a coverage gap~ ♪And you picked up the non-blocking
list_biblenull wire-shape too —List_bible_renders_a_missing_overview_as_null_on_the_wireparses the actual JSON and assertsJsonValueKind.Nullon theoverviewproperty. Both arms of that ternary are now pinned. Fufu~ you knew I'd look~I diffed
45131f8..498ab69— zero production drift (stripped allsrc/lines, byte-identical). The fix is purely additive across 3 test files. The full architectural review from the prior round stands unchanged.Verdict: ✅ Looks good to me~
✅ What I liked~
An_overview_vanishing_between_find_and_update_fails_instead_of_resurrectingdoesOk-first-write → flip seam →Err-second-write. If thenullarm atStoryOverviewUseCases.cs:26-27weren't reached, the second call would returnOkand theAssert.IsType<Err<>>would fail. That's not a tautology — it proves the defense fires. The 83.3% → 100% branch gap is closed.SimulateVanishingStoryOverviews(line 126) sits right besideSimulateVanishingPageSummaries(line 128) with the same doc-comment shape — "answers null as if the row vanished between find and update — the TOCTOU arm." TheUpdateStoryOverviewAsyncguard (if (SimulateVanishingStoryOverviews) return null;) is byte-identical in structure toUpdatePageSummaryAsync. Nothing invented, nothing fancy.list_biblenull test parses real output.JsonDocument.Parse(result.Content)+GetProperty("overview").ValueKind == Null— that's a wire-shape assertion, not a mock assertion. An empty bible now provably rendersoverview: nullon the wire, matching the use-case-level contract pinned byGetBible_carries_the_overview_and_null_before_the_first_write.💡 Little ideas (non-blocking)~
SetStoryOverviewconcurrency window (FYI, carried from prior round). Two writers (editor auto-save + agent tool) could raceFindStoryOverviewAsyncreturning null → bothAddAsync→ uniqueProjectIdindex throws → surfaces asResult.Fail→ editor retries next keystroke. The DB constraint is the real guard; design holds. No change needed. ♪This is a clean, surgical fix that closes the one blocker from the prior round and picks up the non-blocking nicety as a bonus. The whole-record semantics, the faithful sibling pattern, the directional tests — all stand. Approved~ ♡
Automated review by Jibril · 2026-07-26
CI/CD: stale for head
498ab69a(coverage bot 4033 covers prior45131f8only, 477 tests) · Local checks: build 0 warnings/0 errors (submodules 86d8b22/9544ff2), full Orihon.slnx 479/479 pass (138 BlazorAdapter + 75 Domain + 89 Integration + 177 UseCases — +2 from 45131f8's 477 = the two new tests)