fix: a transcript says which round the run died on, and why #119
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/reasoning-detail-tolerance"
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?
Found by reading a real failure:
google/gemini-3.5-flashkilled three pages' boxing pass, and the transcript the tab now shows could not say why.Its header read
ApiError after 4 round(s), the trail rendered three rounds, and then simply stopped. The reason — the actual provider error — appeared nowhere in the file, only in the execution row's error string on another screen. A transcript that cannot account for its own last round fails at the one job it has.The cause
Agent's loop breaks on an API failure before building aRoundDetailfor the round that failed, soIterationscounts it andRoundsdoes not.AgentResult.ApiErrorcarries the failure — flat message, HTTP status, and the provider's structured error when there is one — andAgentTranscript.Renderwas ignoring it entirely.The change
When
ApiErroris set, the trail closes with the round that died:…plus a
provider said:block fromApiError.Details.Metadatawhen the upstream explained its own refusal (a moderation reason, a raw provider message). That block is conditional on purpose: a transport failure has no metadata, so its presence is a positive fact rather than an always-there field that is sometimes empty.Note the status is
200in that example — the provider returned a perfectly good HTTP response that this client then refused to parse. That is exactly the class of failure the old trail hid.The other half of this bug
The failure itself is a client-side defect in OpenRouter.Net, fixed in TeamAI/OpenRouter.Net#9:
ReasoningTextDetail.Textwasrequired, and Gemini sendsreasoning.textsegments with notextat all, so every response carrying one failed to deserialize and burned all three retry attempts on the same unparseable answer.Merge order: OpenRouter.Net#9 first, then I push the submodule-pointer bump onto this branch. This PR is the diagnostics half and stands on its own — the submodule pointer is untouched here.
Tests
+3 → 1093 total, all green (Domain 114 · UseCases 510 · Integration 219 · BlazorAdapter 250), full
dotnet test Orihon.slnx, 0 warnings / 0 errors.A_run_that_died_on_the_provider_says_so_where_the_rounds_stop— the real deserialization message, and the round number that accounts for the gap between the header's count and the rounds rendered.A_provider_that_explained_its_refusal_has_that_in_the_trail_too— the metadata block, with the provider name and its raw message.A_run_that_did_not_fail_says_nothing_about_a_failure— the negative, so the block cannot creep into a clean trail.Notes
0|thoughtprefix ahead of its reasoning, rendered undersaid:. Nothing in this codebase or in OpenRouter.Net formats a string that way, so it is arriving from the provider or its OpenRouter transform. Worth a look if it recurs, but I would be guessing at the cause today.🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 95.6%
Orihon.Domain - 100%
Orihon.Infrastructure - 96.7%
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 94.4%
Orihon.UseCases - 97.5%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A transcript that couldn't explain its own death — and now it can! This is the kind of bug I love to see hunted down. You read a real failure, traced it from the field all the way back to the loop's break-before-build, and fixed the exact seam where the story fell silent. fufu~ ♡
I traced every link in your chain myself:
Agent.cs:150-321— confirmed.iteration++runs at:155before the API call; the failure break at:177fires before anyrounds.Add(:223/:265). SoIterations = iterationat:326counts the dead round andRoundsdoes not. Your diagnosis is exact.Result<T>.Failure—Erroris non-nullablestringin the record signature, soAppendIndented(text, failed.Error, " ")is null-safe. Good.ApiError.Metadata—IReadOnlyDictionary<string, object>?, andis { Count: > 0 }resolvesCountviaIReadOnlyCollection<KeyValuePair<>>. The?.+ property-pattern guard is correct on both layers.result.Iterationsoverinvocation.MaxIterations, notRounds.Count. That's the load-bearing choice, and your test pins it:round 4/4against aRoundslist of one. If anyone ever swapped that toRounds.Count, the test goes red. That is how you test a number. ♪Your pattern fidelity to the rest of the file is impeccable —
is { Count: > 0 }mirrorsround.Provider is { Length: > 0 }, theAppendIndentedusage matchesCap(call.Result)(also unguarded, also fine), and the conditionalprovider said:block absent-on-transport-failure is the same "presence as positive fact" philosophy as the image-clause tests above it. Consistent to the bone.Verdict: ✅ Looks good to me~
💡 Little ideas (non-blocking)~
AgentTranscript.cs:124— the null-StatusCodearm. EveryRenderFailuretest case sets aStatusCode(200, then 400). Theif (failed.StatusCode is { } status)false arm — no status line at all — has no test exercising it. It's a real runtime path:Agent.cs:185constructsnew Failure("Response contained no assistant message.")withStatusCodedefaulting tonull(a 200-OK with empty choices). The transcript would still render correctly (error message present, status line simply absent) — the logic is fine — but a one-lineAssert.DoesNotContain("status:", trail)on a no-StatusCode failure would pin the silence and close the last branch. Pure coverage nicety; the three substantive branches (ApiError present/absent, metadata present/absent) are all directional and solid.✅ What I liked~
A_run_that_died_on_the_provider_says_so_where_the_rounds_stop. Beautiful.RenderFailurehelper models the real gap precisely — one round inRounds, four inIterations— so the round-number assertion is genuinely load-bearing, not decorative.provider said:block being conditional-on-presence rather than always-there-sometimes-empty — philosophically consistent withA_call_that_delivered_no_image_says_nothing_about_imagesin the same file. The "absence is the diagnosis" thread runs clean through the whole suite.$0.023 / 1.8k-tokencost note and the0|thoughtprefix in the field transcript and not acting on them blindly — disciplined. ♡Automated review by Jibril · 2026-07-30
CI/CD: absent for head
2bbc9ef(PR just opened, 0 comments) · Local checks: skipped — submodule-pinned type verification performed via clone of OpenRouter.Net @ b975727 (the pinned pointer), full agent-loop trace confirms PR diagnosisThe submodule half has landed.
32aa864bumpsexternal/OpenRouter.Netfromb975727to75ec695— mergedorigin/mainof TeamAI/OpenRouter.Net#9, repointed onto the merge commit rather than my branch tip, which the merge leaves behind. So this PR is now the whole fix rather than half of it, and the merge-order note in the body is discharged.The bump comes with the regression test that belongs on this side of the boundary:
AgentRunnerTests.A_reasoning_segment_the_model_withheld_does_not_cost_the_round— a full agent round over the gateway's real HTTP path, answering with the shape Gemini actually sent: tworeasoning_detailssegments, one carrying{"type":"reasoning.text","index":0,"signature":"Cs4CAcv3..."}with notextat all, one carrying a thought it did share. The run has to come backOkwith its final text, not anApiError.The vendor library proves the parse; this proves the thing that was actually lost — an agent round. It also pins the submodule pointer: revert it and this test is what complains.
I verified it is load-bearing rather than assuming it. Checked the submodule back to
b975727, rebuilt, ran that test alone → FAIL. Restored75ec695, rebuilt → PASS. So the assertion is measuring the fix and not something incidental.1094 total, all green (Domain 114 · UseCases 510 · Integration 220 · BlazorAdapter 250), full
dotnet test Orihon.slnx, 0 warnings / 0 errors.One honest note on the way there: an intermediate full-suite run had
SetupChatTests.A_stop_at_the_round_cap_ends_the_run_without_a_ghost_restartfail once. It passes alone, passed two consecutive full BlazorAdapter runs afterwards, passed the final full-solution run, and lives in a file this branch does not touch — so I am calling it flaky under parallel load rather than caused by this change. Recording it because a reviewer who sees it once should know it has been seen before, and where.🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? The plot thickens~ The first half of this bug taught the transcript to say why it died — and now you've come back to make sure it doesn't die in the first place! The submodule pointer lands OpenRouter.Net #9 (
b975727→75ec695), and you've pinned the cure at the level it was felt: a lost agent round. This is how you close a loop. fufu~ ♡Let me show you the thread I pulled~
75ec695— that's the merge commit of OpenRouter.Net #9, and0161bb9(my last-approved commit on that PR) is a direct ancestor. I diffed the trees:0161bb9..75ec695is empty — the merge introduced no new code. The pointer lands exactly the fix I already vetted, nothing more.ReasoningTextDetail.Text— nowstring?(wasrequired). I read the file at75ec695: the doc comment is precise ("a Geminireasoning.textblock can carry only the metadata around a thought whose plaintext is withheld"), andOptional rather than required because a missing nicety must not fail the whole completion— that's the exact philosophy that makes your new test meaningful.A_reasoning_segment_the_model_withheld_does_not_cost_the_round— this is NOT a tautology and I'll prove it. At the old pointer (b975727), thereasoning.textsegment in yourFinalResponseWithWithheldReasoning(a segment withsignaturebut notext) would have hit therequiredproperty and thrown the very deserialization error your transcript now reports —FinalTextwould never be"Nothing to change here."and the run wouldn't have completed. Your test asserts the round survives (Iterations == 1,FinalText == "Nothing to change here.") — that assertion would have gone red before the bump. That is a directional regression test. ♪FinalResponseWithWithheldReasoningmodels the real Gemini response precisely: tworeasoning_detailssegments side by side, one withheld (signature only) and one shared (textpresent), beside a normalcontentanswer. You didn't just test the empty-text arm in isolation — you proved the polymorphic deserializer handles both shapes in the same response without the whole call failing. That's the real-world payload.Your
FinalResponseWithWithheldReasoninghelper mirrors the establishedFinalResponsepattern exactly — same structure, samechoices[0].messageshape, samefinish_reason: "stop", just with the reasoning segment the old code couldn't parse. Consistent to the bone with its sibling.Verdict: ✅ Looks good to me~
No blockers, no new suggestions. The one non-blocking from my first pass (the null-
StatusCodearm) still stands — it's a coverage nicety on the transcript rendering, not this commit's concern, and I won't repeat myself. ♡✅ What I liked~
http.Enqueue(FinalResponseWithWithheldReasoning(...))driving the realOpenRouterLlmGateway.RunAgentAsyncover the real loop, asserting a cleanOk<AgentOutcome>— this is the most honest integration test shape: no mocking around the seam that broke.FinalResponseWithWithheldReasoninghonestly names both segments ("one that did share its thought") — the test isn't just exercising the failure shape, it's proving the parser handles a mixed batch correctly. That's a stronger assertion than the minimum.Automated review by Jibril · 2026-07-30
CI/CD: coverage bot stale for
32aa864(covers2bbc9ef) · Local checks: build 0 warnings/0 errors, 21/21 Integration.Tests pass (AgentRunnerTests + AgentTranscriptTests, was 17 on main — +4 = the new withheld test + 3 transcript tests from2bbc9ef), submodule verified at 75ec695 = merge of approved OpenRouter.Net #9