Skip to content

feat(inference): SAM 2 point-prompt adapter and suggest route (#424 slice 3a) - #448

Merged
JArmandoAnaya merged 1 commit into
mainfrom
feat/sam-suggest
Aug 8, 2026
Merged

feat(inference): SAM 2 point-prompt adapter and suggest route (#424 slice 3a)#448
JArmandoAnaya merged 1 commit into
mainfrom
feat/sam-suggest

Conversation

@JArmandoAnaya

@JArmandoAnaya JArmandoAnaya commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Everything the editor's suggest gesture needs server-side: a SAM 2 adapter that serves
PointPrompt, the mask-to-geometry conversion D3 describes, and the route the editor will call.

cf. #418 (the port and its spike findings), cf. #421 (the connections a provider is
instantiated from), cf. #424 (the feature; D1, D2, D3, D5 govern this slice). This is slice 3a
and does not complete #424 — the tool, the preview and acceptance are 3b.

What changed

  • LocalSamProvider (inference/sam_provider.py) — the counterpart to the detector adapter:
    that one answers words and refuses points, this one answers points and refuses words. Positive
    and negative point sets in the asset's own pixels, one region out, iou_scores as the published
    confidence.
  • inference/masks.py — mask to domain geometry. Moore-neighbourhood boundary tracing,
    Douglas–Peucker simplification, and the narrowing to what the active class admits. Written over
    plain sequences of booleans, so the whole of D3 is drivable with literals on a machine with no
    GPU and no extra installed.
  • inference/cache.py — a bounded LRU, and the two capacities that use it with the arithmetic
    behind each written down.
  • inference/providers.py — family resolution and the provider pool. provider_for moved
    here from the package __init__, which now re-exports it.
  • inference/suggestions.py — the orchestration: resolve the connection, read the asset's
    bytes, run the provider, narrow the answer. Here rather than in a route because a command and a
    tool would each need the same four steps.
  • POST /inference/suggest — asset ref, connection ref, points, and the geometry kinds the
    class admits. openapi.json and the generated TS client are regenerated and committed.
  • Docsdocs/inference.md gains the suggest flow, the stateless-refinement rule, the
    encode-once behaviour and the refusal order.

No annotation is written anywhere in this slice, no frontend, no batch invocation, and no new
allowed_actions.

SAM 2 is available at the locked versions

The dispatch made this a halt condition, so it was checked rather than assumed. At the locked
transformers 5.14.1: Sam2Model, Sam2Processor, Sam2ImageProcessor, Sam2VisionModel,
alongside a sam2_video package for the 0.2.0 door D1 keeps open.

More to the point, the encode/decode split D5's latency budget rests on is a first-class API
rather than something this adapter had to synthesize:

  • Sam2Model.get_image_embeddings(pixel_values) — the once-per-asset encode.
  • Sam2Model.forward(input_points=…, input_labels=…, image_embeddings=…) — the per-click decode,
    taking the cached embedding directly.
  • Sam2Processor.__call__ accepts original_sizes without images, so a refine click never
    re-preprocesses the image.

The cache key therefore sits exactly where the library already draws the line. No substitution
question arose.

Design decisions worth review

The port carries no notion of what a class allows, and that is deliberate. D3 needs the
answer shaped to the active class's schema, and PredictionRequest has nowhere to say so.
Widening it would push a project's schema into a protocol that has to be implementable by a
service which has never heard of this workspace — the dual test #418 sets. So the adapter answers
with the most informative shape it has, and suggestions.py narrows: a polygon stands where
polygons are allowed, becomes its own bounding box where only boxes are, and is refused where
neither is. D3 ends up as pure code above the adapter, and the port is untouched.

Adapter resolution is by the model's declared family, not by ConnectionType. That enum says
only where a model runs. It cannot say whether the weights behind a local connection are a
detector or a segmenter, and those answer different questions. The family is read from the
model's own config — a small JSON file already in the cache beside the weights. A connection
pointed at a detector and asked with points is then refused with UNSUPPORTED_PROMPT rather than
dying inside a forward pass on a shape mismatch.

The two caches are one mechanism. Keeping an image embedding is worth nothing if the provider
is rebuilt per request: a fresh provider carries an empty cache, so every click pays the encode
too. Each cache is defeated by the absence of the other, which is why both land in this slice.
Bounds are 8 embeddings (~64 MB on a hiera-base-plus-shaped encoder) and 2 providers — the
detector-plus-segmenter co-residency D1 describes.

The SAM adapter answers with an empty label. Pointing says where, not what; this model
has no vocabulary. The editor already knows the active class — that is what chose the geometry
kinds — so a name invented in the adapter would be a second, worse source for something the
caller already holds. The response shape carries no label at all.

Polygon tolerance is relative, not absolute. D3 asks for one "detail" knob landing typical
objects in 10–40 vertices. An absolute pixel tolerance cannot do that: three pixels is nothing on
a car and is the whole of a bottle cap. The tolerance is a fraction of the region's own bounding
diagonal, and the property is asserted across a 37x size range.

One kernel change the dispatch did not ask for

ImageFormat → media type lived private to server/routes/assets.py. visionset.inference needs
it to build a PredictionTarget — a provider has to be told what the bytes it is handed are — and
importing server from inference is forbidden by contract. A second copy of a two-line map is
how a product ends up serving image/png on one surface and application/octet-stream on another
for the same asset, so it moved down into kernel/domain/media.py. Both readers now index the
same table, the route's declared content types are built from it rather than restated, and the
exhaustiveness test follows it.

The shared surface, and why 3b is sequential

openapi.json and frontend/ui-core/src/generated/ are regenerated in this PR. Slice 3b consumes
exactly that generated client, so it cannot start until this merges — which is the whole reason
the work was cut here rather than shipped as one change.

Mutation verification

Rule broken Mutation Test that went red
The embedding cache returned the computed embedding without holding it tests/inference/test_sam_provider.py::test_a_second_click_on_the_same_asset_decodes_without_encoding_again
D3's allowed-geometry narrowing returned the polygon regardless of allowed_geometries tests/server/test_suggest.py::test_a_box_only_class_is_offered_the_outlines_extent

Both are worth naming because correctness cannot tell the mutated versions apart, and the
blast radius was measured rather than assumed. With the cache bypassed, exactly one test fails
across the whole adapter suite and the whole suggest route suite — every assertion about the
answer stays green, which is why the provider carries an encodes counter at all. With the
narrowing removed, two fail, both of them D3's own cases; an un-narrowed polygon is a perfectly
valid answer to every other question the suite asks.

Each mutation was applied to a committed tree with its anchor asserted present exactly once
beforehand, and reverted with git apply -R on the recorded patch. git status --porcelain was
empty after each revert.

Rebased onto #447

main took fix(annotator): completing a job flips the workspace to read-only in place (#447)
while this PR's first CI run was going, and it conflicted: both sides appended to __all__ in
kernel/domain/__init__.py at the same place — theirs OPEN_JOB_STATES, mine MEDIA_TYPES and
OCTET_STREAM. All three kept.

The full gate was re-run after the rebase rather than assumed to still hold, which is the rule
for a rebase bringing in commits you did not write: a branch and somebody else's can be textually
compatible and semantically not, and the targeted tests you would think to re-run all pass. The
table below is that post-rebase run. It also shows #447's own tests arriving and passing beside
this work — ui-core 742 (was 737) and annotator e2e 231 (was 230). No OpenAPI or generated-client
drift followed the rebase, so #447 did not move the contract under the artifacts regenerated here.

The mutation verification above was performed on the pre-rebase commit; neither mutated line was
touched by the rebase.

Test plan — full gate, exit codes verbatim

bash scripts/check.sh (no --fast), all four groups, in the worktree. Node 24.19.0 + pnpm
10.30.2.

Stage Exit
python tests 1 — baseline-proof exception, see below
ruff (lint) 0
ruff (format) 0
mypy 0
import contracts 0 — 4 kept, 0 broken
frontend build 0
frontend tests 0 — annotator 868, ui-core 742
frontend lint 0
openapi drift 0
generated client drift 0
mcp tool reference drift 0
version sync 0
annotator + app e2e (chromium) 0 — 231 passed
browser cycle, real server (chromium) 0 — 1 passed

check.sh: FAILED ran=python,frontend,generated,browser skipped=none, exit 1, on 06a3dd7.

An earlier run of this gate also failed ruff (lint) and ruff (format) — four lint errors and
three unformatted files, all in the new test files, all mine. Fixed, and the table above is the
re-run after the fix rather than the first attempt.

Merged under the baseline-proof exception — step python tests, cf. #444

The one failing step was already red on main, and every condition the protocol sets is met.

Reproduced on unmodified main at the merge-base (7ca7363), in this environment, in this
worktree, before the first edit
— not taken from a previous session's report:

=========================== short test summary info ============================
FAILED tests/kernel/test_ingest_service.py::test_a_truncated_clip_keeps_what_decoded_and_reports_the_break
FAILED tests/kernel/test_video_processor.py::test_a_truncated_clip_yields_what_decoded_and_then_refuses
    FAILED (python tests, exit 1)

The branch run, verbatim — the same two ids, and nothing else:

=========================== short test summary info ============================
FAILED tests/kernel/test_ingest_service.py::test_a_truncated_clip_keeps_what_decoded_and_reports_the_break
FAILED tests/kernel/test_video_processor.py::test_a_truncated_clip_yields_what_decoded_and_then_refuses
    FAILED (python tests, exit 1)

The diff does not touch that step's surface. The failure is in video frame extraction —
ffmpeg_video_processor.py and the ingest path that drives it, exercised through
write_corrupt_video. This branch touches visionset.inference, the media type table in
kernel/domain/media.py, two server route modules and the models beside them. It adds no video
code, changes no decoder invocation, and the media-type move is a lookup table for
Content-Type strings, which the extraction never consults. No still-image test regressed.

Tracked as #444, which also corrects the diagnosis this branch would otherwise have shipped:
the variable is not the ffmpeg version — CI installs the identical 7:6.1.1-3ubuntu5 and passes —
but -xerror tearing the run down before the muxer flushes, with how much survives depending on
the host's core count. This host has 20.

The matching CI job is green on this PR — see the checks.

Found, not fixed

…lice 3a)

Everything the editor's suggest gesture needs server-side: an adapter that
serves PointPrompt, the mask-to-geometry conversion, and the route the editor
will call.

The port carries no notion of what a class allows, deliberately — widening it
would push a project's schema into a protocol that has to be implementable by a
hosted service. So the adapter answers with the most informative shape it has
and the orchestration narrows it to the geometry kinds the caller names.

Adapter resolution reads the model's own declared family rather than the
connection kind, which says only where a model runs. A connection pointed at a
detector and asked with points is then refused with the port's vocabulary
instead of failing inside a forward pass.

The embedding cache and the provider pool ship together because each is
defeated by the absence of the other: a provider rebuilt per request carries an
empty cache, so every click would pay the encode.

The ImageFormat media-type table moves into the kernel domain, since the
inference adapters became its second reader and a route may not be imported
from there.

cf. #418, #421, #424
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant