A second architecture answers a point prompt - #599
Merged
Conversation
JArmandoAnaya
marked this pull request as draft
August 16, 2026 01:51
Contributor
Author
|
Moved to draft: this does not work as it stands, and the green check set is not evidence that it does. With access granted and a token in the environment, the one thing the body listed as unverified turned out to be the thing the feature depends on.
So every test here passes and the capability is unreachable. Two of the three names I registered are traps rather than near-misses: This is the failure mode that module exists to prevent, and I walked into it: the family is supposed to be read from the model and I reasoned it from class names instead. Nothing in the suite could have caught it, because every test agreed with the same guess the implementation made. The lesson for the fix is that a family entry needs a config it was read out of, not a plausible derivation — and the test that would have failed is one asserting the register against a real published config rather than against itself. Correcting the register now, and confirming which |
Contributor
Author
|
Corrected in The register is Run on CPU in a container against
The guard that was missing now exists, and it is the one that would have caught this: a parametrised test naming each of the three near-misses — Gate after the fix: python PASSED, frontend+generated+docs PASSED, browser 272 passed with the real-server cycle run beside it — the two #550 scenarios passed this time, which is what a flake does. Back out of draft. |
JArmandoAnaya
marked this pull request as ready for review
August 16, 2026 02:22
The resolver picks an adapter from the family a model's own config declares, so registering a second point-prompt architecture is an entry in the segmenter family register rather than a new adapter. SAM 3 declares `sam3` when a repository publishes the whole model and `sam3_tracker` when it publishes only the promptable half; both are served here, and the capability each one grants is derived from the register as before. What could not be derived is the pair of `transformers` classes a family loads through, so `sam_provider` gains a table. SAM 2 keeps `AutoProcessor`, which resolves correctly for the repositories it is pointed at. SAM 3 cannot: a repository publishing the whole model declares the concept processor, which takes text and has no `input_points` argument, so resolving there would hand the adapter something no click can be expressed to and the failure would land inside a call rather than in a refusal. Everything below the loader is untouched, because the tracker agrees with SAM 2 signature for signature on the encode, the forward and the mask post-processing. Fetching weights gains one behaviour. A repository whose terms have not been accepted was reported by re-raising the hub client's own text, which opens with a status line and a request id; it now says what happened and what to do, and the transport stays out of it. The case is told from a mistyped model id by the exception's class and never by its status code, because both answer 401 and only the class separates them. cf. #595
… be accepted Adding SAM 3 to the point-prompt ladder puts an entry in the list that nobody can simply download, which is a thing the list had not had to say before. `CuratedModel` gains an optional `access` pair — one sentence and the page where it is cleared — and the form states it while the model is being chosen rather than leaving it to the refusal a download would eventually produce. A person who has not been granted access now reads that before creating anything. The entry's size is the hub's figure for the pinned revision, as every entry's is, and it is roughly twice the model it installs: the repository publishes its weights as both a checkpoint and safetensors, and fetching a revision fetches all of it. What lands on the disk is the number that belongs beside a download. The module's own note said both groups were Apache-2.0, which this makes false, so it now states the exception and why the entry is offered anyway. Leaving a gated model out of a curated list does not spare anybody its terms; it means the people who want it type the id in from somewhere else, having read nothing. The annotator's not-capable blocker named the SAM 2 family as what a click needs. It now names what the model has to do, because more than one architecture does it and prose repeating a register is the copy that goes stale without any build failing. cf. #595
…revision The access line resolved its entry through `curatedEntry`, which compares the revision as well as the model id. That is the right question for the select, where it answers "is this row showing exactly this curated entry", and the wrong one here: pinning a different commit of a gated model does not exempt anybody from its terms, so the line vanished while the requirement still applied. Found by mutation rather than by reading. Replacing the lookup with one keyed on the model id alone left every test green, which said the distinction was unverified — and looking at which of the two behaviours was correct showed the mutation was the fix. The test that now pins it drives the edit dialog of a stored connection naming the gated model at another commit. cf. #595
…ching The curation rules excluded custom-gated licenses alongside copyleft ones, and a section recorded `facebook/sam3` as blocked on two counts. Both are now decided the other way and the model ships, so the rules state the position that replaced them rather than leaving a reader to infer it from the catalog disagreeing with the prose. The reference documentation gains the two steps a gated model needs — request access, set HF_TOKEN — and says where the gate is not: reading a size needs neither, which is what lets the form price a download before anybody asks for one, and running weights already in a workspace needs neither, which is what keeps a copied workspace working. Two measurements are written down because both contradict the assumption a reader would otherwise make: the hub sizes a gated repository unauthenticated, and a gated repository answers 401 rather than 403 — the same status a repository that does not exist answers, so the two are told apart by the client's exception class and never by the code. cf. #595
…ves in Measured against the wheels rather than inferred: 4.57.6 ships neither the `sam3` nor the `sam3_tracker` module and 5.0.0 ships both, exporting `Sam3TrackerProcessor` and `Sam3TrackerModel` at the top level, which is what the adapter names. 5.0.0 is also where naming the processor rather than resolving it stops being a preference and becomes the only thing that works: `sam3_tracker` is absent from the auto processor map in that release, so `AutoProcessor` has no answer for it at all. It gained one later. The adapter's table works on both. The bump is a major one for everybody carrying the extra, including the SAM 2 and Grounding DINO users who gain nothing from it. What it mostly does is make the floor honest: `uv.lock` has resolved 5.x for as long as the extra has existed, so `>=4.44` was a compatibility claim no run has ever tested. cf. #595
… is reachable The api service passed no token, so weights that have to be asked for could not be fetched from the dev stack at all. The failure mode is the bad one: the refusal names `HF_TOKEN`, the reader sets it on the host, the container never sees it, and the identical refusal comes back with nothing about it changed. A refusal naming a remedy nobody in that position can take is the mistake `NotAWorkspace` is the standing example of. One forwarded variable, empty when the host has none, which is what every ungated model already expects. Verified through `docker compose config` in both directions: absent it renders empty, set it renders the value. cf. #595
…ctually opens The conditional is already held in jsdom. What that layer cannot say is anything about the journey: that the sentence appears in the real dialog, above the real size line, before the control that would fetch anything is reachable. This opens Inference in chromium, picks the gated entry and reads the line and its link. cf. #595
…ideo
The register carried `sam3` and `sam3_tracker`, derived from the names of the
`transformers` classes. `facebook/sam3` declares neither. Its config is a
`Sam3VideoConfig` whose top-level `model_type` is `sam3_video`, and the two names
that were registered sit one level down inside it:
Sam3VideoConfig model_type = sam3_video <- what resolves
|-- detector_config model_type = sam3
+-- tracker_config model_type = sam3_tracker_video
`family_of` reads the top level, so a connection to that repository was refused
with "this build has no adapter for that model type" and the capability was
unreachable. Every test passed, because they had all been written against the same
guess the implementation made.
`sam3` is now deliberately absent rather than merely corrected: it is what the
detector half declares, so serving it here would hand a text-prompt model to the
point adapter — a confident wrong answer rather than a gap somebody notices.
`sam3_tracker` is absent because no known checkpoint declares it, and a set entry
nobody has measured is a guess wearing a frozenset's clothes.
The classes are now measured against a real load on CPU rather than inferred.
`Sam3TrackerModel.from_pretrained` on this checkpoint reports missing_keys 0,
unexpected_keys 0, mismatched_keys 0, and a point prompt runs the whole way
through: an embedding, `pred_masks` of (1, 1, 3, 288, 288) beside `iou_scores` of
(1, 1, 3), and `post_process_masks` lifting them to (1, 3, 240, 320) — the shape
`_segments` already reads. The same run confirms `AutoProcessor` answers
`Sam3VideoProcessor` for this repository, so naming the tracker processor is
required rather than tidier.
cf. #595
Forwarding `HF_TOKEN` was a single line with nothing holding it, and losing it fails the quiet way: the refusal still names the variable, the reader still exports it on the host, and the identical refusal comes back because the container never saw it. Three assertions, and the second is the one a grep for the variable could not make. Both inference stacks are `-f` overlays that amend the base's own `api` service, so the credential reaches CUDA and CPU alike by inheritance — which stops being true the day an overlay grows an `environment:` block of its own and somebody assumes it merged. The gate reads the files rather than rendering them, because `docker compose config` needs a daemon and the job that runs this has none; what that gives up is exactly what the second test pins by hand. Verified against the rendered configuration for all three stacks first, in both states — token exported and token absent — and then by mutation: removing the line reddens the first test, and giving an overlay its own environment block reddens the second. cf. #595
JArmandoAnaya
force-pushed
the
feat/sam3-point-segmentation
branch
from
August 16, 2026 05:16
f6a1121 to
8ec8418
Compare
…t sits "SAM 3 — a newer architecture, the largest download here" positioned the entry against two things a reader does not have. There is no released history for it to be newer than, and the list is grouped by the question a model answers rather than ranked, so there is no order for it to be largest within — both halves read as a comparison whose first half is missing. "Different" and "other" fail the same way for the same reason. The entry now says what it needs, which is the one fact about it not already on screen: the size sits beside the hint and the access requirement sits under it. The field's own documentation invited this — it asked for "the difference between this rung and its neighbours", which is a sound brief for the rungs of one ladder and a trap for an entry that is nobody's rung. It now says what a hint may claim and names the words that point somewhere the reader cannot see. The module note called the entry "the last rung of the point-prompt ladder" and carried the same implication; it names the model instead. cf. #595
…g cut `SelectContent` bounded nothing and its viewport did not scroll, so a list longer than the room under its trigger was clipped by the `overflow-hidden` that rounds the corners. The options past the edge stayed in the DOM and stayed reachable by keyboard, while a pointer had no way to get to them and nothing on screen said they were there — a list that looks complete and is not. Measured on a 600px-tall window before the fix, the list's bottom edge sat at 836px. Not a fault of any one list. Every select in the app shares this primitive and the short ones — devices, precisions — never grew far enough to show it; the curated model list did, because its options are two lines each. The bound is `--radix-select-content-available-height`, Radix's own measurement of the gap between trigger and viewport edge, so it follows a window resize and a trigger near the bottom of the screen rather than a constant here guessing at both. The scroll buttons are the affordance rather than decoration: macOS hides overlay scrollbars until a scroll is already under way, so without them a truncated list and a whole one look identical until somebody gambles on a gesture. Asserted in chromium, because it is layout under a real viewport — jsdom reports every height as zero and would have agreed with the implementation this replaces. Verified by putting that implementation back and watching the assertion fail on the same 836px. cf. #595
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
1 participant
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.
facebook/sam3becomes a second driver of the interactive point-segmentation capability thatalready exists, alongside SAM 2. It is not a new capability:
ModelCapability.POINT_SUGGESTisunchanged, the dashboard gains no section, the annotator gains no control, and a connection
pointed at SAM 3 becomes a candidate for the suggest tool on the same terms as a SAM 2 one. The
scope and the licence treatment are the decision recorded on the SAM 3 provider issue (#595).
openapi.json, the generated client andVERSIONare byte-identical. No migration, no new kernelservice, model, error or event, nothing added to
ERROR_RULES, and no CI job added or renamed —so the
mainruleset needs no edit.The task's premise was wrong in one load-bearing way
The work was specified against
Sam3ModelandSam3Processor. Those are the text-and-conceptpath, which this issue puts out of scope. Point prompts run through
Sam3TrackerModelandSam3TrackerProcessor, loaded from the same repository with no subfolder. The library's ownauto-maps are unambiguous:
The consequence is that
AutoProcessorcannot be used here. Asked aboutfacebook/sam3itanswers
Sam3VideoProcessor, measured against the real repository — the video path, which nosingle-image click can be expressed to. The failure would land inside a call rather than in a
refusal.
Those class names are not the family names, and conflating the two is the mistake this branch
made once. A model's family is the
model_typeits config declares at the top level, which forthis checkpoint is
sam3_video;sam3andsam3_trackername classes and sit one level down ornowhere at all. See the end-to-end section below.
Why the change is small
Resolution is family-driven:
SEGMENTER_FAMILIESis a set ofmodel_typestrings read from amodel's own downloaded config, and
CAPABILITY_BY_FAMILYis derived from it, so an adapter and itscapability declaration are the same edit. And the interface has no model-centric surface — the unit
is a connection, the dashboard groups by the wire's
capabilities, and the annotator's pickerselects a connection filtered on
point_suggest, remembering the choice per project already.So there is no new adapter class, no new capability, no registration table and no new component.
providers.pyneeded one argument threaded through and nothing else.The tracker is a straight port of SAM 2's image path, compared in the installed source rather than
taken from documentation. Every signature the adapter depends on is identical:
get_image_embeddings(pixel_values, **kwargs), the fullforwardkeyword list,post_process_masks(masks, original_sizes, …), a processor__call__that acceptsoriginal_sizeswithoutimages, and outputs carryingpred_masksandiou_scores. That is whythe embedding cache, the decode path, the fp16 guard and the mask-to-shape pipeline are untouched
and only two class names vary.
sam_provider._CLASSESholds them, one row per family, keyed by exactly the members ofSEGMENTER_FAMILIESand held to it by a test. SAM 2 keepsAutoProcessoron purpose: thecheckpoint the form suggests declares
processor_class: Sam2VideoProcessor, so that call alreadyreturns the video processor and that is shipped behaviour, not an oversight to tidy.
The one behavioural fix
Fetching weights for a repository whose terms have not been accepted re-raised the hub client's own
text, which opens with a status line and a request id:
That reached a person verbatim on a failed job. It is now two sentences — what happened, and what
to do — and the translation lives in one helper both
downloadandmeasureroute through ratherthan at the one call site that prompted it.
Two details decide how it is written, and both were measured:
401, not the403the word suggests — and so does a repositorythat does not exist. So the case is told by the client's exception class and never by the
status code; a branch on the number sends somebody with a typo'd model id to request access to a
model nobody publishes. There is a test for exactly that discrimination.
GatedRepoErrorinherits fromRepositoryNotFoundError, so a handler naming the parentfirst silently swallows the gated case — the ordering trap the media adapter records for
UnidentifiedImageErrorunderOSErrorand the persistence adapter forIntegrityErrorunderDatabaseError.No new error class:
LocalInferenceUnavailablealready carriesexpose_message, so the sentencereaches a person unchanged and nothing moves on the wire.
Measurements that changed the design
model_infofor a gated repositoryunauthenticated, so the form prices a download before anybody decides whether to go and ask for
it. An earlier draft of this work assumed the opposite.
twice, as a checkpoint and as safetensors, and fetching a revision fetches all of it. The curated
entry carries the figure that describes the disk.
transformersfloor moves>=4.44to>=5.0, probed against the wheels: 4.57.6 shipsneither the
sam3nor thesam3_trackermodule, 5.0.0 ships both. 5.0.0 is also where naming theprocessor stops being a preference —
sam3_trackeris absent from the auto processor map in thatrelease and gained an entry later. The bump is a major one for everybody carrying the extra,
including SAM 2 and Grounding DINO users who gain nothing from it; what it mostly does is make the
floor honest, since
uv.lockhas resolved 5.x for as long as the extra has existed.A defect found by mutation rather than by reading
The access line resolved its entry through
curatedEntry, which compares the revision as well asthe model id. Replacing that with a lookup keyed on the model id alone left every test green, which
said the distinction was unverified — and asking which of the two behaviours was correct showed the
mutation was the fix. An access gate belongs to the repository: pinning a different commit of a
gated model does not exempt anybody from its terms, so the line vanishing when the revision was
edited was hiding a requirement that still applied. A test now drives the edit dialog of a stored
connection naming the gated model at another commit.
The dev stack could not have taken the remedy
docker/compose.yaml's api service passed no token, so the failure mode was the bad one: therefusal names
HF_TOKEN, the reader sets it on the host, the container never sees it, and the samerefusal comes back unchanged. A refusal naming a remedy nobody in that position can take is the
mistake
NotAWorkspaceis the standing example of. One forwarded variable, empty when the host hasnone, verified through
docker compose configin both directions.Documentation that had recorded the opposite
docs/architecture/backend/inference.mdcarried a curation rule excluding custom-gated licencesalongside copyleft ones, and a section recording this model as blocked on two counts. Both are now
decided the other way, so the rules state the position that replaced them rather than leaving a
reader to find the catalog disagreeing with the prose. The new rule is that a gated model may be
curated and may never be the default, and there is a test holding the default to a model anybody
can fetch.
Found, not fixed
annotate.spec.tsscenarios fail under the local gate's ten workers — the trackedlocal-only flake, cf. Two annotate.spec.ts scenarios fail only under the local gate's ten workers #550. Evidence below.
frontend/app/cycle/cycle.spec.tsneverexercises auto-labeling, so "a suggestion works end to end against a real server" is not gated
anywhere. Out of scope here, worth its own issue.
_device.resolvedcovers a device being absent, not anallocation failing mid-forward. SAM 3 is roughly ten times the parameters of the balanced SAM 2
rung, which makes an out-of-memory during a forward pass reachable in a way it was not before.
Nothing here changes it and no refusal names it.
polarsandpolars-runtime-32are yanked inuv.lock. Pre-existing, surfaced as a warningby any
uv lock; untouched here.Test plan
New coverage: the family register's two spellings and the exclusion of nested config halves; the
class table's agreement with
SEGMENTER_FAMILIESand the per-family classes; that no load reachesthe network; the gated refusal from both
downloadandmeasure; that a repository which is notthere is not reported as a licence to accept; that a client too old to name the error still
reports the failure; the catalog's access note and its absence from the default; the access line
appearing while a model is chosen, disappearing when the choice moves back, and surviving a
different pinned revision; and one chromium scenario reading the line in the real dialog.
Twelve mutations were applied and reverted by recorded patch, each asserted present-once before and
present after, with a clean-tree check between cases. Every one turned a named test red. The
harness itself was rewritten partway through: a zsh version using heredocs inside a function
silently failed to apply its mutations while reporting the replacement absent, which is the
false-calm failure the protocol warns about, so it was rebuilt in Python where there is no shell
quoting to get wrong.
The gate was run in stages against the ~10-minute ceiling. Exit lines verbatim:
Frontend suites:
frontend/annotator 1039 passed (1039),frontend/ui-core 1013 passed (1013).Browser stage, this branch:
and the real-server cycle run beside it:
1 passed (44.7s).The two failures were red at the merge-base
Reproduced by me, in this environment, on unmodified
mainat the merge-base9bf9266, with thecommand #550 gives. It is a flake, so it takes repetition to show — three runs:
The same file on this branch, same command:
1 failed,1 flaky,90 passed (1.6m)— the sametwo scenario names.
The diff does not touch what those scenarios exercise. They walk asset navigation and a canvas
scroll position; the only annotator file in this diff is
SuggestPanel.tsx, and the change there isone sentence of copy in the blocker shown when no connection can answer a click.
annotate.spec.tsitself is untouched. Worth knowing for anybody reading a red gate on their own branch: adding any
scenario anywhere reshuffles what runs beside what across the ten workers, so a branch touching
nothing in that file can still flip it — two consecutive full browser runs on this branch were green
before a scenario was added to
inference.spec.ts, a different file. A fresh reproduction isrecorded on #550.
Verified end to end, on the real weights
Access was granted and the whole path was run on CPU in a container, which settled the question
this body previously left open — and caught a defect that made the feature unreachable. The
resolution is on
sam3_video, which is what the published checkpoint declares; an earlier revisionof this branch registered
sam3andsam3_tracker, derived from thetransformersclass names,and neither appears anywhere
family_oflooks. The full account is in the commit and in a commenton this PR.
Measured against
facebook/sam3at the pinned revision,torch 2.13.0+cpu,transformers 5.15.0:Every parameter the tracker needs came out of the checkpoint and nothing is left randomly
initialised, and the post-processed shape is exactly what
_segmentsalready reads — three masksat the asset's own size, scored by the IoU head that
best_ofpicks from.What is still not covered is the product path above the adapter: this ran the
transformerscallsthe adapter makes, not
visionset inference downloadfollowed by a click in the editor. Themachine that could do that is not this one —
torchpublishes no macOS x86_64 wheel at the lockedversion — so the remaining check is somebody driving the CPU stack end to end.