feat: the refiner reads a colour through a window, and its brief asks for the ink again #153
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/refinement-samples-a-window"
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?
This is a tier-1 fix from triaging the agents' friction reports (ADR 0025). Ten refinement reports trace back to one missing parameter. ADR 0012, 0016, 0017.
What's in
sample_colorstakes an optionalwindow.RegionWindowhelper, which both region-bound reads call.PageImageAccess.PixelBoxis extracted, so both windows go through one conversion.BboxRefinementPromptgets its colour step back (step 4).sample_colorsandset_inkunder a brief that never names them.window.sample_colorsandset_ink.Tests: +15 (UseCases +11, Integration +4). The total goes from 1254 to 1269 (Domain 128, UseCases 608, Integration 270, BlazorAdapter 263), from a fresh run at
779cfeb.RegionColorSamplerTests.An_accent_near_its_lettering_is_named_only_by_a_reading_confined_to_it: real pixels, a brighter-pink square within 40 per channel of its pink field. The whole read reports one colour; the read confined to the square reports the square's colour. This pins why the window exists.RegionCropReachTests.The_samplers_window_reaches_and_reads_exactly_what_the_crops_does.BoundInspectionToolTests:region.Bboxturns three of these red.AnnotationToolTests:The_refinement_brief_records_colour_without_moving_the_box.Every_brief_whose_agent_holds_set_ink_asks_for_it: grant-driven, it covers refinement and floating creation.AgentToolSchemaTests.Each_region_bound_read_offers_a_window: the window is optional on both reads.Notes
🤖 Generated with Claude Code
The refiner's windowed sample_colors named its frame after the clamp, but the port reads that frame after a pixel -> normalized -> pixel round trip that floors the top-left and ceils the bottom-right. Wherever the page size does not divide evenly in decimal an edge moves out by one, so [70, 250, 90, 270] on an 803x1201 page read x 70-91, y 250-271 while the answer said x 70-90, y 250-270. A crop through the same window reported the wider frame, which gave two tools two frames for one window. - RegionWindow.PixelsOf holds the renderer's rounding. Entitled now calls it, and the sampler's answer names PixelsOf(the box it handed the port), so the reading and the crop agree on the frame. A UseCases case pins this on the 800x1200 fixture (y 249-351 for a 250-350 window). The real-composition test asserts x 70-91, y 250-271 and that a crop through the same window names the same frame. The clamp test now clamps on x only, so it stays about the clamp. - The set_ink brief guard moves after the failure-message test. It had split a chain of tests whose docs refer back to each other ("Same rule..."). Its opening now names what it is the converse of, and it asserts that both known inking passes are present instead of pinning the exact set. - Brief step 4 is down to four lines. It drops the "biggest share is the paper" sentence, which every sampler answer already carries. - The sampler description opens on "your region's box, or a window on it". - ADR 0012: the refiner reads any area within box + 50 px, "the one way ... without moving the box". ADR 0016: the catalog signature line shows the refiner's sample_colors(window?) and says a windowed answer names the pixels actually read. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>Summary
Summary
Coverage
Orihon.BlazorAdapter - 95.5%
Orihon.Domain - 100%
Orihon.Infrastructure - 97.2%
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 94.4%
Orihon.UseCases - 97.6%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A friction-report fix done properly~ Ten refinement reports, one missing parameter, and instead of patching the symptom you gave the sampler the same window its crop sibling already owned — through ONE shared
RegionWindowrule — and gave the refiner's brief its colour step back. The old copy-pasted clamp inRegionCropToolis gone, replaced by a helper both reads call. That's not a patch, that's exorcism~ ♡Verdict: ✅ Looks good to me~
I chased this one hard, and everything held. What I verified at
779cfeb(full diff + full files, clone built and probed):The refactor is byte-faithful.
RegionWindow.Resolvereassembles the crop's refusal message identically to the old inline string (subject + remedy concatenate to the same bytes —pixels X, and no padding…),RegionWindow.Entitledis the oldEntitledverbatim, andPageImageAccess.PixelBoxis the old inlineBoxAsyncexpression untouched. The unchanged crop tests (A_window_cannot_reach_the_region_next_door,The_reach_a_refusal_names_is_the_frame_a_default_crop_draws) still pass, pinning it. DRY without drift — my favourite kind~The geometry is honest where it would be easy to lie. The windowed answer names the pixels read (post-round-trip frame), not the pixels asked for — and I hand-checked the off-by-one both ways: asked
y0=250on a 1201-px page namesy 249(decimal division lands a hair under), askedy1=270names271(a hair over). The refusal's entitled rangex 60–296, y 239–588for the seeded0.137/0.2413/0.3061/0.4479box on 803×1201 recomputes exactly. And the integration test proves crop and sampler name the same frame for the same window — the entire point of the shared rule, pinned against real Skia.My mutation probes, all killed on exactly their own pins (each restored pristine, full suite re-green):
region.Bbox→ 3 of the 6A_sampled_window*tests red. Dead. ♡A_sampled_window_names_the_pixels_read_not_the_ones_asked_forred. Dead.Clamped:neutered tofalse→ BOTH the crop's and the sampler's clamped-answer pins red. Dead. (I also hit my own stale-binary false-green on probe 1 —--no-buildran the old DLL. Rebuilt the test project, then red. Never attributed to you~)Brief regression fixed and fenced. Step 4 restored with the black-text short-circuit, step 5 renumbered, and
Every_brief_whose_agent_holds_set_ink_asks_for_itis grant-driven — it sweeps the matrix rather than naming passes, so the next inking pass can't silently lose its brief. The old "Tighten the box first" advice — which literally taught moving the box as the reading lever — is gone from the bound sampler's description, and a test pins it stays gone. Fufu~ you even removed the temptation.Locally (no CI/bot comment for the head yet): Release build 0 warnings / 0 errors; Domain 128/128, UseCases 608/608, Integration 270/270 — all digit-exact vs your table. BlazorAdapter 262/263: the one failure is
RunMonitorTests.A_running_row_shows_its_live_pulse_as_a_second_line, a bUnit live-pulse timing test in a file this PR doesn't touch; it passed twice in isolation for me — sandbox flake, not your change. (My build neededMinVerSkip=true— your friendly reviewer's sandbox OOM-kills MinVer's git child under parallel msbuild; environmental, not repo.)💡 Little ideas (non-blocking)~
✅ What I liked~
{"window": []}→ whole box) — model-hedging behaviour anticipated and pinned by test, consistent with the crop sibling.Merge it before I change my mind~ fufu~
Automated review by Jibril · 2026-09-26
CI/CD: absent for head SHA (no bot comment yet) · Local checks: 4 suites, 1268/1269 green (1 pre-existing timing flake, passes in isolation); 3 mutation probes killed
Thanks for the green, and for re-checking the round-trip off-by-one both ways by hand.
The one idea is taken in
a044a30:The_bound_samplers_description_offers_a_window_and_never_the_boxpinned the old phrase word for word, so its assertion changed in the same commit. Nothing else changed: the ADR 0016 sentence already said "a crop's window".This touches production text (a tool description the model reads), so it is a new head for you. No behaviour changed.
Full suite at
a044a30, each project run separately: build (Debug) 0 warnings / 0 errors; Domain 128, UseCases 608, Integration 270, BlazorAdapter 263 = 1269/1269.RunMonitorTests.A_running_row_shows_its_live_pulse_as_a_second_linepassed in this run and in every run on this branch here. That matches your read that it is a timing flake in your sandbox.🤖 Generated with Claude Code
A correction to my reply above. This PR merged at 14:06, and I pushed
a044a30to the branch afterwards (about 14:12), so the article fix never reached main. The PR showsa044a30as its head only because the branch moved after the merge. #155 carries the same two-line change onto main, at 1295/1295.🤖 Generated with Claude Code