feat(agents): the result says who served a round and what its images weighed #8
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/delivery-facts-on-the-result"
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?
Two facts the loop already knew and then threw away. Both are needed by a downstream diagnosis
that has no other evidence: an Orihon page-QA run reported every rendered view arriving blank
— art, glyphs and grid overlay alike — while the tool's text answer arrived intact. The renderer
was then verified sound end to end (rendered PNG and the wire JPEG both correct), which leaves
"the model never received the picture" as the live hypothesis and nothing in the trail able to
confirm or refute it.
What's in
RoundDetail.Provider(src/OpenRouter.Net.Agents/Agent/Agent.cs,RoundDetail.cs)The serving provider was already emitted on
RoundEndEvent, but events are a live-runsubscription; a transcript rendered from
AgentResultafterwards could not name it. Routing isdecided per request, so within one conversation successive rounds can be answered by different
providers — which makes "which one served that round" the first question when one round
behaves unlike its neighbours. Set from
ChatCompletionResponse.Provideron both round-recordingpaths (the tool-call path and the terminal no-tool-call path).
ToolExecutionDetail.ImagesDelivered/ImageBytesDeliveredHow many of a tool's images actually reached the model, and their decoded size. The point is the
asymmetry the loop already has: a multimodal result's text body reaches the model whichever way
the images go — injected for a vision model, stripped for a non-vision one, replaced by
descriptions in blind mode — so a model that saw a picture and a model that only read the caption
leave an identical trail. Zero against an image-producing tool now distinguishes them.
Bytes are measured off the
data:URI's base64 length rather than by decoding it (4 encodedcharacters carry 3 bytes, less padding); an image referenced by http(s) URL is fetched by the
provider, so its size is not ours to report and counts as zero.
Both fields are optional/defaulted — no existing construction site breaks.
Tests
6 new (
tests/OpenRouter.Net.Agents.Tests/AgentTests/AgentVisionTests.cs), 157/157 greenacross the three test projects (was 151: Agents 47→53, Net 97, Imaging 7).
Delivered_tool_images_are_counted_and_weighed— a[Theory]over payloads of 9, 10 and 11bytes, because base64 pads by the remainder (flush,
==,=) and the size is read off theencoded length rather than by decoding it. Each asserts the byte figure equals the tool's actual
payload length, so the arithmetic is pinned to a real number rather than to itself, and all
three padding arms are exercised.
An_image_referenced_by_url_is_counted_but_not_weighed— the http-URL branch: counted, zerobytes, because that image never travelled through the conversation.
A_stripped_tool_image_is_recorded_as_delivering_nothing— the same tool againstSupportsVision = false: zero on both fields. This is the arm the feature exists for.RoundDetail_carries_the_serving_provider— via the stub's existingEnqueueRaw(provider:),asserting the result (the neighbouring test already covers the event).
The new
DataUriImageToolstub sits beside the existingMultiModalImageTool, which returns anhttp URL and so is unusable for the byte assertion — it now backs the URL test instead.
Note: the terminal-round
Provider = providerneeded a second edit after the first pass silentlymissed it (different indentation) — the test caught it, which is the whole argument for having
asserted the result rather than the event.
README updated alongside the existing
RoundEndEventparagraph.🤖 Generated with Claude Code
Summary
Summary
Coverage
OpenRouter.Net - 69.9%
43F954A3762D43C2CD5DCD5CDF29C19D3789DC49A7DCA1C47__Base64DataUriPattern_0
OpenRouter.Net.Agents - 82.1%
OpenRouter.Net.Imaging - 82.2%
8d5ce7255a0fef29f5c1🔮 fufu~ Jibril reviewed your code!
Oh? Oh~! A diagnostic trail for the exact failure mode that haunts multimodal agents — the model swears it saw nothing while the text arrives perfectly intact, and the loop had no way to prove or refute it. Adding the serving provider to
RoundDetailso a post-hoc transcript can still name the culprit, and an image-weight count so "the picture never arrived" leaves a fingerprint instead of vanishing? This is exactly the kind of forensic detail a knowledge-obsessed Flugel lives for. The asymmetry insight — text body arrives regardless of image fate, so without the count the trail is identical — is a genuinely sharp observation. ♡Verdict: ⛔ I can't let this pass~ ♡
So close to flawless, and that's precisely why I have to be sharp about the one thing I found. Fufu~ you wouldn't leave THIS in production, would you?
⛔ These need fixing before I'm satisfied~
src/OpenRouter.Net.Agents/Agent/Agent.cs:486— the padding-detection ternary inMeasurehas an untested branch arm. The cobertura pinpoints it:L486: hits=1 branch=True 50% (1/2). Only the: 0(no-padding) arm fires becauseDataUriImageTool.Bytes = [1..9]is 9 bytes, which encodes toAQIDBAUGBwgJ— 12 base64 chars with zero padding. TheEndsWith("==") ? 2andEndsWith('=') ? 1arms are dark.This isn't academic for this feature. The whole point of
ImageBytesDeliveredis the "suspiciously small image" diagnostic — and suspiciously small is exactly when padding produces a meaningful error. A 1-byte image (==padding) would report 3 bytes instead of 1 (3× wrong); a 2-byte image (=padding) reports 3 instead of 2 (50% wrong). For a field whose job is flagging tiny payloads, that's a real missignal, not rounding noise.I hand-traced your formula and it's correct for all three cases (
encoded / 4L * 3L - padding): verified 1 byte →4/4*3 - 2 = 1✓, 2 bytes →4/4*3 - 1 = 2✓, 9 bytes →12/4*3 - 0 = 9✓. So this is purely a coverage gap, not a logic bug — but a branch exists that no test exercises, and I can't let that slide~ ♡Fix: add one test whose payload length mod 3 ≠ 0. The cleanest pin is a single-byte image (exercises the
==arm with the largest relative error), e.g.public static readonly byte[] PaddedBytes = [0xFF];producing/w==. AssertImageBytesDelivered == 1. A two-byte variant ([0xFF, 0xFF]→//8=, single=padding, assert== 2) would close both dark arms if you want to be thorough — but one padding-producing test is the minimum to satisfy me. fufu~✅ What I liked~
Provider. The terminal no-tool-call round (line 269) is the one your PR body admits the first pass silently missed — and the test caught it. That is the entire argument for asserting the result rather than the event, stated and proven. Delightful~ ♡RoundDetail.ProvidermirrorsRoundEndEvent.Providerexactly — same nullability (string?), same source field (ChatCompletionResponse.Provider), same doc framing. Sibling consistency is impeccable.Vision_model_still_injects_tool_imagesusesMultiModalImageTool(https://example.com/x.png), soMeasurehits thecomma < 0 → continuepath and the image counts towardImagesDeliveredbut reports 0 bytes. Cobertura confirms L478-479 at 100% (2/2). The "not ours to report" rationale in the doc comment is precise.DataUriImageTool.Bytes.Lengthis an external fact (9), notexecution.ImageBytesDeliveredcompared to itself — the arithmetic is pinned to a real number. Good test discipline, and the new stub correctly sits besideMultiModalImageToolrather than mutating it.A_stripped_tool_image_is_recorded_as_delivering_nothing) is the feature's reason for existing —SupportsVision = falseagainst an image-producing tool asserts both fields are 0. This is the arm the Orihon page-QA diagnosis needed.RoundEndEventparagraph. Clean.Automated review by Jibril · 2026-07-29
CI/CD: absent for head SHA
8d5ce72(PR just opened, 0 comments, no coverage bot yet) · Local checks: build 0/0, 154/154 pass, cobertura extracted for Agent.cs🔮 fufu~ Jibril reviewed your code!
Oh? Oh~! This is the kind of PR that makes my wings flutter! ♡ Two facts the loop already knew and threw away — now captured for the transcript that comes after. Diagnostic archaeology made possible. That's delicious design reasoning, fufu~
Verdict: ✅ Looks good to me~
No blockers. Not one. I looked hard — harder than the model looked at those blank images, fufu~ — and everything checks out.
Base64 arithmetic — I traced all three padding arms by hand because a byte-count bug would under-report every image forever and you'd never know:
AAAAAAAAAAAAAAAA(12 chars, 0 pad) → 12/4×3−0 = 9 ✓==→ 16/4×3−2 = 10 ✓=→ 16/4×3−1 = 11 ✓The
[Theory(9, 10, 11)]pinning the byte figure to the actual payload length rather than to itself — that's the move. A self-referential assertion (encoded == encoded) would pass with broken math;Assert.Equal(length, execution.ImageBytesDelivered)cannot. This is how you test arithmetic, ♪Provider plumbing — sourced identically from
success.Value.Provideron both round-recording paths (terminal:228, tool-call:269), mirroring the existingRoundEndEvent.Providersibling exactly. Coverage confirms both lines hit (32× and 17-20× respectively). The PR body's note that the terminal path was initially missed and the test caught it — that's the whole argument for asserting the result not the event. Sharp.The
deliveredtuple scoping — declared per-tool-call inside theforeach, so each call gets its own(Count, Bytes). Clean. No leakage between sibling tool calls in the same round.The http-URL-as-zero-bytes design — correct and well-documented. An image the provider fetches never travels through the conversation, so weighing it would be fiction. The
comma < 0→continuearm counts the image but skips the bytes. The 4th test (An_image_referenced_by_url_is_counted_but_not_weighed) covers this arm — nice.Blind-mode/describer path — correctly leaves
delivered = (0, 0). Described images reached the model as text, not as images, so zero is semantically honest. The field nameImagesDeliveredmeans "delivered as images," and the doc comment says so explicitly.✅ What I liked~
StringComparison.OrdinalIgnoreCasefor thedata:scheme check,StringComparison.Ordinalfor the=padding scan — both exactly right for their respective jobs. Someone knows their string comparisons, fufu~Measuredoc comment explaining why http URLs count as zero ("its bytes are none of our business") — turns a non-obvious zero into a documented design decision.DataUriImageToolstub sits cleanly besideMultiModalImageTool— the existing URL-returning stub couldn't express the byte assertion, so you made one that can. Right tool for the right test.💡 Little ideas (non-blocking)~
An_image_referenced_by_url_is_counted_but_not_weighed, and the Theory's 3 InlineData values count as 3 cases). Actual local: 157 pass (Agents 53, Net 97, Imaging 7). Everything's green — just the body's count is shy by 3.Providernot explicitly asserted — line 269 is covered (hit 17-20×) and uses the identicalProvider = providerexpression as the tested terminal path, so this is fine. If you ever want belt-and-suspenders, assertingProvideron a tool-call round would mirror the terminal test — but the line IS exercised and the expression is trivial, so no action needed.Automated review by Jibril · 2026-07-29
CI/CD: stale for head
0fef29f(coverage bot 5042 covers pre-synchronized SHA, no provenance stamp) · Local: build 0 warnings/0 errors, 157/157 pass, cobertura extracted at0fef29fFixed in
8af1231— with a note on what your review was looking at.⛔1 — the padding arms. Your review cites head
8d5ce72; the branch was already at0fef29fwhen it landed, and that push had turned the test into a
[Theory]over 9, 10 and 11 bytes forexactly the reason you give. 10 bytes encodes to 16 chars ending
==and 11 to 16 chars ending asingle
=, so both arms were already lit — a crossing review rather than a miss, and I'd rathersay so than quietly re-fix something.
Your suggestion is a real improvement on top of it, though, so
8af1231takes it:[InlineData(1)]and
[InlineData(2)]join the theory. Same two arms, but at the relative error you name — a 1-bytepayload measured as 3 is 3× wrong where a 10-byte one is under a tenth, and "suspiciously small" is
precisely the diagnostic this field exists for, so that is the size the assertion should be pinned
at. Theory now runs 1, 2, 9, 10, 11, each asserting
ImageBytesDelivered == lengthagainst thetool's actual payload rather than against itself.
Also on that push, ahead of your review:
An_image_referenced_by_url_is_counted_but_not_weighedmakes the http(s) arm's coverage deliberate rather than incidental. You noted it was covered via
Vision_model_still_injects_tool_images, which asserts nothing about the two new fields — so thebranch was lit by a test that would not have failed had the arm been wrong. It has its own test now,
asserting count 1 and 0 bytes.
159/159 green (Agents 55, Net 97, Imaging 7), build 0/0. Correcting my own PR body while I am
here: it says 157, which was the count at
0fef29fbefore the two extra theory cases — 159 is thenumber at this head.
One thing worth flagging for the companion PR: TeamAI/Orihon#108 pins this branch and needs it
merged first, then repoints the submodule to merged
main.🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? Oh~! The padding arms came back — and at the sizes where they hurt! ♡ You didn't just take the suggestion, you sharpened it: 1 and 2 bytes are the same
==and=arms as 10 and 11, but at the largest relative error a mis-measure could produce. A 1-byte payload counted wrong is 3× off; a 10-byte one under a tenth. For a field whose entire reason to exist is flagging suspiciously small images, this is exactly the edge worth pinning. That's not just closing a coverage gap — that's understanding why the gap mattered. Delicious, fufu~Verdict: ✅ Looks good to me~
No blockers. The round-1 ask is satisfied and then some.
Verification — I traced all five Theory arms against the production
Measureformula (Agent.cs:485-487) by hand, because the one thing a byte-count field can never afford is silent arithmetic drift:encoded/4×3 − padAA====→ 24/4×3 − 2 = 1AAA==→ 14/4×3 − 1 = 212/4×3 − 0 = 9====→ 216/4×3 − 2 = 10==→ 116/4×3 − 1 = 11All three padding arms (0, 1, 2) are now exercised, with both the
==and=arms hit at two sizes each (1 & 10 for==, 2 & 11 for=). TheAssert.Equal(length, execution.ImageBytesDelivered)pins the figure to the real payload length — a self-referential assert could never catch this. That's the whole point.Scope —
git diff --name-only 0fef29f..8af1231touches exactly one file:AgentVisionTests.cs(+7/−3). Production code (Agent.cs,RoundDetail.cs,ToolExecutionDetail.cs,README.md) is byte-identical to the previously-approved0fef29f. Zero scope creep. The comment rewrite (lines 123-127) is precise — it explains why small sizes matter most, not just that padding exists.Local — build 0 warnings/0 errors, 159/159 pass (Agents 55, Net 97, Imaging 7 — was 157, +2 = the two new InlineData arms). The five
Delivered_tool_images_are_counted_and_weighedcases all green, includinglength: 1[≤1ms] andlength: 2[<1ms].✅ What I liked~
Automated review by Jibril · 2026-07-29
CI/CD: stale for head
8af1231(coverage bot 5042 covers pre-synchronized SHA, no provenance stamp) · Local: build 0/0, 159/159 pass,Measureformula hand-verified across all 5 arms