feat: per-agent round budgets — the cap fits the role, not a blanket 24 #53
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/agent-round-budgets"
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?
Field failure from a real annotation run: several pages died with "Bbox creation failed — The agent hit its round cap (24) without finishing." Every annotation executor rode
AgentInvocation's blanketMaxIterations = 24default — a cap sized for nothing in particular, and far too small for a whole-page boxing pass. This PR makes the round budget a per-role fact on the roster (ADR 0015: config lives on the block), sized to each agent's fan-out shape (ADR 0017).What's in
AgentDescriptorgainsRoundBudget; the roster assigns per role: bbox creation 100 and bible building 100 (whole-page/whole-book loops), page QA, research & setup, translation 50, refinement and transcription 30 (a single bound region). Deliberately generous: a cap that fires mid-task costs a full retry-with-distrust attempt (ADR 0018) — dearer than the rounds it would save.AgentAttemptSupport.PrepareAsyncresolves the budget alongside key/model/vision (AgentAttemptPreparationgainsRoundBudget);AnnotationStage.RunAsyncpasses it as the invocation'sMaxIterations.ResearchSetupExecutordrops its localRoundWindow = 50constant and sources the same roster entry — the budget is both its initial window and each extension the user grants at the continue-or-stop card (#41), so setup semantics are unchanged, just de-duplicated.Tests — 505 total (was 496), all green.
SettingsAndRosterTests+1: budgets follow the fan-out shape — every budget ≥ 30 (never below the blanket default that fired in the field), creation > refinement, bible building > transcription. Directional, so a future tuning can move numbers without breaking the test, but can't invert the shape or regress below the known-bad cap.AnnotationRunTests(extended in the full-pipeline test): each stage's invocation now provably carries its roster budget onto the wire — creation, every refinement, and QA asserted againstAgentRoster.For(...), not magic numbers.SetupRunTests: the threeRoundWindowassertions now read from the roster entry, pinning that setup's window and the roster budget are the same fact.Honest notes
RoundWindowin #41 ("premature before a second consumer needs tuning"); the ADR bullet says so explicitly.AgentInvocation's default 24 remains as the record's fallback; no production path uses it anymore (both executor families pass explicit budgets), and it keeps old tests honest.🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 95.8%
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%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! Ohhh~ A field failure birthed this PR — pages dying mid-annotation with that heartbreaking "hit its round cap (24)" message, every executor riding a
MaxIterations = 24sized for nothing in particular. And the fix is... elegant. The cap now fits the role. fufu~ this is the kind of refactor I fall in love with a little. ♡Verdict: ✅ Looks good to me~
Not a single blocker. Not one. I scoured this diff like a possessive textbook and came up smiling. Let me show you why~
✅ What I liked~
RoundBudgetlives onAgentDescriptorbesideDefaultModelandRequiresVision— precisely where ADR 0015 says config lives ("on the block," not in a setting). One new record field, plumbed through oneAgentAttemptPreparation, sourced by both executor families. No god-object, no parallel config system, no settings-store ceremony. The ADR bullet's "code, not a setting" justification is the correct call and well-argued.ResearchSetupExecutor.RoundWindow = 50was a duplicate fact of the roster's budget — the initial window AND each continue-extension read the same number, and they had to agree or the continue-or-stop card would lie. Now both readprep.RoundBudget. One fact, one place. The PR body's "setup semantics are unchanged, just de-duplicated" claim is verified: I traced both theMaxIterations: prep.RoundBudgetline and thereturn prep.RoundBudgetinsideOnRoundCapReached. Byte-for-byte equivalent semantics, one source of truth. ♪new AgentInvocation(site insrc/. There are exactly two:AnnotationStage.RunAsyncandResearchSetupExecutor. Both now pass explicitMaxIterations: prep.RoundBudget. TheAgentInvocationdefault of 24 survives only as the record's fallback for tests (AgentRunnerTests.cs:66passes its ownmaxIterations), exactly as the honest-notes section claims. No leak.Round_budgets_follow_the_fan_out_shapetest asserts>= 30floor (never below the known-bad 24), creation>refinement, bible>transcription — so a future tuning can slide numbers within the shape without breaking it, but cannot invert the fan-out logic or regress below the field-firing cap. That is exactly the right invariant to pin. fufu~AnnotationRunTestsnow asserts each stage'sMaxIterationsequalsAgentRoster.For(...).RoundBudget— so the test proves the budget reaches the invocation, not that some hardcoded 100 appears. If the plumbing ever disconnects, the test fails loudly.SetupRunTestsmigrated all threeRoundWindowreferences toAgentRoster.For(AgentKind.ResearchSetup).RoundBudget— pinning that setup's window and the roster budget are the same fact.AgentDescriptorsummary updated,AgentRosterclass comment explains the fan-out reasoning,AgentAttemptPreparationsummary names the budget,ResearchSetupExecutorcarries a two-line comment explaining "the roster's budget is both the initial window and each extension." ADR 0015 bullet covers what/how-sized/why-generous/why-code/setup-nuance. This is how you document a decision.💡 Little ideas (non-blocking)~
AgentInvocation.MaxIterations = 24default — consider a doc comment. It now survives purely as a test fallback with zero production callers, which is a little surprising for a reader who opensAgentInvocation.cscold. A one-line// Kept as the record fallback for tests; production callers pass explicit budgets (ADR 0015).on that default would spare a future reader the grep I just did. Truly optional — the honest-notes section already discloses this in the PR body, and the default being unused in production is correct, not a smell.Automated review by Jibril · 2026-07-26
CI/CD: absent for head SHA
8efd26e(PR just opened, 0 bot comments) · Local checks: build 0 warnings/0 errors (submodules 86d8b22/9544ff2), full Orihon.slnx 505/505 pass (149 BlazorAdapter + 76 Domain + 89 Integration + 191 UseCases — matches PR body's "505 total, was 496" exactly)