Repository navigation
feat(api) expose resolved model package metadata in HTTP inference responses - #2999
Conversation
|
👋 Thanks for the pull request! Here is how automated Claude review works here, so you spend credits (and reviewer time) wisely. 🚦 This PR is marked Ready for review, so automated Claude review will run — and every pass spends real credits. Warning 💸 The Claude reviewer bills in credits, not vibesAutomated review spins up a real agent that reads real code and spends real credits on every pass. It is glad to help — but it is not a rubber duck, a linter you poke in a loop, or a substitute for reading the contributing guide. Treat it like an expensive senior reviewer whose time you booked, and show up prepared. Draft when unsure, Ready when you mean it:
However you get there, arrive prepared:
Reviews are not free. A draft costs nothing to review; a Ready PR is a promise that it is worth reviewing.
|
b395723 to
96d51aa
Compare
7c8ea3c to
6c091d2
Compare
|
🤖 Claude review started at commit New commits are not auto-reviewed. Add the |
|
@leeclemnet — one question needs your answer before this can advance. ⏳ This PR is on hold pending your answers — the review will not advance to sign-off until the IMPORTANT question below is answered. IMPORTANT questionIs it intended that
@property
def resolved_model(self) -> Any:
if not USE_INFERENCE_MODELS:
model_id = getattr(self, "endpoint", None) or getattr(self, "model_id", None)
return ResolvedModel(model_id=model_id) if model_id is not None else None
return getattr(getattr(self, "_model", None), "resolved_model", None)Under the default flag-on config, many models are still served by legacy
Why this matters: a "not intended" answer confirms a real defect in the default production config affecting every legacy-served model; an "intended" answer resolves it with only a doc/wording tweak. Please look at New commits are not auto-reviewed — add the Reviewed at HEAD: 01f0d92 |
Review summarySkills: review-http-api-server, review-inference-models-pkg, review-legacy-models-registries, review-topic-backward-compat-and-versioning, review-topic-external-contract-and-silent-fallback, review-core-infra, review-topic-test-hygiene No confirmed blocking finding surfaced, but the PR is not merge-ready: one IMPORTANT question is open (see the on-hold comment above) and, until it is answered, the review will not sign off — its answer could itself turn into a defect. What I verified positively:
Minor doubts (non-blocking)
Reviewed at HEAD: 01f0d92 |
Not intended. Fixed in f8c8c6e |
|
🤖 Claude review started at commit New commits are not auto-reviewed. Add the |
|
😎 PR passes the vibe-check and trust-me-bro verification. |
…Cosmos reasoners
Two paths returned no `resolved_model`, and no test noticed.
Legacy classification. `ClassificationBaseOnnxRoboflowInferenceModel`
overrides `infer_from_request` without calling `super()` and never attached
the metadata. With USE_INFERENCE_MODELS=False, YOLOv8, ResNet, ViT and DINOv3
classifiers returned nothing while object detection on the same server
returned `{"model_id": ...}`. The override now attaches it before returning,
as `Model.infer_from_request` does. The only legacy-mode HTTP test used object
detection; classification is now covered through `/infer/classification`, a
batch request, and the legacy route, plus unit cases for all four classes,
single-label and multi-label, single image and list.
Cosmos action recognition. The auto-loader stamps `resolved_model` on the
object it returns. For a fine-tune that is a bare `Cosmos3EdgeReasoner`, which
`Cosmos3EdgeActionRecognition.from_reasoner` then wraps in a new object, so
the adapter found no metadata on `self._model`. The wrapper now takes the
reasoner's `resolved_model` in `__init__`, which covers every `from_reasoner`
return path. A loader that returns the wrapper itself still overwrites the
attribute after construction. The adapter test used a fake wrapper; a new one
goes through the real `_as_action_recognition_model` path with a real reasoner
instance.
Each new test set fails without its one-line production change.
Not changed here: PP-OCR, sam3_3d and the stub model still return no
`resolved_model`.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The package version on this branch is 0.37.2, so the entries that sat under "Unreleased" ship in that release: `resolved_model` metadata from `AutoModel.from_pretrained`, the same metadata on a `Cosmos3EdgeActionRecognition` wrapped over a loaded reasoner, and the YOLO-NAS TRT stream ordering fix. An empty "Unreleased" section stays on top, as after earlier releases. Also correct a heading. The `GitPython` lower-bound entry was added by the commit that bumped 0.37.0 to 0.37.1 (#2972) but was titled `0.37.0`, which left two `0.37.0` sections and no `0.37.1`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`inference_models` is 0.37.2 on this branch and that version is published on PyPI. The server reports `resolved_model` package metadata only when it runs that release, so move the pin in every requirements file that carries it: cpu, gpu, gpu.cu13, jetson and vino. Also set the package's own version in `inference_models/uv.lock` to 0.37.2, as the 0.37.1 bump (#2972) did. Only that line changes; the lock was not regenerated. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
🤖 Claude review started at commit New commits are not auto-reviewed. Add the |
|
😎 PR passes the vibe-check and trust-me-bro verification. |
013ec33
…olerating it Drop the optional-key helper. The 10 response assertions that receive `resolved_model` since #2999 now list it in the expected key set, so the suite also fails if the field disappears. Visualisation-only responses do not carry the field and stay unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Accept optional `resolved_model` field in hosted platform e2e tests #2999 added an optional top-level `resolved_model` field to HTTP inference responses. The hosted platform e2e tests assert exact response key sets, so 20 tests failed on staging once the new build was deployed there. The field is optional by design: some models do not report it, and staging and production are deployed independently, so the suite must pass against servers both with and without it. `required_response_keys()` in conftest strips the optional keys before the exact-set comparison and checks the shape of `resolved_model` when it is present. Unknown extra keys still fail the tests. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Expect `resolved_model` in hosted platform e2e responses instead of tolerating it Drop the optional-key helper. The 10 response assertions that receive `resolved_model` since #2999 now list it in the expected key set, so the suite also fails if the field disappears. Visualisation-only responses do not carry the field and stay unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
What does this PR do?
Adds
resolved_modelto structured inference responses so clients can identify the model that served each result. WithUSE_INFERENCE_MODELS=true, it reports the canonical model ID, package ID, backend, and quantization from the serving instance. Legacy models report their loaded model ID, with package details optional. Existing null fields and binary response formats are preserved.Related Issue(s): n/a
Type of Change
Testing
{ "inference_id": "7885cedc-2b7b-443b-aa9f-dd4dbb7ae47f", "time": 0.08670124830678105, "image": { "width": 2048, "height": 1371 }, "resolved_model": { "model_id": "lee-sandbox/chess-pieces-fmhpz-131-rfdetr-small-t1", "model_package_id": "c70f32369a54d61e06ef4e6b56c82524", "backend": "trt", "quantization": "fp16" }, "predictions": [ { "x": 1720.0, "y": 1007.0, "width": 144.0, "height": 180.0, "confidence": 0.9199306964874268, "class": "black-rook", "class_id": 5, "detection_id": "e51c2831-cc93-461e-b185-40acad81bbd7" }, ...Checklist
Additional Context
inference-modelsmetadata for fresh loads, cached loads, and successful package fallbacks.inference-modelschanges, bump the server dependency pin, rebuild the server image, then enableUSE_INFERENCE_MODELS.