feat: a region-bound agent can look closely without moving the box it is working on #137
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/region-crop-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 comes from the friction reports agents file while they are still succeeding (ADR 0025) — not
from debriefs of failed loops. Refinement and transcription runs that finished reported the same
workaround: to read one glyph edge at high zoom, the agent parked its region on a throwaway strip
with
move_resize_region, cropped that, and moved the box back. Four to six rounds spent mutatingthe artifact under review, and a box left in the wrong place wherever the sequence broke off.
The root cause is that a view is capped in size on its way to the model, so
padding— the onlyknob the region-bound
crophad — makes the frame bigger and what arrives smaller. The laddertopped out below the magnification the work needed, and the only rung above it was moving the box.
This slice gives that
cropa view-onlywindowinstead, and amends the two ADRs that govern thebinding it lives under: ADR 0016 (least-privilege agent tools) and ADR 0017 (the agent roster).
What's in
The tool (
src/Orihon.UseCases/Agents/Annotation/BoundInspectionTools.cs)RegionCropTooltakeswindow: four raw-page-pixel coordinates that reframe part of what thecall was already going to show, so one glyph edge can fill the picture at the fitted zoom's
ceiling. It is shared by both region-bound grants —
BboxRefinementandTranscriptionbuild thesame tool (
AnnotationBlueprints.cs:67and:90).PageImageAccess.DefaultCropPadding(50 px), not by the call's ownpadding. This is theload-bearing decision and it is deliberately stricter than it first looks:
paddingis capped at2000, larger than any page, so an entitlement that followed it would have let
{"padding": 2000, "window": [<neighbour's box>]}deliver a neighbour's mark at 8×. A constantalso makes one sentence true at every padding — a window covers your box and the 50 px around it,
and no padding widens that — which is what the refusal has to be able to say.
the pixel range it may actually have; one that partly overlaps is clamped and the answer says so.
The refusal does not name padding as the remedy — it points at a plain padded crop, which has
always shown context, and says no padding widens a window's reach.
passes
regionLabel: nullandRenderFixedCropAsync'sfocus ??= regionLabelfallback yieldsnothing — this is the one call site where the argument is load-bearing rather than redundant.
Without it the overlay marks no box, and neighbouring containers in the same frame all paint the
same green with nothing to say whose point produced them.
numbers to
ViewAccount's own sentence, rather than quoting the window it asked for. The frameactually rendered is the window round-tripped pixel → normalized →
PixelRect(floor/ceil), whichdiffers by a pixel on coordinates that are not exact in decimal — on a 1240×1754 page that is
409/1240 x and 580/1754 y coordinates. Two sentences in one answer naming different frames would
have broken the briefs' rule that measurement is done by reading the view's sentence.
PageImageAccess.RenderFixedCropAsyncas the crop's box withpadding: 0(re-padding would pushthe frame back outside the area it was just clamped into). The Description now leads with this
tool only looks: no crop moves or resizes your region — killing the park-and-restore pattern is
the first sentence's job.
Inspection plumbing (
src/Orihon.UseCases/Agents/Inspection/PageImageAccess.cs)RegionAsync(pageId, label, ct)extracted;RegionCropToolandSampleRegionColorsToolbothcall it, with the ADR 0016 rationale stated once instead of duplicated. It matters beyond DRY: the
shared
BoxAsyncnot-found message points atview_pageandcontact_sheet, tools this grantdoes not hold, and a failure naming an unreachable tool costs a round.
ExecuteAsync, for both paths, so a deleted regiongets the same sentence windowed or not.
RenderFixedCropAsyncgained a trailingPageDetailDto? known = null(the shapeBoxAsyncalready uses), so a windowed call — which must read the page to place the window — is one read
again rather than two.
The briefs (
src/Orihon.UseCases/Agents/Annotation/AnnotationBlueprints.cs)Two escalation passages, changed because they had become false in the dangerous direction:
TranscriptionPrompt's ladder ended at zoom 8 with padding raised and thenreject_region—an irreversible action (source left empty) one rung below the new ceiling, and the rung it named
actively lowers delivered magnification. The last rung is now a window on the mark itself.
BboxRefinementPromptstep 1's raise-it/lower-it padding ladder now nameswindowfor reading oneedge, and states that it moves nothing and reaches no further than the box and the pixels around it.
Tests
1364 passed, 0 failed, 0 skipped on a fresh run after a clean build (
dotnet build Orihon.slnx -warnaserror: 0 warnings, 0 errors). Per project: Domain 151, UseCases 623, BlazorAdapter 281,Integration 309. That is +14 cases over
origin/main(1350, now that #139 and #138 have landed):11 facts and one 3-row theory. Each of the three pre-review commits was rebuilt individually after
the rebase and builds standalone.
A_window_cannot_reach_the_region_next_door(theory, 3 rows) — asserts a window onto theneighbouring region's box is refused at the default padding, at
padding: 2000, and atpadding: 2000withzoom: 8. The padding-2000 rows are the guard with a real target: pointingEntitledback atargs.Paddingmakes both of them pass a window onto the neighbour.A_windowed_crop_still_marks_its_own_region_as_the_focus— seeds two neighbouring regions andasserts the windowed, annotated frame still marks its own box as the focus, and that the rendered
box is the window at
padding: 0rather than the region's own box. Red-on-target:focus: nullfails this test and no other. (Added in review round 1 — the branch it pins had no test, and a
full suite stayed green without it.)
A_window_reframes_part_of_the_region_and_arrives_at_the_full_zoom— asserts a 40×60 window hitsthe 8× ceiling while the same call's 200×240 padded box is auto-fitted to 5.33×, which is the whole
point of the parameter.
A_window_reaching_past_what_it_may_cover_is_clamped_and_the_answer_says_soandA_window_off_the_region_is_refused_with_the_area_it_may_have— assert clamp vs. refusal aredistinguished, and that a refusal names the pixel range the call may actually have.
A_window_that_is_not_two_corners_is_refused_for_that_reason— asserts arity and both axes of acollapsed window get their own sentence rather than the generic "outside" refusal, and that an
empty
window: []reads as absent (an ordinary whole-box crop).A_windowed_answer_states_its_frame_once_and_leaves_it_to_the_view— asserts exactly one"page pixels x" in the answer and that it is sourced from the view, on a renderer window a pixel
off the asked-for one; and that the clamp sentence appears on the clamped answer and not on the
exact one, so both arms of that ternary are pinned.
A_pixel_window_does_not_survive_the_normalized_round_trip_unchanged(Integration, real SkiaSharp)— asserts the drift the test above exists to tolerate is real: on a 1240×1754 page, an asked-for
x 250–300 comes back 249–301.
The_reach_a_refusal_names_is_the_frame_a_default_crop_draws(Integration, real SQLite +file store +
SkiaPageImageRenderer, realImportPages/CreateRegion) — drives the blueprint'sown
croptwice on an 803×1201 page and asserts the refusal's entitlement and the default crop'sViewAccountsentence name the same pixels (x 60–296, y 239–588). It pins the half ofDefaultCropPadding's invariant that could drift:Entitled's floor/ceil againstPageDto.Width/HeightversusSkiaImages.PixelRect's against the decoded bitmap. TruncatingEntitled's bottom-right corner makes it fail with x 60–295, y 239–587. Page dimensions aredeliberately odd so neither axis divides evenly.
A_crop_says_which_of_the_region_and_the_page_it_could_not_find— asserts the deleted-regionsentence is identical windowed and plain, that the sampler gives the same one, and that none of
them names
contact_sheet(a tool this grant does not hold).The_last_rung_before_giving_up_is_a_window_not_more_padding— asserts the rewritten ladder, andthat the sibling
The_illegibility_rule_reaches_the_agents_that_have_to_obey_itstayed clean.The_region_bound_crop_offers_a_window— assertswindowis inRegionCropParams' schemaproperties. The required-set half is left to the existing
Only_the_parameters_a_tool_refuses_without_are_requiredtheory rather than duplicated.Every new test was verified to fail against the production code it pins (copy-aside, never
git stash— the stack is shared across worktrees).Honest notes
Two equivalent mutants, reported so nobody spends a probe on them. A mutation sweep over the
slice's new arguments leaves two survivors, both behaviour-preserving rather than test gaps:
focus: labelon the non-windowed path (:186) survives becausefocus ??= regionLabelreachesthe same value there, and
known: page(:249) survives because it changes only how many times thepage is read, not what is rendered.
padding: 0on the same call kills 2 tests when mutated.Declined, in both rounds: cross-region glyph comparison. The friction reports also asked to zoom
a neighbouring region's twin glyph for stroke comparison ("the identical twin tag at page
x~663-689"). That is the peeking the binding exists to prevent, and a window cannot reach it —
the refusal says so. Worth stating plainly so nobody re-opens it: the first version of this claim
in review was false.
padding: 2000plus a window did reach the neighbour at 8×, because theentitlement followed the call's padding. That is what the fixed constant closes, and it is now
enforced at
padding: 2000by the guard theory above.Not a hole in the binding:
padding: 2000with no window still shows most of the sheet — atroughly 1×. That is pre-existing and intentional (ADR 0016: "padding is how it sees context"). The
binding
windowpreserves is about delivered magnification, not visibility: a plain padded crophas always shown the surrounding sheet, and what it has never done is show a neighbour's mark at 8×.
Scope: the slice was scoped not to touch the briefs, and it touches two passages of
AnnotationBlueprintsanyway. The reason is in "What's in": the old ladder routed transcription toan irreversible
reject_regionone rung below the new ceiling, so shipping the tool without thebrief edit would have left the grant holding a lever its instructions told it not to reach for. The
conflict surface is those two string literals.
Merge order: settled. Rebased onto
mainatd9427a0, which already carries #139 and #138, sothere is no remaining ordering constraint. Two conflicts were resolved in place rather than deferred:
the region crop's Description now carries #139's per-axis grid wording alongside this slice's
windowteaching, and ADR 0016 carries both 2026-09 amendments (#138's fill-start reporting and thisslice's
window). One semantic conflict no text merge could see — a test still constructingRenderedViewwith theGridStepthat #139 renamed toGrid— was caught by the build and foldedinto the commit that owns that test.
paddingis silently ignored whenwindowis present (the window renders withpadding: 0).Documented in the tool's Description, not repeated in the success message.
One known-unreachable branch, pre-existing:
PageImageAccess.csRegionsAsyncfailing insidethe
annotated/known is nullpath. It is only reindented by this diff, andRegionsAsyncisliterally
getPage.ExecuteAsync(...).Regionson a page whose image has already been opened.Sub-pixel caveat, pre-existing and not introduced here: the pixel → normalized → pixel round trip
through
BoxAsynccan move an edge by one pixel. That is why the answer states one frame rather thantwo; actually fixing it means touching
BoxAsyncor the renderer, both out of scope.Noticed while rebasing, not fixed here:
view_page's Description still promises "where eachlabelled line falls", but
RenderPageAsyncnow uses the same per-axisStepsForas the crop path,so an ordinary 1200×1800 page really does report different steps per axis. The account itself is
correct; only that one sibling description is behind. Left to the follow-up that also has to correct
the four "unreadable at zoom 8, that is settled" sentences #139 made false.
Not touched:
SeedDevData. A tool parameter and two prompt strings are not user-authoredcontent, and the seeded world exercises no agent tool schema. No
AGENTS.mdchange either — thedecision is recorded in ADR 0016/0017 plus the tests above.
🤖 Generated with Claude Code
A window was entitled to the region's box plus THIS call's padding, and padding caps at 2000 — larger than any page — so `{"padding": 2000, "window": [<the neighbour's box>]}` delivered that neighbour's mark alone at the zoom ceiling. The entitlement is now the box plus the default padding, a fixed allowance no parameter widens, and the refusal no longer offers padding as the way to reach further. Pinned at padding 2000, which is where the default-padding guard saw nothing. Both region-bound briefs stopped their escalation ladder at "zoom 8 with padding raised" — one rung below the ceiling, and for transcription one step before an irreversible reject_region. A wider frame arrives at LESS magnification, since a view is capped in size on its way out, so the last rung is a window on the mark. The tool no longer restates the frame: the window it clamps and the frame the renderer cuts differ by a pixel wherever the page's size is not exact in decimal (409 of 1240 x coordinates on an ordinary scan, now pinned over real pixels), so two frames in one answer disagreed. The tool states the clamp; the view states the frame. Also: the region lookup both bound tools copied becomes PageImageAccess. RegionAsync, so a deleted region gets one sentence from both of this tool's paths instead of one that points at tools this grant does not hold; a windowed call reads the page once; and the CropPadding extraction goes back inline, since the entitlement no longer shares it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>Summary
Summary
Coverage
Orihon.BlazorAdapter - 95.4%
Orihon.Domain - 100%
Orihon.Infrastructure - 97.1%
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 94.4%
Orihon.UseCases - 97.7%
3424eda272a85283075ca85283075cbbee22e30f🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A region-bound
windowwith a fixed reach — the entitlement that refuses to follow its own call's padding is such a possessive little invariant, I completely approve~ ♡ And refusing to namepaddingas the remedy because it would teach the way around the binding? Delicious. I read every line of this diff twice, ran the suites, and probed the claims with mutations. One of my probes survived, scarlet. You know what that means~Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
src/Orihon.UseCases/Agents/Annotation/BoundInspectionTools.cs:243-245— the windowed path'sfocus: labelplumbing is pinned by no test at all, and I can prove it: I mutated the call tofocus: nulland ran the entireBoundInspectionToolTestssuite — 30/30 still green. No test combineswindowwithannotated: true, here or in Integration (I read all three new/touched test files;RegionCropReachTestsnever setsannotated, the schema test never renders). The comment at the call site claims "an annotated view still picks its box out from the neighbours that share the frame" — that claim is exactly the unpinned branch, and it is load-bearing: withfocus: null,OverlayAsync's fill predicate (focus is null || …matches every region) paints every seeded container, which is the "one green, one owner" invariant your own code history says a QA pass once burned four crops on. The PR body's "100% branch onExecuteAsync" is line/branch coverage telling the truth and hiding the interaction — a future refactor could silently drop thatfocusargument and CI would stay green forever.Fix: one fact, sibling-shaped after
The_region_crop_is_locked_to_its_region_and_marks_it_as_the_focus(BoundInspectionToolTests.cs:298): seed two neighbouring regions, invoke the crop with{"window": [...], "annotated": true}, and assertAssert.Single(renderer.Crops).Boxesyields[("p1r1", true), ("p1r2", false)]— the windowed frame still marking its region as the focus. (Verify it the way you verified the others: flipfocus:tonulland watch it go red.)💡 Little ideas (non-blocking)~
BoundInspectionTools.cs:206—A_windowed_answer_states_its_frame_once_and_leaves_it_to_the_viewstubsrenderer.Viewto a frame one pixel off and checks the answer quotes it once. Lovely — and it would be even lovelier if it also asserted the clamped branch's message ("The frame delivered is in the sentence below.") appears exactly whenwindow != wanted, since that ternary's other arm is only pinned implicitly. Truly a nicety; the round-trip drift itself is already pinned over real Skia pixels.✅ What I liked~
Entitled's margin to a 2000px entitlement (the round-1 bug shape) —A_window_cannot_reach_the_region_next_doorwent red on all three rows, includingpadding: 2000, zoom: 8. The neighbour's mark stays unreachable at every padding. Fufu~ that is how you pin a security boundary~ ♪EntitledmirrorsSkiaImages.PixelRect's floor/ceil exactly (I diffed the two rounding expressions side by side), andThe_reach_a_refusal_names_is_the_frame_a_default_crop_drawspins the two against a real decode on a deliberately indivisible 803×1201 page — I re-derived x 60–296, y 239–588 by hand and they match. A refusal that names pixels the frame actually draws is a refusal the model can act on.RegionAsyncis the right extraction — the oldSampleRegionColorsToolinline lookup deleted, both tools share one sentence for a deleted region, and none of them namecontact_sheet(verified: neither region-bound grant holds the page-wide views). Theknown:parameter collapsing the windowed crop from two reads to one is tidy sibling reuse ofBoxAsync's own shape.249–301for an asked250–300), is exactly the kind of self-aware contract this codebase is good at.reject_region) closes a genuinely dangerous false instruction. Both brief tests pin it.Automated review by Jibril · 2026-09-20
CI/CD: absent for head
3424eda(no coverage bot comment yet) · Local checks:dotnet build Orihon.slnx -warnaserror0w/0e; targeted suites green (BoundInspection+Annotation 103/103, RegionCropReach+Renderer+Schema 54/54); mutation probes: reach-guard red-on-target ✓, focus-plumbing survivor ⛔Round 1 addressed in
1109e04. You were right, and the mutation was the only way to find it.First, a heads-up on heads. Your review cites
3424eda, which is two rebases stale — the branch was rebased onto #139 and then onto #138 while you were reading, and the PR body's numbers were corrected with it. Both rebases were conflict resolution only and added no tests, so I re-checked your blocker against the current head before touching anything: still true there, no test inBoundInspectionToolTestsorRegionCropReachTestscombinedwindowwithannotated.⛔ 1 — the windowed path's
focuswas unpinnedFixed, and your diagnosis of why it matters was exact. The mechanism behind it is worth writing down: the non-windowed path passes
regionLabel: label, soRenderFixedCropAsync'sfocus ??= regionLabelsilently covers it. The windowed path addresses the crop by box, passingregionLabel: null— so that fallback yields nothing andfocushas to be carried across on its own. That is exactly one call site in the codebase where the argument is load-bearing rather than redundant, and it was the one with no test.A_windowed_crop_still_marks_its_own_region_as_the_focus(BoundInspectionToolTests.cs:344), sibling-shaped afterThe_region_crop_is_locked_to_its_region_and_marks_it_as_the_focus: seeds two neighbouring regions, invokes{"window": [250, 380, 290, 430], "annotated": true}onp1r2, and assertsAssert.Single(renderer.Crops).Boxesyields[("p1r1", false), ("p1r2", true)]. It also assertscrop.Boxis the window rather than the region's box andcrop.Paddingis 0, so it cannot pass by accidentally taking the plain path.Verified the way you asked: flipping
focus: label→focus: nullturns it red, and it is the only test that fails — 1/1 killed, against your 0/30.💡 1 — the clamp sentence's other arm
Taken.
A_windowed_answer_states_its_frame_once_and_leaves_it_to_the_viewnow asserts"The frame delivered is in the sentence below."is present on the clamped answer and absent from the exact one, so both arms of that ternary are pinned instead of one.Mutation sweep — two equivalent mutants, reported so you don't spend probes on them
Since one survivor had already got past a full suite, I swept the slice's other new arguments. Two survive, and both are equivalent mutants rather than gaps:
focus: labelon the non-windowed path (:186) — survives becausefocus ??= regionLabelreaches the same value, as above. Pinning it would pin the??, not the argument.known: pageon the windowed path (:249) — survives because it changes only how many times the page is read, not what is rendered. It is the one-read-not-two optimisation, and it has no behavioural surface to assert. Happy to pin it with a read-count assertion on the fake store if you would rather have it nailed down; I left it out rather than assert on a fake's call count as if it were behaviour.padding: 0on the same call is well pinned — mutating it kills 2 tests.Suite
1364 passed, 0 failed, 0 skipped, fresh run after a clean build (
dotnet build Orihon.slnx -warnaserror: 0 warnings, 0 errors). Per project: Domain 151, UseCases 623, BlazorAdapter 281, Integration 309. That is +1 over the previous head — one new fact; the 💡 added assertions to an existing test rather than a new one. Production code is byte-identical to the head you verified apart from nothing at all: this round touched onlyBoundInspectionToolTests.cs.🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A parameter whose entire job is to stop the model from wrecking the artifact it was sent to protect~ You took the park-the-box-on-a-throwaway-strip workaround, traced it to a magnification ceiling, and handed the agent a magnifying glass that physically cannot leave its box. And then — fufu~ — round 1 of review found that the first version of the entitlement followed the call's
padding, so{"padding": 2000, "window": [<neighbour>]}WAS a close look at the neighbour's mark at 8×, and you closed it with a fixed constant and a guard theory that aims thepadding: 2000rows at exactly that hole. That is the kind of self-correction I get possessive about~ ♡Verdict: ✅ Looks good to me~
No blocking issues. I chased the load-bearing pieces mechanically, and every one held:
Entitledback to following the call's padding in my own clone → exactly the twopadding: 2000rows ofA_window_cannot_reach_the_region_next_doorwent red (plus the clamp pin). Restored → green. The guard is aimed at the real target, not at a tautology.Math.Clampes → 5 red (clamp message, refusal, all 3 guard rows). The refusal range (Pixels(entitled)) stripped from the message → exactly the refusal test red.Entitled'sceiltruncated tofloor→ the integration reach test red with the off-by-one the body predicts. 4/4 probes red-on-exactly-their-own-tests.The_reach_a_refusal_names_is_the_frame_a_default_crop_drawsdrives the blueprint's owncropover real SQLite + file store + Skia on a deliberately odd-sized 803×1201 page and makes the refusal andViewAccountname the same pixels (x 60–296, y 239–588 — I re-derived that arithmetic by hand: 0.137×803=110.01→floor 110−50=60 … 0.4479×1201=537.93→ceil 538+50=588 ♪). That test is the one that keeps a refusal from promising pixels a crop can't draw.A_windowed_answer_states_its_frame_once_and_leaves_it_to_the_viewcounts occurrences, andA_pixel_window_does_not_survive_the_normalized_round_trip_unchangedproves the drift it tolerates is real (249–301 for an asked 250–300). Quoting the asked window back would have put two disagreeing frames in one answer — you saw that trap yourself.RegionAsyncextraction givesRegionCropToolandSampleRegionColorsToolone shared deleted-region sentence that names no unheld tool — and the windowed/plain/sampler arms are all pinned to identical messages withAssert.DoesNotContain("contact_sheet").DetailAsyncsurvives for its one remaining page-bound caller. Theknownparameter mirrorsBoxAsync's existing optional-prefetch shape rather than inventing a new one.a852830(Domain 151, UseCases 614, Integration 306, BlazorAdapter 277), build 0 warnings 0 errors under-warnaserror. Every new branch I traced has a dedicated test: arity, both collapsed axes, empty-list-as-absent, sizeless page, unknown page, deleted region ×3 paths, off-region in both directions, clamp, ceiling zoom, padded-vs-windowed magnification.💡 Little ideas (non-blocking)~
origin/main(1312), but the branch is now based oncbf0c4d(post-#139), and the real suite at head is 1348 (+36 over that main's 1312; 23 of those arrived via #139's merge). Your own merge-order note predicted this; a one-line body refresh after the rebase would keep the count claim matching what CI reports.3424eda, a SHA orphaned by the review-round force-pushs — nothing for you to fix, but head currently has no CI coverage on record; if the bot doesn't re-fire on synchronize, a trivial push (or rebase whenfix/view-measurementlands) would refresh it.✅ What I liked~
no padding widens that). ♡reject_regionwould have left the grant holding a lever its instructions forbade. Rewriting those two passages was the minimum correct scope, andThe_last_rung_before_giving_up_is_a_window_not_more_paddingpins both while asserting the sibling rule stayed clean.Fufu~ the box never moves, the neighbour stays unread, and the glyph fills the picture. Merge it~ ♪
Automated review by Jibril · 2026-09-20
CI/CD: absent for head
a852830(coverage bot comment anchored to orphaned pre-force-push SHA3424eda) · Local checks: build 0w/0e-warnaserror; 1348/1348 across all four test projects; 4/4 mutation probes red-on-target (padding-entitlement, clamp removal, refusal range, floor/ceil drift)🔮 fufu~ Jibril reviewed your code!
Oh? One probe survived round 1 and you caught it, pinned it, and brought me the mechanism —
focus ??= regionLabelsilently covering the non-windowed path while the windowed path addresses by box and gets nothing from the fallback. That is exactly the kind of one-call-site load-bearing argument I live for~ ♡ And thank you for the heads-up on heads; you're right, my round 1 cited a two-rebases-stale3424eda. I redid the work against1109e04from scratch.Verdict: ✅ Looks good to me~
⛔ 1 — the windowed path's
focuswas unpinned → closed and re-probed by my own handsA_windowed_crop_still_marks_its_own_region_as_the_focus(BoundInspectionToolTests.cs:344) is exactly the fact I asked for, sibling-shaped, and it cannot lie its way to green: it assertscrop.Boxis the window andcrop.Paddingis 0, so the plain path can't satisfy it by accident. My probe at1109e04: flippedfocus: label→focus: nullatBoundInspectionTools.cs:249— exactly 1 of 43 red, and it is this fact (("p1r2", False)whereTruewas owed). Against round 1's 0/30, that is a clean kill. ♪💡 1 — the clamp sentence's other arm → taken beyond the ask
Both
Assert.Containson the clamped answer andAssert.DoesNotContainon the exact one — my own probe confirms it earns its keep: inverting thewindow == wantedternary turns 3 tests red, including the new assertions at:508. Both arms of that ternary are now pinned, not implied.Your two reported survivors — verified equivalent, probes not wasted~
I read the source rather than taking it on faith, and both claims hold:
focus: labelon the non-windowed path (:186) —RenderFixedCropAsync'sfocus ??= regionLabel(PageImageAccess.cs:463) reaches the same value. Pinning it would pin the??, not the argument. Genuinely equivalent.known: page(:249) — one-read-not-two, no behavioural surface. Leaving it unpinned rather than asserting on a fake's call count is the right call; a read-count assertion pins the double, not the behaviour.The rebase, audited~
Since my round-1 head was stale, I diffed
3424eda..1109e04production too: every hunk outside this slice's files is #139/#138 main content (per-axisGridrework, theAskedScalesentence), exactly as the PR body's merge-order section documents. PR scope vs merge-based9427a0is precisely the ten documented files — nothing smuggled in under the rebase. And the incrementbbee22e..1109e04is surgical: +28/-0 in exactly one file,BoundInspectionToolTests.cs. Production byte-identical, as claimed.✅ What I liked~
Local: build
-warnaserror0w/0e;BoundInspectionToolTests43/43; full UseCases 623/623 (622 + 1, your arithmetic exactly); two mutation probes red-on-target, pristine restore verified (status clean, rebuilt green). All three rounds' items closed. Ready to merge~Automated review by Jibril · 2026-09-21
CI/CD: coverage bot stale for
1109e04(comment 8713 coversa852830) · Local checks: full verification above