feat(annotator): match Python mask-to-geometry semantics in TypeScript - #873
Merged
Merged
Conversation
`_pointed_at` chose among the pieces a prompt's points land in with `max` over a `set` of labels. A label is a run index, so when two pointed pieces have exactly the same area the answer came from CPython's hash-table bucket order — reproducible, but not a rule, and not one a second implementation of this pipeline could hold to. It is now the `(-size, label)` order `components` already applies to every piece behind the head: biggest first, ties by the piece whose earliest run comes first.
simplify.ts claimed Math.sqrt and Python's ** 0.5 are the same correctly-rounded operation. They are not: CPython routes ** 0.5 through libm pow, which disagrees with a correctly-rounded square root by one ulp on roughly 0.088% of random doubles (measured). What the parity actually depends on is that every expression is in the same order as the Python it mirrors — that stays true and is now the stated reason. The ulp difference has never reached a comparison boundary here: it is absorbed by the truncation in closingRadius, and the distance comparisons it feeds are decided by margins far wider than an ulp. Noted in closingRadius's own doc comment too. Making Python call math.sqrt would be a second Python semantic change and is out of scope. Also documents the equal-area tie-break inference.md was missing: two pieces of the same size answer with the one whose earliest run comes first.
shapesFromMask trusted BinaryMask.mask to be width * height bytes but never checked it. A transposed width/height, or a short buffer, answers with a silently wrong bbox instead of an error — demonstrated with a 4x4 mask carrying only 8 bytes. Because RawSegmentation is only structurally assignable to BinaryMask, a caller can get this wrong without any type error catching it. Add a guard at the top of shapesFromMask that throws when the byte count doesn't match. Python's Mask can't be malformed this way, so this restores parity of guarantees rather than adding a new rule; every existing empty/degenerate case still answers [] rather than throwing. Also fixes closingRadius's own test, which compared the function to itself on identical input and, despite its name, never held the piece's size fixed while the frame changed. It now checks the actual claim: the same 64x64 piece gives the same radius in a 64x64 and a 200x200 canvas.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase D of the browser-inference arc.
@visionset/annotatornow reproduces the kernel's complete mask→geometry pipeline in TypeScript, and a golden fixture proves the two agree exactly. Nothing is wired into the editor.Python remains authoritative
src/visionset/inference/masks.pydecides what a mask means. Where the two disagree, the TypeScript is wrong. Two Python stabilizations make previously implementation-dependent choices explicit:_pointed_atchose among the pieces a prompt's points land in withmax()over a set of integer labels. A label is a run index, so on an exact area tie the answer came from CPython's hash-table bucket order — reproducible in CPython, but not a rule. On a real mask with two five-pixel components labelled 3 and 8, the set iterates[8, 3](because8 & 7 == 0) andmaxanswered with the later piece; any natural port answers with the earlier one. It is now the deterministic(-size, label)ordercomponents()already applies to every piece behind the head: biggest first, ties by the piece whose earliest run comes first.Nearest-component selection now compares
_squared_gapvalues instead of square roots. Square root is monotonic, so this preserves the intended nearest ordering while avoiding floating-pointsqrt/powrounding ties that could otherwise fall through to reading order.The port
Ported line for line:
spans,bbox_from,runs,_adjacent,_components,_squared_gap,_areas,_pointed_at,_cropped,components,_bits,_grown,_shrunk,_joined,_met,closing_radius,filled,_set_bits,_turned,outline,smoothed,contour,_shifted,_boxed,target_kind,_union_of,shapes_from.simplified()andpolygonAt()are reused, not duplicated. They already live incore/geometry/simplify.tsand are already held to Python bytests/fixtures/simplification.json. There is exactly one Douglas-Peucker in the repository, and this branch proves the contour fed into it is identical too — which is what closes the loop that fixture left open.New directory
frontend/annotator/src/core/mask/, inside the headless boundary: no React, no DOM, no new dependency. Morphological closing runs overBigIntrow bitsets, because Python'sintand JavaScript'sBigIntare the same arbitrary-precision two's-complement under&,|,~,<<and>>— that is what makes it a port rather than a rewrite.Semantics worth naming
polygonAt(tolerance). A box takes the union of the extents of every surviving piece, with no closing, no contour, and no dependence on the tolerance. A mask arriving in several pieces is usually one object seen around an occlusion, so the box spans them while the polygon traces one. The fixture carries a case where the two visibly disagree.["bbox","polygon"]and["polygon","bbox"]are separate fixture keys asserted to give identical answers.[], never a throw.Parity
tests/fixtures/mask_geometry.jsonis written by Python (scripts/export_mask_geometry_fixtures.py), kept current bytests/inference/test_mask_geometry_fixture.py, and consumed independently byfrontend/annotator/src/core/mask/maskGeometry.test.ts. Two gates sharing data and no toolchain — the frontend job installs no Python. 20 cases, 824 assertions, exact equality, no epsilon anywhere. The added squared-distance near-tie case fixes the ordering at the floating-point boundary.Each pipeline stage is asserted separately, so a failure names the stage rather than only the answer:
shapes_fromThe rounding trap
_pointed_atcalls Python'sround(), which is half-to-even;Math.roundis half-up and answers-0for-0.5. A naive port selects a different component for a half-pixel click while every integer-coordinate test still passes.pythonRoundimplements the real rule.The fixture's half-pixel case needs two points to catch it. A single half-pixel click provably cannot discriminate — the two candidate pixels are either one run or 8-connected, so they share a component, and a rounding that lands on nothing falls back to the piece the click is nearly inside anyway. With a second point the largest-wins rule decides: Python answers the 12 px piece at
x=0,Math.roundanswers the 4 px piece atx=8.Verification
All 18 of the planned mutations were applied, observed red, and reverted. Notably the closing-radius cap was initially caught only by a constants assertion — every case had an uncapped radius of 1, so a port that dropped
Math.minentirely would have passed all 742 assertions. A 320×320 case was added; that mutation now fails exactly one behavioural assertion.A reviewer additionally ran a differential fuzzer of 1,198 Python-authoritative cases — random noise, swiss-cheese, checkerboards, near-touching blob pairs at gaps 1–13, 2,072-point organic boundaries, negative and out-of-frame prompts, and every closing radius 0–6 — with zero divergences.
The
browser models (EfficientSAM-Ti export, parity, real browser)check completed successfully but was path-skipped for this PR; it did not re-run the heavy model-export and real-browser path.Performance
Node 24, best-of-five after warm-up, one machine. Not a benchmark suite.
The worst case is under a third of Phase C's ~100 ms warm decoder.
Scope
No editor integration:
AnnotationPage,SuggestPanel,OssSession,browserPort.ts,AssetImageand the browser runtime are untouched. No "This device" target, noSuggestionExecutor, noSuggestionOut, no model artifacts — the tests here need none. No dependency is introduced between@visionset/annotatorand@visionset/browser-inferencein either direction;BinaryMaskis named so Phase C'sRawSegmentationis structurally assignable with no copy and no import, and a host composes the two later. Phase E and Phase F are not started.Follow-up
simplify.tsclaimedMath.sqrtand Python's** 0.5were the same correctly-rounded operation. They are not — CPython routes** 0.5through libmpow, and ~0.09% of doubles differ by one ulp. No output difference was found in simplification (the gap is absorbed by truncation in the closing radius across 200,000 synthetic reductions and all real contours), so this branch corrects the claim only. The mask pipeline avoids the analogous nearest-selection boundary by comparing squared distances; changing Python's simplification calculation tomath.sqrtremains a separate semantic change.