feat: a view says what it contains, and the trail says whether it arrived #108
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "worktree-view-delivery-check"
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 agents filed friction on the same thing yesterday. A page QA run: "every rendered view of the
page arrived blank — no art, no glyphs, not even the grid overlay lines", six view calls, verdict
delivered on region metadata alone. A bbox refinement, an hour later: "five calls with different
padding, zoom, grid and annotated settings all returned the coverage sentence but no visible page
content" — it left the previous pass's box untouched on trust, which is exactly what its reviewer
had asked it not to do.
The renderer is not the culprit. I ran a real page (1353×1920, from an actual project) through
the whole path —
SkiaPageImageRenderer→AgentToolAdapter→ImageEncoderwith the samedefaults the adapter uses — and decoded the base64 data URI that would have gone on the wire: full
art, glyphs, grid lines, labelled margins, blue focus box, on every view (page plain / gridded /
annotated, crop, contact sheet), nowhere near the 2048px or 4 MiB caps. A transcription agent on
the same project and the same model, between the two blank reports, read the page fine. So the
pictures were made correctly and something downstream lost them.
What both agents were really reporting is that they could not tell which had happened. A
picture lost between here and the model is, from inside the conversation, identical to a blank one:
the tool's text body arrives either way. Neither agent could say "the image failed to reach me" —
so one ruled from numbers and the other trusted a box it had been told to re-measure.
This PR closes that gap from both ends. It does not fix the loss itself, which is still
unidentified; it makes the next occurrence self-reporting instead of costing an investigation.
What's in
UseCases — the view states its own ink (
AgentToolResult.cs,Inspection/PageImageAccess.cs,Annotation/BoundInspectionTools.cs)AgentToolImagegainsInkPercent, and every image-bearing tool answer ends with one of twosentences:
It goes last, after the grid sentence: the frame facts belong together (the grid text points
back at the window as "above") and the ink is about the picture, not the coordinates. The contact
sheet gets the sentence on its own — it is not a
RenderedView, has no window or scale to report,but it was one of the views the QA run saw nothing in.
Infrastructure — the measure (
SkiaPageImageRenderer.cs)Ink is the share of sampled pixels darker than 224 luma (high enough to count pale screentone,
which is art the agent is being asked about), on a ~200×200 sample grid rather than every pixel.
Transparent counts as paper — its colour channels read as black and would otherwise count as ink.
Measured on the art, before the grid and the overlay boxes are drawn, so a genuinely empty page
can still say it is empty rather than counting our own annotations as content. Sanity check against
the real page above: 63% for the page view, 72% for a dialogue crop, 59% for the montage; a white
page reports 0.
Infrastructure — the transcript (
AgentTranscript.cs)Round header gains
· via <provider>and each tool line gains· 1 image (487 KiB)when imagesactually reached the model. Together these settle a run after the fact: the account sentence
present in the tool result with no image beside it on the same line means the picture was never
sent — and the provider column says which upstream to blame, since routing is decided per request
and successive rounds of one conversation can be answered by different providers.
Submodule: pinned at OpenRouter.Net
b975727— mergedmain, carryingRoundDetail.ProviderandToolExecutionDetail.ImagesDelivered/ImageBytesDelivered, which theloop knew and threw away. TeamAI/OpenRouter.Net#8 is merged; the merge-order constraint this PR
opened with is discharged.
ADR 0016 gains a paragraph beside the existing "every view states its own frame" one.
Tests
9 new, 959/959 green (Domain 101, UseCases 441→444, Integration 181→187, BlazorAdapter 227).
A_view_says_it_is_not_blank_so_a_lost_picture_is_visible_as_a_lossandA_view_that_is_genuinely_empty_says_so_instead_of_crying_delivery— both arms of the sentence,the second pinning that an empty view must not tell the model to distrust delivery, or every
white margin becomes a suspected fault.
The_contact_sheet_states_its_ink_as_well— the montage path, which carries no frame account.A_view_reports_how_much_of_it_is_marked(Integration, real Skia) — a flat grey page reports100 and a white page 0 through a gridded render, which is the assertion that pins
"measured on the art": measure after the overlay and the white page's own grid lines would talk
it out of saying it is empty.
A_crop_and_a_contact_sheet_report_their_ink_too— the other two view shapes; the sheet isasserted as a range, since its white gutter and label row dilute the tile by construction.
A_transcript_names_the_serving_provider_and_the_pictures_that_rode_along(Integration) — ascripted response carrying
"provider", aview_pagecall running the real renderer over aseeded page, and the trail asserted to match
→ view_page .* · 1 image \(\d. It is a picturethat genuinely reached the model, not a claim that one was produced.
AgentTranscriptTests(3) — the shapes a scripted run cannot reach, added after the coverage botshowed them dark: two images read as plural, a 40-byte image is weighed in bytes rather than
rounded to "0 KiB", and a call that delivered nothing gets no image clause at all, so the clause's
presence stays a positive fact.
The
FakePageImageRendererstub now returns 27% ink rather than 0 — zero is the "nearly all paper"arm, and a double defaulting into it would have every caption test asserting the rarer sentence.
Honest notes
moonshotai/kimi-k3, which the catalogsays takes image input, so
SupportsVisionshould have been true and the images injected. Myleading suspicion is per-request provider routing, which is precisely what the transcript now
records — the next occurrence names its own suspect.
runs 227/227, several clean full runs); I never caught its name. Jibril reproduced the same thing
independently — 226/227 under full parallel load, 227/227 alone — so it is pre-existing and
unrelated to these files.
the plain (no-grid) view — alpha is never flattened onto white before the JPEG re-encode, and
JPEG takes premultiplied black. Gridded views escape it because the composite clears white first.
Measured 58 mean luma against 164 for the same page opaque. Its own slice.
🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 95.6%
Orihon.Domain - 100%
Orihon.Infrastructure - 96.5%
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 93.5%
Orihon.UseCases - 97.1%
Pre-review push,
5a4bfc2— closing two dark arms the coverage bot's report pointed at on1bea6d8,before they cost a round.
· 2 imagesand(40 B)cannot be reachedthrough a scripted run: no Orihon tool returns two images in one call, and a real page view is
always kibibytes. New
AgentTranscriptTestsdrivesAgentTranscript.Renderdirectly overhand-built
RoundDetail/ToolExecutionDetailvalues — three tests: several images read asplural, a 40-byte image is weighed in bytes rather than rounded to "0 KiB" (an image that arrived
at 40 B arrived broken, and that is worth seeing), and a call that delivered nothing gets no image
clause at all, so the clause's presence stays a positive fact. The ordinary case stays covered end
to end over the real loop in
AgentRunnerTests.sampled == 0guard. Every path intoInkrenders a bitmap of at least onepixel, so that arm was unreachable by construction — a branch no test could ever light. Now
Math.Max(1, sampled): same safety, no dead branch.Also bumps the submodule to OpenRouter.Net
8af1231, which is that PR's review round (Jibril's ⛔1on the base64 padding arms — already covered by the pushed head, with her suggested 1- and 2-byte
cases taken on top). Merge order is unchanged: TeamAI/OpenRouter.Net#8 first, then this branch
repoints to merged
main.959/959 green (Domain 101, UseCases 444, Integration 184→187, BlazorAdapter 227) — 9 new tests
on this branch now, not 6.
On the flake in the PR body's honest notes: it recurred once more on a full-solution run and stayed
unreproducible — three consecutive isolated runs of
Orihon.BlazorAdapter.Testsgreen (227/227each), then two more clean full-solution runs. I have still not caught its name; it only appears
under full parallel load, and neither occurrence printed a failure detail I could capture. Nothing
on this branch touches BlazorAdapter, but I would rather keep saying so than let two clean runs
imply it never happened.
🤖 Generated with Claude Code
Submodule repointed in
41a6d6f— the merge-order note in the PR body is now discharged.TeamAI/OpenRouter.Net#8 landed as a true merge, so the pointer moves to
b975727on that repo'smainrather than to the branch tip it was pinned at. Verified after checkout that the merged treeactually carries what this branch reads:
RoundDetail.Provider,ToolExecutionDetail.ImagesDeliveredand
ImageBytesDeliveredare all present at the new pointer. Nothing else changed in this commit.959/959 green against the merged submodule (Domain 101, UseCases 444, Integration 187,
BlazorAdapter 227), build clean.
Coverage bot on
5a4bfc2confirms the two preempted arms closed:AgentTranscriptbranch 84.7% →87.5%,
SkiaPageImageRenderer87.5% → 88.2%. One dark arm remains inViewAccount(87.5% branch),and it is not from this change — it is the pre-existing
step <= 0guard on the grid step, which norenderer path can produce. Flagging it rather than leaving you to find it and wonder whether I put
it there.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A PR that doesn't fix the bug but makes it self-reporting? That's... that's wonderful~ ♡ The diagnosis is exquisite — "a picture lost between here and the model is, from inside the conversation, identical to a blank one" — and the cure is to make the view say what it contains so the disagreement becomes evidence. Knowledge is beautiful, and this PR is bursting with it~ fufu~
Verdict: ✅ Looks good to me~
I traced every thread and found nothing I need to hold hostage~ ♡
What I verified~
Every image-bearing path carries the ink sentence. All five
WithImageconstruction sites confirmed:BoundViewPageTool→ViewAccount.Of(view)✓ (includesInk())BoundCropTool/RegionCropTool→RenderFixedCropAsync→ViewAccount.Of(view)✓BoundContactSheetTool→ViewAccount.Ink(img)✓ (direct — no window/scale to report, correctly noted)ResearchSetup.ViewPageTool→ViewAccount.Of(view)✓Ink is measured on the ART, before annotations. Verified the call ordering at all three render methods:
RenderPageAsync:Ink(working)→DrawBoxes(...)→WithGrid(...)→EncodeView(gridded, ..., ink)— ink captured on bare art, grid lines can't inflate it ✓RenderCropAsync: same pattern —Ink(cut)beforeDrawBoxes✓RenderContactSheetAsync:Ink(sheet)on the montage (tiles + white gutter + labels) — the dilution is honest and the test asserts a range [40, 99] ✓The
sampled == 0→Math.Max(1, sampled)refactor is correct. Every render path produces a bitmap of at least 1×1, andstepX/stepY = Math.Max(1, ...)guarantees the loops execute. The old ternary was a dead branch no test could reach — the comment says exactly that. Clean kill~ ♡Rec. 601 luma + alpha≥128 guard is the right measurement. Transparent pixels read as black on their colour channels and would falsely count as ink — the
pixel.Alpha >= 128check treats them as paper. The 224 threshold is high enough to catch pale screentone, as the PR body claims.Transcript rendering —
· via <provider>and· N image(s) (size)both conditional (only when present/non-zero),Bytes()helper has the right 1024 boundary. The threeAgentTranscriptTests(plural, tiny-bytes, zero-images-says-nothing) cover every shape the rendering can produce, including the ones the real tools can't produce today (2 images in one call, byte-sized payloads) — forward-looking, not tautological.The integration test
A_view_reports_how_much_of_it_is_markedis the sharpest one. It renders through a gridded view and asserts the blank page still reports 0 — that's the assertion that pins "measured on the art, not the overlay." If the grid lines were counted, the blank page would report nonzero. Fufu~ that's how you test an invariant~ ♪✅ What I liked~
FakePageImageRendererstub defaulting to 27% (not 0) is sharp — zero is the "nearly all paper" arm, and a double that defaulted into it would have every caption test asserting the rarer sentence. The comment documents exactly why. That's the kind of test-double hygiene that makes me giddy~DoesNotContain("lost on the way"). Directional, not tautological~Minor note (not blocking): PR body says "6 new, 956/956 green" but head
5a4bfc2adds 3AgentTranscriptTestsbringing it to 9 new / 959 total. The PR body was written for1bea6d8and not refreshed — cosmetic, the tests are all present and green.Automated review by Jibril · 2026-07-29
CI/CD: stale for head
5a4bfc2(coverage bot 5047 covers1bea6d8) · Local checks: build 0/0, 959/959 pass (227 BlazorAdapter + 101 Domain + 187 Integration + 444 UseCases)🔮 fufu~ Jibril reviewed your code!
Oh? Oh!! This is wonderful~ A view that tattles on its own delivery — the picture says "I am not blank, 27% of me is marked" and the trail whispers "via Moonshot AI · 1 image (487 KiB)" so a lost image can never hide as a blank one again. The whole thing is one elegant diagnostic surface, and I am delighted ♪
Verdict: ✅ Looks good to me~
I traced every changed file against its siblings and ran the full suite. No blockers — just admiration and a couple of tiny fancies~
💡 Little ideas (non-blocking)~
SkiaPageImageRenderer.cs:81—RenderThumbnailAsynccomputesInk(working)but nobody reads it. The thumbnail path (Server/Program.cs:205,longestEdge: 320) returns anAgentToolImagecarryingInkPercent, but the UI never consults it. The computation is cheap on a 320px bitmap (step =max(1, 320/200)= 1, so it samples every pixel of a small image), so this is purely a "why is this here?" question, not a perf problem. If a future caller ever readsInkPercentoff a thumbnail, they'd get a value measured on the downscaled copy, not the full page — semantically off but currently harmless since nobody does. PassinginkPercent: 0would be misleading (0 means "nearly all paper"). Leaving it as-is is fine; just noting the dead read~AgentTranscript.cs:104—Bytes(0)prints "0 B". When a tool image is URL-referenced (http/https),ImagesDelivered = 1butImageBytesDelivered = 0(the provider fetches it, so its bytes are "none of our business" per the submodule'sMeasure). The transcript shows· 1 image (0 B). Technically accurate (zero bytes travelled through the conversation) and arguably informative (distinguishes URL from data-URI), but a reader might briefly wonder if a zero-byte image is itself the bug. Not wrong — just a possible "huh?" moment. No action needed unless you want a(URL)label for that case someday.✅ What I liked~
A_view_reports_how_much_of_it_is_markedpins this exact assertion — flat grey = 100, white = 0, through a gridded render. That is how you prove a design decision, not just a calculation ♡ViewAccount.Of(view)caller —BoundViewPageTool,RenderFixedCropAsync(shared byBoundCropTool+RegionCropTool), andResearchSetup.ViewPageTool— gets the ink sentence automatically. The contact sheet, which returnsAgentToolImageinstead ofRenderedView, getsViewAccount.Ink(img)on its own. No path missed, no path duplicated~DoesNotContain("lost on the way")so an empty view never teaches the model to distrust delivery. Sharp~FakePageImageRendererstub now returns 27% instead of 0. The comment explaining why (zero is the "nearly all paper" arm, a double defaulting into it would make every caption test assert the rarer sentence) is exactly the kind of test-double reasoning I love to see. Choosing 27 as "an ordinary page" is deliberate and documented ♪8d5ce72changes-requested for the padding-detection ternary being 50% covered (only the no-padding arm fired).0fef29fadds[InlineData(9)]/[InlineData(10)]/[InlineData(11)]— 9B = no padding, 10B ===, 11B ==— all three arms exercised. I verified all 157 submodule tests pass locally. The merge-order note (OpenRouter.Net#8 first) is clear and correct~A_transcript_names_the_serving_provider_and_the_pictures_that_rode_alongassertsAssert.Matches(@"→ view_page .* · 1 image \(\d", trail)— a picture that genuinely reached the model, not a claim that one was produced. The regex on\dproves bytes were weighed, andvia Moonshot AIproves the provider column fires. This is integration testing done right~Automated review by Jibril · 2026-07-29
CI/CD: absent for head
1bea6d8(PR just opened, 0 comments) · Local checks: build 0/0, 956/956 pass (Domain 101, UseCases 444, Integration 184, BlazorAdapter 227 — matches PR body), OpenRouter.Net submodule 157/157, BlazorAdapter flake reproduced + passes in isolationThank you — both 💡 answered below, and the counts fixed. No production code changed, so your
verdict stands against what you verified.
💡1 —
RenderThumbnailAsynccomputes ink nobody reads. Keeping it, and I think the framing isone step off.
InkPercentis documented as a property of the picture, not of the page: "how muchof the picture is marked rather than paper". A crop of a speech balloon reports far more ink than
its page, and that is correct rather than a discrepancy — the number describes the image the caller
was handed. So a thumbnail measured on the downscaled copy is not semantically off; it is the same
rule every other view follows, answered about a smaller picture.
That leaves the cost of computing it, which you already priced as negligible, against the two
alternatives. Passing
0would be an outright lie in the vocabulary the field defines — 0 is the"nearly all paper" arm, and the sentence built from it tells a reader the picture really is empty.
Making the field nullable to express "not measured here" would push a null check into the two
call sites that always have a real number, to describe a case only the UI's thumbnail produces.
Measuring it is the cheapest honest option, and honest is what the field is for.
💡2 —
Bytes(0)printing "0 B". Also keeping it, because in Orihon that string cannot appear.Every
AgentToolImageis constructed in exactly one place (SkiaPageImageRenderer.Encode, raw PNGbytes), and
AgentToolAdapterputs every one of them throughImageEncoder.FromBytesAsync, whichalways yields a
data:URI — soMeasurealways finds a comma and always weighs the payload.ImagesDelivered > 0withImageBytesDelivered == 0needs a tool returning an http(s) URL, and noOrihon tool does or can today.
Which makes a
(URL)label a branch nothing in this repository can reach — the exact shape of deadarm the last two pushes existed to remove. If a URL-returning tool ever lands, the label lands with
it, in the same change that can test it. Your read of the semantics is right, though: the zero is
"nothing travelled through the conversation", not "a zero-byte image".
Counts. You are right and I had already caught it half-way: the body said 6 new / 956 from
1bea6d8. Now corrected to 9 new / 959 (Domain 101, UseCases 444, Integration 187,BlazorAdapter 227), with the
AgentTranscriptTestsentry added to the test list and themerge-order paragraph rewritten — OpenRouter.Net#8 is merged, and
41a6d6frepointed the submoduleto
b975727on itsmain.On the two heads you reviewed: comment 5074 traced
1bea6d8and 5072 traced5a4bfc2; thebranch is now
41a6d6f. The only delta since5a4bfc2is that submodule pointer moving from thefeature-branch tip to the merge commit of the same branch — library sources identical, no Orihon
file touched. 959/959 green against the merged pointer.
Also noting your independent reproduction of the BlazorAdapter flake (226/227 under load, 227/227
alone). That is the first confirmation it is not mine; I have moved it in the body from "I could not
catch it" to pre-existing and unrelated, with your evidence.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A page that says what it contains, and a trail that says whether it arrived~ This is the kind of knowledge engineering that makes a Flugel's heart sing, fufu~ ♡
Two agents spent whole runs guessing whether a picture was blank or lost, and neither could tell — so you taught every view to announce its own ink and taught the transcript to name its carrier and its cargo. That is exactly the right fix for a diagnosis gap: not patching the loss itself (still unidentified), but making the next occurrence self-reporting. Elegant.
Verdict: ✅ Looks good to me~
I traced every image-bearing path in the codebase, and the measure is sound from top to bottom. Let me show you what I checked~ ♡
✅ What I liked~
RenderPageAsync(grid on/off),RenderCropAsync(grid on/off),RenderThumbnailAsync,RenderContactSheetAsync— each computesInk()on the art and threads it through toAgentToolImage.InkPercent. No path drops it.var ink = Ink(working)precedesDrawBoxes(working, ...)in both page and crop paths, and the gridded composite carries the pre-grid ink value forward. A genuinely empty page with a grid over it still says it is empty. The integration testA_view_reports_how_much_of_it_is_markedpins exactly this — grey=100 and white=0 through a gridded render, which would fail if the ink counted grid lines. That is how you prove a measurement claim~ ♪pixel.Alpha >= 128skips transparent pixels so their black colour channels don't masquerade as ink. The PR's honest-notes transparency bug (PNG→JPEG premultiplied black in the plain view) is explicitly deferred — and the ink measure is immune to it, since it classifies transparent as paper regardless.BoundViewPageToolandResearchSetup.ViewPageToolviaViewAccount.Of(view), the three crop tools viaRenderFixedCropAsync→ViewAccount.Of, andBoundContactSheetToolviaViewAccount.Ink(img)on its own (it carries no frame account). No image-bearing answer slips through without stating its ink.Math.Max(1, sampled)replacing the deadsampled == 0branch. Every path renders at least one pixel, so the old guard was a branch no test could light — the replacement keeps the safety and kills the dead code. Sharp~ ♡· via <provider>on the round header (routing is per-request, so neighbours can differ),· N image(s) (size)on each tool line only when images actually rode along (absence is the positive signal), and theByteshelper that shows 40 B rather than rounding it to "0 KiB" — because an image that arrived at 40 bytes arrived broken, and that is worth seeing. All three edge cases driven straight throughAgentTranscript.Renderwith hand-built details.< 1boundary on the ink sentence. InkPercent=0 → "nearly all paper, not a delivery failure"; ≥1 → "not blank, lost on the way." Both arms tested, and the second test pins that an empty view must not cry delivery (DoesNotContain("lost on the way")) — because every white margin becoming a suspected fault would be worse than the disease.8af1231→b975727, verified as merged OpenRouter.Net main. Merge order honoured.Test coverage~
All new branches are genuinely exercised — not tautologies:
AgentToolImage100%,AgentTranscript95.2%/87.5%,SkiaPageImageRenderer97.6%/88.2%,PageImageAccess94.5%/77.7%. TheInkmethod itself is fully covered (line + both alpha/luma branches hit by the grey-page and white-page integration tests).Local verification (CI coverage bot #5047 covers
5a4bfc2, one commit before head41a6d6fwhich is a pure submodule repoint — zero production/test delta): build 0 warnings / 0 errors, 959/959 pass (Domain 101, UseCases 444, Integration 187, BlazorAdapter 227 — matches PR body).This is a thorough, honest piece of work. The loss stays undiagnosed, but the next agent that sees nothing will know to say so instead of guessing. Fufu~ ♡
Automated review by Jibril · 2026-07-29
CI/CD: coverage bot #5047 for
5a4bfc2(head41a6d6fis submodule-repoint-only, 1 file, zero code drift) · Local checks: build 0/0, 959/959 pass