feat: cleaning 6/12 — compare and measure: gridded views split not downscaled, ink hints, PNG pass-through #172
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/cleaning-compare-and-measure"
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 slice 6 of Phase 5 in
PROJECT_PLAN.md: ADR 0027'scompareandcropviews andmeasure_ink. The agent judges cleaning through these. The slice also adds the pass-through that sends a cleaning view to the model as its own PNG bytes. Nothing calls them yet: slice 7's use cases record the views and build the tools on them.What's in
Views:
ICleaningViews, withRasterCleaningViews.A compare shows the original first, then each source, at one frame and one scale: 1× or the 3× preset.
PagePatch: image bytes plus the page rect they cover. So a fill patch over T + 6, a raw over its call's window or the plate all fit. A source at another size, such as a raw at canvas size, is resized with Pillow-parity Lanczos.The page-pixel grid is always drawn. It tints every block's first image column and row on a multiple of the step: 20 page pixels at 3×, 50 at 1×, from
cleaner.py'sview().Options:
Uncovered pixels are checkered. No text is drawn, so no pixel depends on a font.
Every image fits 2048 px a side and 4 MiB as PNG, and nothing is downscaled. The class doc gives the split order:
The result states exactly what was delivered, which slice 7 records and computes the survey's coverage and accept's rule from:
One sentence states the frame and the scale.
The survey's crop is the same render for one page, with the grid on or off.
CleaningFrames.Aroundports_crop_boxas a region's default frame, widened by its suggested box + 12 asview()does.measure_ink:ICleaningMetrics, withRasterCleaningMetrics. Its hints never gate.cleaner.py's_metrics,outline_guard,_edge_inkandghost_scoreare ported with numpy's float32 arithmetic: ink left (n/a over textured ground), change, flatness, seam, outline lost and ghost.InkHints), andAnyHintsays whether any fired, which later decides whethercompareadds the raw.Pass-through:
AgentToolImagegainsPassThrough, which only the cleaning views set. The adapter sends a flagged image as its own PNG and everything else through the JPEG re-encode, as before.Shared code:
CleaningViewOptions;NumpyMath's pairwise sum is now generic over float64 and float32, so the float32 seam reuses the summation the oracle already pins;SkiaPixelDigest's grey-PNG test is now internal.Docs: ADR 0027 now says every cleaning view passes through, at 1× as at 3×.
Tests
2,398 green and 2 skipped, against 2,263 on main (+135). It was 2,396 at the first review, +2 in the review round:
Every project ran separately under the 60 s cap, and the build has 0 warnings. What they pin:
Delivery. The image the adapter sends for a zoomed view has exactly frame × scale pixels, as its stated scale says. An annotation crop and a reading-size view still arrive as JPEG.
Views:
Pixel assertions sample only coordinates the layout guarantees.
Metrics parity.
Cleaning/oracle/gen_metrics_oracle.pyimports the lab'scleaner.pyand writes ADR 0027's additions beside it. It covers 50 cases: 22 drawn pages and 28 pages that sit on a limit. These include the registration reach and slack, the structure step at 64 and 65, and the ghost guard either side of 1e-6. A rerun reproducesmetrics.jsonbyte for byte.The options' defaults and the hints' thresholds are each pinned.
Mutation. Two independent reviews, mutation coverage and contract, then 51 more mutants on the fixes and 5 in the review round, each killed by its intended test.
Browser-verified
Nothing to verify in a browser: this slice has no UI, no seed change and no caller.
Honest notes
cv2.phaseCorrelate. The raster layer has no FFT, and parity must be exact. Its thresholds and the structure hint's are uncalibrated: a median of 3 px or more, 24 px, edge level 64 and 10 % slack. Real use will tell.🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 95.8%
Orihon.Domain - 100%
Orihon.Infrastructure - 97.6%
n
ration
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 94.6%
Orihon.UseCases - 97.7%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A 3,335-line slice with a 533-line parity oracle that imports the lab's own
cleaner.pyand freezes its arithmetic to the bit? A pixel-exact compare renderer that splits instead of downscales, a full-blown tile-registration field with cost-slack shortest-shift search, and a PNG pass-through so the judged bytes ARE the delivered bytes? ♡ I read every line of this diff twice, and then I read the oracle generator a third time just for fun~ This is exactly the kind of slice I adore reviewing.Verdict: ✅ Looks good to me~
No blocking findings. Not one. I tried, fufu~ — here is what I attacked and what held:
ViewTiling.Split→Capacity→Emit's recursive halving: I traced the byte-cap recursion (sources split first, then frame halves), the frame partition (Partwith(long)length * index / parts— no overflow), and the refuse-when-one-pixel-cannot-fit path.CleaningViewTestspins every arrangement: 6-part split, 8+2 source paging, exact-cap boundary (MaxBytes: lenpasses,len-1splits), 681-px tile boundary at the side cap. The recursion terminates and the caps hold.Overrefuses anything but exactly the call's window;Insiderefuses a result past the page; T must be a box insideBefore. Every refusal string asserted verbatim inCleaningMetricsTests, including the off-window-inside-the-page case (a strict!=, not an easy accidental "contains").(255·165 + level·90 + 127)/255asserted per channel; theOffsetsign convention (subtract) is used consistently inLocal/Stretch/Pasted— I checked each againstPixelBox.Offset's definition; overlay draw order (footprints → model rects → flags → T on top) pinned including the shared-edge case; diff gain and stretch rounding (half-up,AwayFromZero) both pinned at their boundaries.AgentToolImage.PassThroughdefaults false, onlyPainter.Paintsets it, andCleaningViewDeliveryTestsdecodes the actual data-URI bytes off the wire and asserts they equal the view's PNG — while annotation crops and reading-size views are asserted to still go through JPEG. Exactly the contract ADR 0027 now states, at 1× as at 3×.gen_metrics_oracle.pymirrors the C# line for line:edges(right/lower neighbour, none past the edge),structure(T+2 zone, cross dilation, both directions),registration(keep-out, reach-clear edge margin, min-edges, slack100·c ≤ (100+slack)·least, median via numpy),hints(same order, same>/>=split — median at the threshold fires, ink-left at it does not).MetricsParityTestsasserts exact doubles on all 52 cases including the registration reach/slack limits, structure at 64/65, and the ghost guard either side of 1e-6.💡 Little ideas (non-blocking)~
Render/Emit— the split logic is the densest code in the slice. It is fully pinned, so this is pure polish: a sentence in the ADR (or the class doc) sketching the split decision tree (frame parts → source groups → byte-cap halves) would spare slice 7's author a re-derivation. You already owe yourself that diagram anyway~costsislong[441]reallocated per square andleastis a full 441-entryMin()even though only shifts within the slack matter. At registration-tile 64 on a 2048-px window this is at most ~1M longs — irrelevant. Truly a nicety, not a need.✅ What I liked~
_metricsand regeneratingmetrics.jsonbyte-for-byte deterministically means "parity" is a fact, not a hope. AndThe_oracles_thresholds_are_the_options_defaultscross-pins the generator's constants against the C# options so the two cannot drift apart silently. That is delicious engineering~ ♪CleaningView/CleaningViewTilestate the frame, scale, grid step, stretch, diff gain, every tile's corner and shown rect, plus a one-sentenceAccount. Slice 7 can compute coverage from facts, not pixel archaeology.The source "raw 2" could not be decoded.) — the agent can act on them.NumpyMath.PairwiseSumgoing generic overIFloatingPointIeee754<T>to reuse the summation the oracle already pins — the right kind of sharing, not copy-paste.Automated review by Jibril · 2026-10-06
CI: green for head SHA
2942ba3c· Mutation probes: 3 run, 0 survived · Local suite: skipped (CI green)Mutation probe log
EdgeMetrics.Shift: slack<=→<EdgeMetrics.StructureChanged:t.Grow(StructureClear)→t.Grow(0)RasterCleaningMetrics.Over:!= bounds→ inside-checkReview feedback (Jibril, PR #172):
d7961e7,f5b5ca5,bc4d1ba💡1
d7961e7,bc4d1ba:RasterCleaningViews' class doc gives the split order as the code takes it: frame parts, then source groups, then byte-cap halves. Three steps it names had no test, so each is now pinned:Each mutant fails only its own test.
💡2
f5b5ca5: the shift costs share one buffer per call.Min()stays global, because which shifts lie within the slack depends on the least itself. Narrowing it fails 4 parity cases.metrics.jsonis unchanged.Tests: 2,398 green and 2 skipped. Integration 1,013 (+2); Domain 144, UseCases 950 and BlazorAdapter 291 unchanged; 0 warnings.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? A re-review gift-wrapped in three little commits — one that answers my doc wish, one that answers my buffer wish, and three pins for the split steps I called untested? Fufu~ you spoil me, scarlet. I read the delta line by line and then went hunting with my mutants again~
Verdict: ✅ Looks good to me~
The shared
costsbuffer (f5b5ca5). The one hazard of hoisting a scratch array is a stale entry leaking between squares — and it can't happen here: the buffer is exactlyside × side, thedy/dxloops rewrite every index beforeMin()reads, andShiftcalls are sequential down the tile loop. The doc comment now says so ("every one rewritten here") — accurate. And keepingMin()global rather than narrowed, because which shifts lie within the slack depends on the least itself — that is the correct reasoning, not the tempting wrong fix. The parity suite confirms: all 52 cases green locally,metrics.jsonuntouched.The split-order doc (
d7961e7). I checked the paragraph clause by clause againstViewTiling.Split/Capacity/HalveandEmit: fewest equal parts via the two arrangements, raster order, lone tile alone, others-before-frame over the byte cap, and the odd-one rule —first = group.Count / 2gives the front halfceil(others / 2)others, which is precisely "the first half taking the odd one." Every sentence is true~The three new pins (
bc4d1ba). Not just present — each one is a real killer, and I proved it (below). Each mutant failed only its own test.One thing to look at, though ♡ — the CI
testjob is red at this head (run 701, job 1, 1m18s;buildgreen). I could not read the job log (actions logs need a login I don't have), so I ran the entire suite locally atbc4d1bainstead: all four projects green — 2,398 passed, 2 skipped, 0 warnings, exactly the PR's claim. The failure does not reproduce from the code at this SHA. I won't name a cause I can't see; someone with log access should glance at it and re-run.✅ What I liked~
Min()where it would change semantics, and verifying the narrowing would fail 4 parity cases before keeping it.Automated review by Jibril · 2026-10-06
CI: test job failed at head
bc4d1ba(log unreadable anonymously; not reproducible locally — full suite green at the same SHA) · Mutation probes: 3 run, 0 survived (+3 earlier probes on unchanged code, all killed — see prior review) · Local suite: all 4 projects green, 2,398 + 2 skippedMutation probe log
ViewTiling.Split: raster order → column-major emissionA_frame_too_big_for_its_scale_splits_into_parts_delivered_at_that_scale, differ at pos 1 (exactly the order pin)ViewTiling.Halve: longer-side guard removed (always cut width)A_tall_frame_over_the_byte_cap_is_halved_across_its_heightEmit: odd other to the second half (1 + first→ floor instead of ceil)An_odd_number_of_sources_parts_with_the_odd_one_first