chore(inference): remove what the fast auto-labeling slices left behind - #465
Merged
Conversation
Three tests in tests/inference/test_provider.py were re-created in test_providers.py when the family resolver got its own file, and the originals were never removed — one of them character-for-character the same name. Each survivor is the stronger test: it drives provider_for through a real InferenceConnectionService rather than a hand-built model. best_of never read its `masks` argument; the choice comes entirely from the IoU head's scores. ConnectionAction had no importers and, unlike its three neighbours, was not package API either. Four comments no longer described the code beneath them: pyproject's claim that uv.lock does not carry extras (it carries them twice), AnnotationPage's note that the Inference surface had not shipped yet, and two ragged docstring wraps. No behaviour changes. Closes #459
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.
The auto-labeling feature is stable, so this removes what the slices that built it left behind.
cf. #464, which carries the full inventory and the items deliberately not acted on.No behaviour changes. Every item is a removal with zero references, a test deletion with a named surviving test proving the same failure mode, or a comment correction where the code is the authority.
Redundant tests
Three tests in
tests/inference/test_provider.py(#434) were re-created intests/inference/test_providers.py(#458) when the family resolver got its own file, and the originals were never removed — the first pair share a name character for character.test_provider.py::test_an_http_connection_is_refused_because_this_build_has_no_adaptertest_providers.py::test_an_http_connection_is_refused_because_this_build_has_no_adaptertest_provider.py::test_a_local_connection_without_weights_is_refused_by_statetest_providers.py::test_a_connection_without_weights_is_told_which_action_fixes_ittest_provider.py::test_the_connections_own_state_is_reported_before_the_machinestest_providers.py::test_a_missing_runtime_is_reported_after_the_connections_own_stateIn each pair the survivor is the stronger test: it drives
provider_forthrough a realInferenceConnectionServicerather than a hand-built model, and matches a longer substring of the message.The third is worth naming separately, because it did not test what its name claimed. It asserted that a not-set-up connection raises
InferenceConnectionNotSetUp— which is the second row's assertion — while stubbing nothing, so there was no second refusal for the order to be against. Its survivor stubsrequireto raise and then asserts both directions.test_provider.pykeeps everything unique to it: the unstubbed missing-extra refusal, theisinstance-against-Protocolcheck, bothmodel_refcases, and the wholeregions_fromhalf — including #434's named mutation test,test_duplicate_detections_are_suppressed_before_they_leave_the_adapter, which is untouched. Its module docstring now says which half ofprovider_forlives where.Deleting the three left
remote()and two error imports with no users; those went with them.Mutation verification
Committed before the first mutation. Each anchor asserted present exactly once before applying and its replacement asserted after; each reverted by
git apply -Ron its recorded patch, withgit status --porcelainempty afterwards.Mutation A —
provider_for's HTTP arm returns_local(...)instead of raisingInferenceConnectionNotRunnable.Mutation B — the
setup_state is not READYguard deleted from_local.Both named survivors turn red, and B catches a third test the deletions never covered.
Dead code
inference/sam_provider.py—best_of(iou_scores, masks)never readmasks; the choice comes entirely from the IoU head's scores. Flagged byruff --select ARG. Three call sites, all in this repository. Not part of theModelProviderport and not invisionset.inference.__all__.frontend/ui-core/src/data/inferenceQueries.ts—ConnectionActionhad no importers. Its three neighbours on the adjacent lines are each re-exported fromui-core/src/index.ts; this one was not, so it was not package API either. Anything needing it has it throughConnection["allowed_actions"].Comments that no longer described the code
pyproject.toml— "uv.lockdoes not carry extras". It carries them twice:local-inferenceunder[package.optional-dependencies], and onerequires-distrow per member with anextra ==marker, plusprovides-extras. The half of the sentence that was true is kept and sharpened: a plainuv syncdoes not install the extra — that takes--extra local-inference— but editing those lines without re-locking leaves the lockfile stale anduv lock --checkfails on it.AnnotationPage.tsx—onConfigureInferencewas documented as "expected to be absent for now: the Inference surface waits on Inference section — user-configured model connections #421's open rail question". That surface shipped in feat(app): Inference section, rail entry, download-size surface (#421 slice 2) #454, andfrontend/app/src/routes.tsx:331passes the prop. The prop stays optional for the real reason —ui-coreimports no router and cannot know whether its host has such a screen.inference/__init__.pyandtests/architecture/test_optional_runtime.py— two ragged docstring wraps left by the edit that addedtorchvisionto the enumerations.ruffdoes not reflow docstrings, which is why they passed every gate.Found, not fixed
detailfield is accepted, published, and ignored #463, filed —POST /inference/suggestaccepts adetailfield, validates it, publishes it inopenapi.json, and ignores it.suggest()takes the parameter and never reads it; the value that reachespolygon_fromis the adapter's construction-time default, andproviders._local()never passes one. Sodetail: 0.9anddetail: 0.01produce the same polygon. Not a cleanup item in either direction: deleting the field removes a published one, and making it work means carrying it per-request, because providers are pooled and shared across calls. No test covers the field reaching anything, which is why it shipped.visionset.inference.__all__(removing a name from__all__is a public-API change whatever the reference count says);masks.spans(), which could be derived frommasks.runs()but is not an identical helper and would edit the Point-prompted suggestions trace the topmost-leftmost blob, not the one under the click #461 fix's fresh code; the one-letter gap betweentest_provider.pyandtest_providers.py, since a rename is not a removal; and every protected test — the mutation-verification tests named in feat(inference): InferenceConnection entity, persistence, wire, REST, CLI (#421 slice 1) #430/feat(inference): ModelProvider port, local adapter, weight download (#418 slice 2) #434/feat(inference): SAM 2 point-prompt adapter and suggest route (#424 slice 3a) #448/feat(annotator): interactive suggest tool with ephemeral preview (#424 slice 3b) #451/feat(app): Inference section, rail entry, download-size surface (#421 slice 2) #454/fix(inference): sam2_video is a point-prompt model, and an unknown type refuses #458, fix(inference): sam2_video is a point-prompt model, and an unknown type refuses #458's refusal tests, the Point-prompted suggestions trace the topmost-leftmost blob, not the one under the click #461 blob-selection tests, and the contract and boundary suites.What the sweep did not find
No
TODO/FIXME/HACKmarkers on any feature surface, no commented-out code, no.tmp/.bak/spike-residue files tracked or untracked, no unreachable branches.ruff --select ERA,ARG,PIE,RUF,F401,F841,B007over every backend feature module reported exactly the two unused parameters — one of which is #463 — and nothing else.src/visionset/mcp/has no inference surface at all.Gate
Every stage run in this worktree, exit code recorded. Split by directory because of the ~10-minute command ceiling on this machine; the split is
ls tests/at run time, andtests/scriptsisnode --test, covered by the frontend stage.pytest tests/{architecture,cli,examples,fixtures,formats,inference,jobs,mcp,packaging,test_versioning.py}pytest tests/kernelpytest tests/serverruff check .ruff format --check .mypy src/visionsetlint-importsuv lock --checkscripts/check.sh frontendnode --testscript gatesscripts/check.sh generatedscripts/check.sh browserCounts rather than pass lines for the pytest stages:
addopts = "-q"inpyproject.tomlplus the explicit-qmakes-qq, which suppresses the summary line. The exit codes are the record.Closes #459
Closes #464