fix(inference): concurrent suggests encode once and build one provider

[JArmandoAnaya]

Three items were dispatched against the suggest tool's latency, from an instrumented diagnosis.
One of the three was falsified before a line was written, and that is the more useful half of
this change: the autograd leak that was the leading explanation for a 4.5 s warm click does not
exist. What is left is a real concurrency defect, which is fixed here, and a checkpoint warning
that turns out to be correct behaviour, which is now recorded where somebody will read it.

Closes #571.

Premise verification

Both premises the dispatch was built on were checked against HEAD before any work started, and
one did not survive.

The running compose stack is bound to a different worktree than this one — docker inspect visionset-api-1 reports its mounts coming from VisionSet-wt/slider-drag, the branch behind the
open detail-slider pull request (#565). That worktree was treated as a banned surface and not
touched. Every measurement below states the checkout and commit that produced it, and every
absolute number is a CPU figure from the CPU-inference image (torch 2.13.0+cpu, 16 cores
visible, torch.get_num_threads() reporting 8). None of them transfer to a machine with a GPU.

A. The autograd leak does not exist

The hypothesis was that LocalSamProvider._embedding runs its encode outside the guard the decode
uses, so cached embeddings might carry a live autograd graph that every later decode pays for.

It does run outside the guard. It does not matter, because transformers closes it one level
down: Sam2Model.get_image_embeddings is itself decorated @torch.no_grad() in the locked
transformers 5.14.1. Measured rather than read — a probe built the real provider against the real
checkpoint and then read the cache:

click 1 (cold, encode+decode): 43929.5 ms   encodes=1
click 2 (warm, cache hit):     4725.2 ms   encodes=1
click 3 (warm, cache hit):     4654.0 ms   encodes=1

--- what is actually in the embedding cache ---
type: list, len=3
  [0] shape=(1, 32, 256, 256) requires_grad=False grad_fn=None is_inference=False bytes=8,388,608
  [1] shape=(1, 64, 128, 128) requires_grad=False grad_fn=None is_inference=False bytes=4,194,304
  [2] shape=(1, 256, 64, 64)  requires_grad=False grad_fn=None is_inference=False bytes=4,194,304

ALL CACHED TENSORS INERT: True
cached bytes for this one asset: 16,777,216 (16.0 MiB)

Every cached tensor is already inert. No guard and no detach() were added, because a test
asserting inertness passes on unmodified HEAD — that is red-before-green failed — and code that
changes nothing cannot be mutation-verified. Two things worth keeping from the probe anyway: the
embedding is a list of three tensors rather than one, which any future detach would have to map
over, and one asset's embedding costs 16 MiB, so eight of them is 128 MiB and the cache is not
where the memory went.

A trap that was avoided and is worth recording for whoever revisits this: torch.inference_mode()
is not interchangeable with no_grad() here. Tensors created under inference mode are
inference tensors, the decode runs under no_grad() rather than inference mode, and using one
there raises at runtime — so a cache that spans the two contexts must not be filled under
inference mode.

The warm click is therefore unchanged, and this pull request does not claim otherwise.

	before (a7e53c9)	after (d403e37)
warm click, second on the asset	4,725.2 ms	4,561.8 ms
warm click, third on the asset	4,654.0 ms	4,624.1 ms

Within noise, as expected: the decode was always the cost and nothing here touches it. What the
decode's 4.5 s actually is remains open and is not diagnosed in this change.

B. Concurrent suggests encode once and build one provider

This is the item that carried the work. Both sites had the same shape — find nothing, compute,
store — with a window between the finding and the storing wide enough to drive a model load
through. In production that window was observed being taken: two clicks on one un-encoded asset
both encoded it, two concurrent first clicks both built a provider, one process loaded the model
four times, and one burst of concurrent suggests took the whole development stack down with nginx
exiting 137.

KeyedLocks in cache.py closes it per key. A second caller for the same asset, or the same
connection, waits for the first rather than duplicating it; two different assets still encode in
parallel, because a global lock would trade a duplicated-work problem for a queueing problem and
cost exactly the latency the cache exists to save. The cache is read twice on purpose: the first
read is the common case and takes no lock at all, the second is what the loser of a race sees.

The BoundedCache contract choice, which the dispatch asked to be stated

BoundedCache is now internally thread safe, rather than being left lock-free with all
synchronisation at its callers. Its docstring had claimed it was deliberately not thread safe, on
the reasoning that a worker process runs one task at a time and a server handler is serialised by
the device it talks to. Neither holds: suggest_region is a plain def, so FastAPI answers
concurrent requests in parallel threadpool threads. Two threads calling move_to_end for
different keys are still mutating one OrderedDict's linked list, which no amount of per-key
locking at the caller protects. The lock is never held across a computation, so it cannot
serialise two encodes; that is KeyedLocks' job, and the two are kept separate because they answer
different questions.

C. The sam2_video warning is correct behaviour, and is now documented in place

transformers warns on every load that a sam2_video-typed checkpoint is being instantiated as
Sam2Model. Asking the loader directly settles it:

config.model_type = 'sam2_video'  (Sam2VideoConfig)
--- Sam2Model loading info ---
missing_keys: 0
unexpected_keys: 0
mismatched_keys: 0
error_msgs: 0

Nothing is left randomly initialised and nothing in the checkpoint goes unused, so the load is
exact. The alternative class cannot serve this call at all: Sam2VideoModel.forward takes an
inference_session and a frame index, which is the video-tracking path with no way to answer a
point on a single image. Why sam2_video is a served family is already argued in families.py;
what was missing was the evidence at the load site, and _load now carries it.

The warning is left visible. Silencing it would hide the same sentence on the day a checkpoint
genuinely does not match, and that day it is the only warning there is.

Red before green

tests/inference/test_concurrency.py on unmodified a7e53c9:

FAILED tests/inference/test_concurrency.py::test_concurrent_clicks_on_one_asset_encode_it_once
FAILED tests/inference/test_concurrency.py::test_concurrent_first_clicks_build_one_provider
FAILED tests/inference/test_concurrency.py::test_concurrent_callers_all_receive_the_same_provider

E       AssertionError: one connection, one provider
E       assert 4 == 1
E        +  where 4 = <visionset.inference.providers.ProviderPool object at 0x108acbad0>.builds

The file's other three cases pass on HEAD and are guards rather than reproductions — stated
plainly, because a green test presented as a fix is worse than no test.

It is the suite's second threaded file, and it follows the first one's rules
(tests/kernel/test_concurrency.py): every wait is a threading primitive rather than a sleep,
every thread is joined with a timeout and then asserted dead, and failures inside threads are
returned rather than printed and lost. Overlap is asserted through a barrier, never through
wall-clock — a barrier that releases proves two encodes were genuinely in flight, one that times
out proves they were not, and neither reading depends on how fast the machine is. Repeated 20
consecutive times, 0 failures before shipping.

Mutation verification

Work committed before the first mutation. Each step an unconditional statement rather than a link
in an && chain, anchor asserted present exactly once before and the replacement after, reverted
by git apply -R on the recorded patch, tree asserted clean between cases.

mutation	named tests that went red
_embedding's per-key lock → if True:	test_concurrent_clicks_on_one_asset_encode_it_once
ProviderPool.get's per-key lock → if True:	test_concurrent_first_clicks_build_one_provider, test_concurrent_callers_all_receive_the_same_provider
KeyedLocks.for_key returns a fresh lock each call	test_one_key_answers_with_one_lock, and all three concurrency tests
BoundedCache._lock → nullcontext()	none — green, and reported as such

The fourth row is honest rather than tidy. The BoundedCache lock guards the container's internal
consistency under concurrent mutation of different keys, and no deterministic test can observe
that: a stress test over an OrderedDict stays green without the lock on CPython in practice,
which would be a test that cannot fail rather than coverage. The rule is stated in the docstring
and left unverified, which is the accurate description of it.

The harness itself had a bug worth recording, because it is the failure mode the protocol warns
about: the first attempt passed two test paths as a single unquoted parameter, and zsh does not
word-split unquoted parameters the way bash does, so pytest was handed one nonexistent path and
exited 4. That reads as a mutation nothing covered. It was caught by the exit code not being 1,
re-run correctly, and the row above is from the corrected run.

The gate

Staged, because the harness kills a command at about ten minutes. Directories derived from
ls tests/ at run time rather than from a remembered list. Every stage's exit code:

tests/architecture 0    tests/inference 0     tests/packaging 0
tests/cli 0             tests/jobs 0          tests/scripts 0
tests/examples 0        tests/kernel 0        tests/server 0
tests/fixtures 0        tests/mcp 0           tests/test_versioning.py 0
tests/formats 0

ruff check .            exit=0   All checks passed!
ruff format --check .   exit=0   381 files already formatted
mypy src/visionset      exit=0   Success: no issues found in 159 source files
lint-imports            exit=0   Contracts: 4 kept, 0 broken.

bash scripts/check.sh generated   exit=0
bash scripts/check.sh frontend    exit=0
  frontend/annotator test:  Test Files  36 passed (36) / Tests  1000 passed (1000)
  frontend/ui-core test:    Test Files  50 passed (50) / Tests   967 passed (967)
bash scripts/check.sh docs        exit=0

The browser stage, and the declared fallback

bash scripts/check.sh browser did not come back clean on this machine, and the reason is the
machine rather than the change. Another session owns the box: fifteen vitest worker processes
belonging to a different worktree, VisionSet-wt/mps-device, at a load average of 137.46 /
226.73 / 155.71. Nothing belonging to that session was touched.

run 1:  1 failed,  1 flaky, 254 passed (3.4m)
run 2:  3 failed,  1 flaky, 252 passed (4.8m)

The four distinct scenarios involved across both runs are timing-sensitive: a scroll-into-view
assertion, a review round trip, a perf spec counting DOM mutations per pointer move, and a token
gate. Run together in isolation on the same worktree they pass 4 of 4 in 32.1 s, and the
single scenario from run 1 passes --repeat-each=3. The change is Python-only, confined to
src/visionset/inference/, with no route, wire shape or frontend file touched, so none of these
specs exercise anything this diff changes.

Per the protocol this is the declared fallback rather than a silent one, and it is declared here:
the two browser suites did not produce a clean full run locally, every other gate did, and CI
on clean runners is the arbiter.

CI is the arbiter, and it agrees

All fourteen required checks are green on clean runners, annotator e2e (chromium) among them —
which settles the local browser runs above as load rather than regression. (annotator bench (chromium, manual) is workflow_dispatch-only and correctly reports as skipping.)

One repair, and what hid the defect

The first CI run failed python at collection:

ERROR collecting tests/kernel/test_concurrency.py
import file mismatch:
imported module 'test_concurrency' has this __file__ attribute:
  /home/runner/work/VisionSet/VisionSet/tests/inference/test_concurrency.py
which is not the same as the test file we want to collect:
  /home/runner/work/VisionSet/VisionSet/tests/kernel/test_concurrency.py

With no __init__.py anywhere, pytest identifies a module by basename alone, so the new file
could not share one with the kernel's threaded file. It is now
tests/inference/test_provider_concurrency.py.

What is worth recording is why the local gate could not catch it. The gate is staged by test
directory to survive the harness's ten-minute ceiling, and each directory collects perfectly well
on its own — the collision exists only in a whole-suite collection. A staged run is not merely
slower to notice this; it is structurally blind to it. uv run pytest --collect-only -q over the
whole tree costs seconds and would have caught it, and is the cheap companion to a staged run.

Found, not fixed

The annotation workspace fetches an asset's content twice on every page load, same URL, back
to back. Filed separately as #572 and deliberately left for a frontend change.

Flags for Armando

• Item A was falsified, so the headline number did not move. A warm click still costs about
  4.6 s and roughly 95% of it is model(...). What that 4.5 s actually is remains undiagnosed;
  the dispatch explicitly ruled out chasing it here, so A warm suggest click costs about 4.8 s, and almost all of it is the model's own decode #571 will need a follow-up decision about
  whether to profile the forward pass. Related but not the same cost: the full-resolution mask
  conversion tracked in The suggest adapter converts a full-resolution mask into Python lists on every click #561 measures 2.1–24.9 ms at 854×480 and is not the problem at this
  resolution.
• One unexplained observation survives from the diagnosis. The single fastest decode ever
  measured, 72.7 ms, came from the one request that had performed its own encode immediately
  beforehand; every decode reading a cached embedding costs about 4.55 s. Sixtyfold, same model,
  same asset, same process. Now that the autograd explanation is dead, this is unexplained rather
  than merely unverified, and it looks like the most promising thread for the follow-up.
• The BoundedCache contract changed, from documented-not-thread-safe to internally locked.
  The reasoning is in the docstring. It is the one change here that no test can verify.
• Tier. The diff is src/visionset/inference/ and its tests — no frontend, no wire shape, no
  allowed_actions — so this is Tier A and the pull request is opened at completion, per the
  dispatch. Item C resolved to documentation, so no produced mask changes; had it changed the
  loaded class it would have been Tier B.
• The dispatch asked for the diagnosis report file to be deleted once its content lived in an
  issue. Agent-response.tmp.md is gone from every checkout, and A warm suggest click costs about 4.8 s, and almost all of it is the model's own decode #571 carries the findings.