build: take OpenRouter.Net's tool-answer ordering fix #81
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "worktree-investigate-tool-image-order"
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?
Submodule pointer only —
9544ff2→354730c, picking up TeamAI/OpenRouter.Net#6 (merged). No Orihon code changes: nothing on our side was wrong.What it fixes
A bbox-refinement agent died mid-page on p7r9 with a provider 400 naming a
tool_call_idit had definitely answered:The agent loop appended a tool's injected image the moment that tool returned, inside the per-call loop. With one call per turn that is fine; with two it puts a user message between the first answer and the second:
The images are now held until every id is answered, then appended in call order. Upstream carries the repro (both tests fail on its old
mainwithActual: ["crop:1"]) and the describer-path twin.Why it reached us now
Three things had to line up, which is worth recording because the first two are ours:
crop. One obvious tool makes batching two calls in a turn the natural move — with five differently-named views the model spread them across rounds.Blast radius was wider than the stage that reported it: every vision agent that batches was exposed — transcription, Page QA, Sfx QA, sfx boxing. If earlier runs showed unexplained provider 400s on those stages, this is the likely single cause rather than several separate ones.
Verification
738/738 green against the new pointer (Domain 82, UseCases 310, Integration 156, BlazorAdapter 190), build clean. Nothing in Orihon's own tests exercises the batched path — the fake gateway never reaches the real loop — so the coverage that matters lives upstream, where it is pinned.
Not browser-verified: no user-facing change.
After this merges, p7's refinement needs a re-run to clear — the failure is deterministic for a given model, so the row will not recover on its own.
🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 95.9%
Orihon.Domain - 100%
Orihon.Infrastructure - 95.9%
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 93.3%
Orihon.UseCases - 97.2%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh~ Oh! A one-line diff. A single submodule pointer,
9544ff2 → 354730c. Fufu~ after a week of the machines demanding my attention, this is the kind of review I dream of — clean, surgical, and carrying a story I already know by heart~ ♡Verdict: ✅ Looks good to me~
✅ What I verified~
I won't pretend I trust the PR body without checking it — you know me better than that, fufu~ So I traced every link in the chain:
git diff base...headtouches one file, one line:external/OpenRouter.Net,9544ff2 → 354730c. No Orihon source touched. Nothing smuggled in. Clean~354730cis the merge commit of OpenRouter.Net #6 ("fix(agents): a turn's tool answers stay one unbroken block"), merged into OpenRouter's main at 2026-07-27 19:28:52 by Björn. I confirmeda756ad0(the PR #6 head I reviewed upstream) is its direct sole ancestor —git logbetween them shows only the merge commit itself. So354730cis precisely "the fix I already approved, now landed on upstream main." Nothing more, nothing different.ExecuteToolCallsAsyncappended each tool's injected image inside the per-call loop, so two calls in one turn planted auser(image)betweentool(crop:1)andtool(crop:2)— breaking the provider's contiguous-tool-answers contract. Strict providers (Moonshot AI) 400'd; lax ones silently accepted the malformed history. The fix holds images in aList<UserMessage>andAddRanges after the loop. I verified both regression tests fail on upstream's oldmainwithActual: ["crop:1"]exactly, and pass on the branch — and thatExecuteToolCallsAsynchit 100%/100% line/branch upstream. The coverage that matters lives there, and it's pinned.🔍 On coverage & the "fake gateway" caveat~
The PR body is honest about this and I appreciate it — Orihon's own suite never reaches the batched tool-call path because the fake gateway never exercises the real loop. That is the correct state of affairs: the contract being fixed lives upstream in OpenRouter.Net, and that is where the coverage lives (and where I verified it). The CI bot at
c89dec1shows 738/738 green at 96.3% line / 83.2% branch across Orihon — the relevant consumers (BboxRefinementExecutor93.7%/81.2%,BoundCropTool/BoundContactSheetTool100%) are unchanged by a pointer bump, as expected. Re-running them is sanity, not evidence — and the evidence is already green.💡 Little ideas (non-blocking)~
✅ What I liked~
Merge it, re-run p7, and call this one closed~ ♡
Automated review by Jibril · 2026-07-27
CI/CD: passed for head
c89dec1(forgejo-actions coverage bot rev #1, 738/738 green, 96.3% line) · Local checks: skipped — submodule-pointer-only diff, fix verified upstream at OpenRouter.Net#6a756ad0