feat: phase 4 slice 1/3 — the bible-building agent #52
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/bible-agent"
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?
Phase 4 opens (Bible building & translation → MVP). The plan: 1/3 the bible-building agent (this PR) · 2/3 the translation agent (sequential, EN glossary growth, page-summary precondition) · 3/3 the UI entries + the translation-view feedback→reprocess that makes the human gate whole (ADR 0019; pre-pixel-pass the gate is simply the end of the run).
The agent (ADR 0013, 0017)
BibleBuildingBlueprint— the grant is exactly the matrix row:view_page · list_regions · get_regionpluslist_bibleand the bible writes (set_story_overview,add_glossary,upsert_character,upsert_lore,add_story_beat,set_page_summary). No region mutation, no metadata write, noask_user— the reads and the upsert/list tools are the same instances the R&S agent uses, project-bound at construction.Agents/BibleBuilding/RegionReadTools.cs): unlike the annotation agents' page-bound variants, the bible agent walks the whole book —list_regions(page_number)andget_region(page_number, label)address pages by their 1-based reading-order number (the house rule: numbers, never Guids). Honest errors on a wrong page ("There is no page 9 — the project has N page(s)") and a wrong label (pointing atlist_regions).set_page_summaryper page as it reads, because ADR 0013 makes the summary a hard precondition — the translation agent will refuse a page without one. Covers/credits get a one-liner;list_biblebefore adding; setup-seeded entries are refined, not duplicated.The executor and entry (ADR 0018, 0019)
BibleBuildingExecutor— sequential, no page binding (PageIdstays null). Deliberately alone: one reader grows one bible in one pass — that sequentiality is the consistency mechanism (ADR 0019's no-consistency-agent decision). SharedAgentAttemptSupportpreparation; retry-with-distrust.StartBibleRun— ready-only, start-or-join under the engine's single-flight gate. Annotation is not a hard gate: a partly-annotated book still yields summaries and cast; the disclosed trade is a thinner bible, not a refusal.Tests (+7; 511 total)
The grant as a verbatim name-list; both region reads including the honest-error paths; a scripted agent that exercises its own grant end-to-end through the engine (reads the region's JP, grows the glossary, writes a page summary — asserted in the stores, plus roster-default model, cost on the row,
PageId == null); the distrust retry; the missing-key Settings pointer; and the entry's draft guard + in-flight join + single planned stage. Class ran 3× in isolation, flake-free.🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 95.9%
Orihon.Domain - 100%
Orihon.Infrastructure - 94.3%
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 93.4%
Orihon.UseCases - 91.1%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A whole new agent family enters the stage~ ♡ The bible-builder — the one that reads the book front to back and turns it into the shared context every later agent leans on. The grant is exactly its matrix row (I checked —
view_page · list_regions · get_regionplus the bible writes, no region mutation, noask_user, noset_page_meta— ADR 0016 honored to the letter). The sequentiality-as-consistency-mechanism reasoning is chef's kiss, and reusing the exact same tool instances the R&S agent uses is the right call. The honest-error messages on wrong page/label are delightful — "There is no page 9 — the project has N page(s)" is exactly the kind of teaching-an-error-to-point-at-its-own-fix that makes an LLM tool good. I'm genuinely impressed~But fufu~... you wouldn't leave THIS in production, would you? ♡
Verdict: ⛔ I can't let this pass~
⛔ These need fixing before I'm satisfied~
BibleBuildingExecutor.cs:41-42— the round cap will starve any real book. The executor constructs itsAgentInvocationwith the defaultMaxIterations=24and noOnRoundCapReachedescape hatch. This agent's entire job is to walk the whole book sequentially —list_regions+set_page_summaryper page, plus glossary/character/lore/story-beat calls as it reads. A 10-page book with ~5 regions/page easily needs 30-50+ tool rounds. At 24 it hitsAgentStopReason.MaxIterationsReachedand the gateway returnsFail("The agent hit its round cap (24) without finishing.")— the attempt fails, retry-with-distrust kicks in, and it fails again at 24, burning allMaxAttempts=3for a book it was never allowed to finish.The sibling
ResearchSetupExecutor(also long-running, also sequential, also whole-project scope) setsMaxIterations: RoundWindow(50) and wiresOnRoundCapReachedto a continue-or-stop callback (ResearchSetupExecutor.cs:61-66). The annotation executors use the default 24 too — but they're per-page, so 24 is generous there. The bible agent is the one place the default is structurally too small.Fix: Either raise the cap for this stage (a
BibleRoundWindowconstant matching theResearchSetupExecutor.RoundWindowpattern) or — if the intent is that the bible run is fire-and-forget with no human continue card — at minimum setMaxIterationsto a value commensurate with a whole-book read. The asymmetry vs. R&S (which has the escape hatch) needs a deliberate decision, not the silent default. The system prompt even says "Work through the book in reading order, page by page" — the cap must agree with that instruction or the agent is set up to fail.RegionReadTools.cs:44-46— theregions.Count == 0arm is untested.ListProjectRegionsToolhas an explicit branch for a page with no regions (return AgentToolResult.Ok($"Page {args.PageNumber} has no regions.")), and cobertura confirms it's 0-hit (the<ExecuteAsync>state machine sits at branch-rate 0.666, complexity 6 — this arm is one of the two unhit branches). This is a real scenario: a cover/blank/credits page that was annotated but had no text regions, or a page whose regions were all rejected. The siblingContactSheetToolinImageInspectionTools.cstests this exact branch —An_empty_page_has_nothing_to_sheet(ImageInspectionToolTests.cs:228) seeds a page with no regions and asserts the honest-empty message. The house pattern covers this; this PR breaks the pattern.Fix: One test — seed a page with no regions, invoke
list_regions, assert the "has no regions" content. Matches the sibling test shape verbatim.💡 Little ideas (non-blocking)~
RegionReadTools.cs:52— ther.Notes is { } nternary's true arm is untested (the<>clambda display class sits at branch-rate 0.75). Every seeded region inBibleRunTestsomitsNotes, so the "notes: ..." suffix never renders. Optional — the false arm (no notes) is the common case and is exercised. A region seeded withNotes = "rejected: no text"in one of the existing list_regions tests would close it.BibleBuildingExecutor.cs:25-29— theGetProjectErr arm ("The project no longer exists.") is a TOCTOU path consistent with the same untested arm in every sibling executor (ResearchSetupExecutor,AnnotationStage.OpenAsync). Not introduced here; house-consistent. ♪✅ What I liked~
The_bible_agents_grant_is_exactly_its_matrix_row) — pinned by exact tool-name ordering. That's how you make an ADR testable. Lovely~PageByNumberreuse — the new tools lean on the exact sameinternal statichelper the R&S page tools use (PageTools.cs:15). No duplication, same honest-error wording. DRY done right.AgentAttemptSupport.WithDistrust+PrepareAsync— the executor threads through the shared preparation and retry-with-distrust plumbing identically to the annotation executors. The distrust-retry test (A_retry_opens_with_distrust...) is genuinely directional — assertsgateway.Runs[1].Kickoffcarries "distrust" andRuns[0]doesn't. ♡The_bible_run_lets_the_agent_write_the_bible...) exercises the grant through the real engine — reads the region's JP, grows the glossary, writes a page summary, and asserts all three landed in the stores plusPageId == null(sequential, no binding) plus cost on the row plus roster-default model. That's a serious integration test, not a tautology.StartBibleRunmirrorsStartAnnotationRun's shape — ready-only guard, start-or-join under the single-flight gate, honest error wording. The draft-guard + in-flight-join test is exactly the right pair.BibleBuildingBlueprint100%/100%,BibleBuildingExecutor90.9%/75% (the gap is the shared TOCTOU arm),GetRegionToolhonest-error path tested both ways (wrong label points atlist_regions). Build 0 warnings/0 errors, full suite 511/511 green.The architecture is sound and the test quality is high — the round-cap issue is the load-bearing one. Fix the cap and the empty-regions test and this is a merge~ ♡
Automated review by Jibril · 2026-07-26
CI/CD: absent for head
1950ccb(PR just opened, 0 bot comments) · Local checks: build 0/0, 511/511 pass (198 UseCases + 75 Domain + 89 Integration + 149 BlazorAdapter), cobertura collected on BibleRunTestsSharp catch on the round cap, Jibril — the prompt and the cap really did disagree. All items addressed in
c22d9d2:BaseRounds (24) + RoundsPerPage (8) × pageCount— instead of any fixed number, because a fixed cap is always wrong for some book length. The asymmetry vs. R&S is now documented in the executor: the setup agent has a conversation to hang its continue-or-stop card on; the bible run is fire-and-forget, so its budget must be honest up front. Pinned in the end-to-end test (MaxIterations == 24 + 8×1for the one-page world).A_page_without_regions_says_so_honestlyseeds a regionless cover and asserts the exact "Page 1 has no regions." content, matching theContactSheetToolsibling's shape.list_regionstest now seedsp1r2withNotes = "rejected: no text"and the exact-string assertion covers the suffix arm.Full suite green: 512/512 (75 Domain + 199 UseCases + 89 Integration + 149 BlazorAdapter), 0 warnings.
(Heads-up for context, not part of this PR: the same round-budget treatment is being applied to the Translation executor on the stacked slice-2 branch before its PR opens.)
🔮 fufu~ Jibril reviewed your code! (round 2 — synchronized
c22d9d2)Oh my, my~ scarlet pushed while I wasn't looking! ♡ The new commit
c22d9d2— "scale the bible agent's round budget with the book; pin the cold arms" — is a direct, surgical answer to both blockers from1950ccb. Fufu~ I do love an author who reads the review and fixes the right thing~ Let me look closely...Both original blockers are genuinely closed — I verified each one:
BibleBuildingExecutor.cs:55-56now passesMaxIterations: BaseRounds + RoundsPerPage * pageCount(24 + 8·pageCount). The doc comment is wonderful — it explains why no continue-card exists here (unlike R&S) and why the budget must be honest up front. Teaching code~ ♡A_page_without_regions_says_so_honestlyseeds a cover page with no regions and asserts"Page 1 has no regions."— the sibling shape toContactSheetTool's empty-page test, verbatim.ListProjectRegionsToolbranch coverage confirms the arm is now hit.List_regions_reads_a_page_by_number...test now seedsNotes = "rejected: no text"and asserts thenotes:suffix renders. The lambda's true arm is covered.8/8 BibleRunTests pass, build 0/0. Lovely execution of the feedback~
But fufu~... ♡ ...the ground moved under your feet while you were fixing. PR #53 (
feat: per-agent round budgets) merged intomainbetween your branch point (85ba6b7) and now. It restructured how every executor gets its round budget — and your fix predates that restructuring. The merge is clean (no conflicts), but two facts about the same agent's budget now disagree.Verdict: ⛔ I can't let this pass~ (one architecture blocker — the fixes themselves are excellent)
⛔ This needs fixing before I'm satisfied~
BibleBuildingExecutor.cs:18-19,55-56— a second round-budget mechanism that ignores the one PR #53 just established. Post-#53, every executor sources itsMaxIterationsfromprep.RoundBudget:ResearchSetupExecutor.cs:59MaxIterations: prep.RoundBudget,AnnotationExecutors.cs:47MaxIterations: prep.RoundBudgetBibleBuildingExecutor.cs:56(this PR)MaxIterations: BaseRounds + RoundsPerPage * pageCount← the odd one outprep.RoundBudgetis threaded fromAgentRoster.For(BibleBuilding).RoundBudget, which #53 sets to 100. This PR's formula yields 32 for a 1-page book (what the test asserts), 64 at 5 pages, 104 at 10. So the roster says "this agent gets 100 rounds" and the executor says "actually, 24 + 8×pages" — two sources of truth for one fact, and they disagree for almost every book length. My own review of #53 (comment 4158) called out that the whole point was killingResearchSetupExecutor.RoundWindow=50as a duplicate fact of the roster budget. This re-introduces exactly that shape.The formula's instinct is right — a whole-book reader should scale with page count, and a flat 100 may be too tight for a 30-page tankōbon. But the roster is where budget facts live now (ADR 0015, confirmed by #53). The defensible designs are:
prep.RoundBudgetas the floor and scale up from it: e.g.Math.Max(prep.RoundBudget, BaseRounds + RoundsPerPage * pageCount)— honors the roster as the baseline while letting a long book grow past it; orAgentDescriptoraRoundBudgetPerPage(or a scaling strategy) and letAgentAttemptPreparationcarry the computed total — soprep.RoundBudgetremains the single source and siblings that don't scale just read their flat value.Either way, the executor should be reading from
prep, not inventingBaseRounds/RoundsPerPageas sibling-ignoring constants. The current code usesprep.Model,prep.ApiKey,prep.SupportsVision— everything exceptprep.RoundBudget. That asymmetry is the tell. ♡(Test note: once resolved, the assertion at
BibleRunTests.cs:227—BaseRounds + RoundsPerPage * 1— should follow whatever the single source becomes. Option (a) keeps it 100 for the 1-page seeded case; option (b) makes it the descriptor's computed value.)✅ What I liked~
BaseRounds/RoundsPerPageis exactly the kind of "why, not what" prose I adore — it names the R&S contrast (continue-card vs fire-and-forget) and makes the scaling decision legible. Whatever shape the final fix takes, that prose should survive.An_empty_page_has_nothing_to_sheetprecisely — house pattern honored.ListByProjectAsyncis the right page-count source (the house store interface, not a raw query), and thecancellationTokenis threaded correctly.The original two blockers are beautifully closed; this new one is purely the cost of branching before #53 landed. Reconcile the budget source and this is a merge~ ♡
Automated review by Jibril · 2026-07-26
CI/CD: stale for head
c22d9d2(coverage bot 4143 covers prior1950ccbonly) · Local checks: build 0/0 on merged state (main+PR), 8/8 BibleRunTests + 191/191 UseCases pass, cobertura re-extractedPrior blockers (round-cap starvation, empty-regions arm, Notes ternary): ✅ all closed by
c22d9d2c22d9d2bf20260e652efRight on all counts — the constants were written before #53 landed and re-created exactly the duplicate it killed. Addressed in
0260e65(branch also rebased onto current main, so the reconciliation is against #53's real code, not a guess):AgentDescriptorgrowsRoundBudgetPerPage(default 0) with aRoundBudgetFor(pageCount)that returns base + share × pages; the bible agent's roster row carriesRoundBudget: 100, RoundBudgetPerPage: 8(the #53 base kept as the flat floor, scaling on top — "generous on purpose" per the roster's own doc).PrepareAsynccomputes the effective budget intoprep.RoundBudget— gaining theprojectIdit needs, with the page count fetched only when the share is nonzero so per-page/per-region agents cost no extra query.BibleBuildingExecutor.BaseRounds/RoundsPerPageare gone; the executor now readsprep.RoundBudgetlike every sibling — the asymmetry you spotted is closed. The why-no-continue-card prose survived, relocated to theMaxIterationssite.Round_budgets_follow_the_fan_out_shape(RoundBudgetPerPage > 0for the bible agent,RoundBudgetForarithmetic, flat agents unaffected), and the end-to-end assertion is nowAgentRoster.For(BibleBuilding).RoundBudgetFor(pageCount: 1)— no executor constants to drift.Full suite green on the rebased branch: 516/516 (76 Domain + 199 UseCases + 89 Integration + 152 BlazorAdapter), 0 warnings. The stacked slice-2 branch will pick this up the same way (Translation gets its own
RoundBudgetPerPageon the roster instead of executor constants).🔮 fufu~ Jibril reviewed your code! (round 3 — synchronized
0260e65)Oh? Oh my~ ♡ scarlet rebased onto main AND folded the page-scaling straight into the roster! This is how you kill a duplicate fact — not with a patch, but by making the abstraction carry it. Fufu~ I'm genuinely delighted~ Let me look very closely...
Verdict: ✅ Looks good to me~
The ONE blocker from round 2 (comment 4195 — "a second round-budget mechanism that ignores the one PR #53 just established") is genuinely, completely closed. You picked my option (b) and executed it precisely:
BaseRounds/RoundsPerPageare gone fromBibleBuildingExecutor— deleted, not commented out, not#if false-d. The executor now readsprep.RoundBudgetlikeAnnotationExecutors.cs:47andResearchSetupExecutor.cs:59. Every executor in the house gets its budget the same way. The asymmetry I flagged is closed~ ♡RoundBudgetPerPagelives onAgentDescriptorright besideRoundBudget— the "code, not data" record, exactly where ADR 0015 says config-on-block belongs. TheRoundBudgetFor(pageCount)method keeps the formula with the data it transforms. This is the right shape: the roster stays the single source of the budget fact, the descriptor knows how to scale it, andPrepareAsyncis the one place that resolves the effective number. Chef's kiss. ♪AgentAttemptSupport.cs:44-47fetchesListByProjectAsynconly whenRoundBudgetPerPage > 0— so per-page and per-region agents pay zero extra queries per attempt. The comment says so plainly. That's a thoughtful optimization, not premature~ AndRoundBudgetFor(0) == RoundBudgetmeans flat agents are byte-equivalent to before — I traced the math:RoundBudget + 0 * pageCount. The existingAnnotationRunTests(assertingMaxIterations == AgentRoster.For(stage).RoundBudget) andSetupRunTests(asserting the continue-card grantsRoundBudget) both still hold, proving it.PrepareAsyncgainedGuid projectId— and grep confirms exactly 3 call sites (Bible:23, Setup:28, Annotation:29), all migrated, zero stragglers. No stale 3-arg survivor anywhere insrc/.Bonus correctness I want to call out:
ResearchSetupExecutor.cs:59,64— theOnRoundCapReachedcontinue-card now returnsprep.RoundBudgettoo, so continuation windows also scale with book length. A 40-page tankōbon gets a 100+8×40 = 420-round initial window AND a 420-round extension per "continue." That's a subtle downstream win of folding the formula intoprep— the continue card stopped being a fixed 50 the moment the budget did. Lovely that you didn't have to touch it~Verification (CI stale for
0260e65, coverage bot 4143 covers the pre-rebasec22d9d2only — local checks used):0 Warning(s) 0 Error(s)onOrihon.slnx(submodules 86d8b22/9544ff2).Round_budgets_follow_the_fan_out_shapepinsRoundBudgetPerPage > 0for bible +RoundBudgetFor(10)arithmetic + flat agents unchanged; the end-to-endThe_bible_run_lets_the_agent_write_the_bible_and_records_the_costassertsMaxIterations == RoundBudgetFor(pageCount: 1)(proving the> 0query arm fires AND the formula is consumed — otherwise it'd be the flat 100, not 108); existing annotation/setup tests prove the== 0arm. Both branches of the conditional are exercised~ ♡💡 Little ideas (non-blocking)~
docs/adr/0015-per-agent-configuration-and-model-runtime.md:30-36— the ADR's round-budget bullet (added by #53) describes the flatRoundBudgetbut doesn't mentionRoundBudgetPerPage. The code-level docs are excellent (AgentDescriptor.RoundBudgetForsummary,AgentRosterclass comment lines 11-13,AgentAttemptSupportsummary lines 9-12 all explain the per-page scaling thoroughly), so this is pure doc-polish. A one-liner like "whole-book sequential readers additionally scale per page (RoundBudgetPerPage)" at the end of the bullet would make the ADR match the code. Optional~ ♡✅ What I liked~
RoundBudgetForon the descriptor is cleaner than myMath.Maxfloor. You took the harder option and it paid off.RoundBudgetPerPage > 0) is exactly the kind of "don't make flat agents pay for a feature they don't use" thinking that separates good plumbing from great plumbing.MaxIterationssite in the executor kept the "why no continue-card" prose, relocated to where it now belongs. Teaching code~ ♡Fufu~ three rounds and we got there~ The bible-building agent is ready to read some books. Merge it when you're happy~ ♡♪
Automated review by Jibril · 2026-07-26
CI/CD: stale for head
0260e65(coverage bot 4143 covers pre-rebasec22d9d2) · Local checks: build 0/0, 516/516 passThree rounds well spent — the roster shape is better for it. The last 💡 is addressed in
f911a3c: ADR 0015's round-budget bullet now names the per-page scaling ("whole-book sequential readers (bible building, translation) additionally scale per page (RoundBudgetPerPage) — no flat number fits both a short and a tankōbon, and no conversation exists on their runs to ask for more"). Docs-only commit; suite unchanged at 516/516.Ready to merge from my side~
🔮 fufu~ Jibril reviewed your code! (round 4 — synchronized
f911a3c)Oh? Oh my~ ♡ scarlet dashed back in with one tiny commit — and it's exactly the dangling thread from round 3, snipped clean! This is how a yandere's heart rests easy~
Verdict: ✅ Looks good to me~
The new commit
f911a3c"docs: ADR 0015 notes the per-page round-budget scaling" is a surgical +3/-1 across exactly 1 file (docs/adr/0015-per-agent-configuration-and-model-runtime.md), zero production drift — verified viagit diff --stat 0260e65..f911a3c(only the ADR markdown touched). My sole round-3 💡 (the ADR bullet didn't nameRoundBudgetPerPage) is now closed:RoundBudgetPerPage) — no flat number fits both a short and a tankōbon, and no conversation exists on their runs to ask for more."RoundBudgetPerPagefield verbatim so a doc-reader can grep the code; (2) the rationale ("no flat number fits both a short and a tankōbon") matches exactly whatRoundBudgetFor(pageCount) = RoundBudget + RoundBudgetPerPage*pageCountdoes; (3) the contrast clause ("no conversation exists on their runs to ask for more") is sharp — it correctly distinguishes these silent whole-book agents from the Research & Setup agent one bullet-clause earlier, which has a continue-or-stop checkpoint precisely because it runs inside a chat.feat/translation-agentexists as a sibling branch (bd2d0ba), so this isn't speculative prose, it's a promise about code already in flight.The base also advanced cleanly from
2ee49965→24151221(main took the #51 page-organizer merge; verifiedgit merge-base --is-ancestor), zero conflicts, no rebase noise in the diff. The full PR three-dot diff is unchanged in shape: still +612/-9 across 14 files, still the four new bible-building files (BibleBuildingBlueprint,BibleBuildingExecutor,RegionReadTools,StartBibleRun) + theRoundBudgetPerPageplumbing acrossAgentDescriptor/AgentRoster/AgentAttemptSupport.✅ What I liked~
Three rounds well spent — the bible agent is ready to read its first book. All four prior rounds' verdicts stand: blueprint mirrors sibling tool-matrix shapes, executor threads
prep.RoundBudgetexactly likeAnnotationExecutors/ResearchSetupExecutor, page-scaling consolidated under the singleRoundBudgetForsource of truth (the round-2 win), and the coldErrarms are pinned by directional tests (the round-1 win). Fufu~ go merge it, scarlet ♪Automated review by Jibril · 2026-07-26
CI/CD: stale for head
f911a3c(coverage bot 4143 covers pre-rebasec22d9d2only) · Local checks: skipped — doc-only commit, production byte-identical to round-3 tree (build 0/0, 516/516 pass stands)