Skip to content

feat(api) expose resolved model package metadata in HTTP inference responses - #2999

Merged
PawelPeczek-Roboflow merged 21 commits into
mainfrom
feat/http-resolved-model-metadata
Sep 18, 2026
Merged

PawelPeczek-Roboflow merged 21 commits into
mainfrom
feat/http-resolved-model-metadata

Conversation

@leeclemnet

@leeclemnet leeclemnet commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds resolved_model to structured inference responses so clients can identify the model that served each result. With USE_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

  • New feature (non-breaking change that adds functionality)

Testing

  • Unit tests (passed in gpu container)
  • Local staging server test on T4 VM, SDK v0 and v1 API
{
  "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

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code where necessary, particularly in hard-to-understand areas
  • My changes generate no new warnings or errors
  • I have updated the documentation accordingly (if applicable)

Additional Context

@github-actions

github-actions Bot commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

👋 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 vibes

Automated 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:

  • 🌱 Not sure the PR is in good shape yet? Keep it (or set it back) as a draft — drafts pause review, so you can push and iterate without burning credits on a moving target.
  • 💪 Feel strong about the contents? Mark it Ready for review and the reviewer will take a look.

However you get there, arrive prepared:

  • 🧱 Bring a SOLID, thorough PR. Point your local agent at our skills/ to tune it to our guidelines first — or, if you are one of those fabled carbon-based contributors, read them yourself. A half-baked diff costs exactly the same to review as a finished one.
  • ✅ Resolve every comment before you re-request review. Re-requesting with threads still open means paying twice for the same conversation.
  • 🔁 Do not use CI review as an inner loop for a local agent. The reviewer is not a step-by-step debugger — do the unfolding locally and arrive with the answer, not the search.
  • 🙋 If something looks off, ask a human. One question to a maintainer is cheaper and faster than three rounds of agent re-review chasing a misread.

Reviews are not free. A draft costs nothing to review; a Ready PR is a promise that it is worth reviewing.

  • Prefer to skip automated review entirely? Add the skip-claude-review label.

@leeclemnet
leeclemnet force-pushed the feat/http-resolved-model-metadata branch from b395723 to 96d51aa Compare September 16, 2026 14:36
@leeclemnet leeclemnet changed the title Expose resolved model package metadata in HTTP inference responses feat(api) expose resolved model package metadata in HTTP inference responses Sep 16, 2026
@leeclemnet
leeclemnet force-pushed the feat/http-resolved-model-metadata branch from 7c8ea3c to 6c091d2 Compare September 17, 2026 12:31
@leeclemnet leeclemnet added the claude-review Use to trigger AI review manually label Sep 17, 2026
@leeclemnet
leeclemnet marked this pull request as ready for review September 17, 2026 16:10
@github-actions

Copy link
Copy Markdown
Contributor

🤖 Claude review started at commit 01f0d929934353d17f0952ae735672065246e889.

New commits are not auto-reviewed. Add the claude-review label to request a re-review — the label is consumed when the review starts, so just add it again next time.

@github-actions github-actions Bot removed the claude-review Use to trigger AI review manually label Sep 17, 2026
@github-actions

Copy link
Copy Markdown
Contributor

@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 question

Is it intended that resolved_model is absent for models served by legacy classes when USE_INFERENCE_MODELS=True (the non-Windows default)?

Model.resolved_model (inference/core/models/base.py:113-120) branches on the global USE_INFERENCE_MODELS flag rather than on whether this instance is backed by an inference_models _model:

@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 Model subclasses that never set _model, so this returns None and no metadata is attached. Concretely, ("object-detection", "yolo-world") is registered unconditionally to the legacy YOLOWorld class (inference/models/utils.py:760) and is not remapped to an adapter in the flag-on block (utils.py:806+), because "yolo-world".startswith("yolov") is False. So the self._attach_resolved_model_metadata(response) call this PR added to inference/models/yolo_world/yolo_world.py is a no-op in the default config — yolo-world responses report no resolved_model at all, not even the model_id the flag-off path would supply. The same applies to any other architecture that stays on a legacy class under flag-on (e.g. legacy grounding-dino / object-detection / classification variants not in the remap list).

  • If this omission is intended (metadata only for inference_models-backed models under flag-on), no change is needed — but then the added attach calls in the legacy-only modules are dead in the default config, and the PR description's "Legacy models report their loaded model ID" holds only when the flag is off; worth a one-line note.
  • If it is not intended, the property should fall back to endpoint/model_id whenever _model is absent (gate on _model presence, not the global flag), and a test should cover a legacy-served model under USE_INFERENCE_MODELS=True (the current tests only exercise the legacy path with the flag disabled).

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 inference/core/models/base.py:113-120, inference/models/utils.py:756-760, and the flag-on remap block starting at utils.py:806. Unanswered questions may keep this PR out of a release.


New commits are not auto-reviewed — add the claude-review label to request a re-review (the label is consumed when the review starts, so just add it again next time).

Reviewed at HEAD: 01f0d92

@github-actions

Copy link
Copy Markdown
Contributor

Review summary

Skills: 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:

  • The inference_models auto-load cache path is well-guarded: a missing backend_type or missing/blank canonical_model_id invalidates the entry and forces a provider refresh (core.py:2290-2292, _verified_auto_cache_package_dir at core.py:2190-2197), so cast(str, cache_entry.canonical_model_id) at core.py:2442 is safe. Tests cover the fresh/cached/fallback cases.
  • Backward-compat of the flag-on rollout: when the pinned inference_models build predates this change, getattr(getattr(self, "_model", None), "resolved_model", None) returns None and nothing is attached — matches the staged-rollout plan in the description.
  • Serialization: HTTP responses go through orjson_response(..., exclude_none=True), so resolved_model (and its null sub-fields) are dropped when absent; the added null-compatibility tests guard the classes that switched base to InferenceResponse.
  • inference_models/docs/changelog.md has the required ## Unreleased entry (§5).
Minor doubts (non-blocking)
  • _is_response_dc_to_dict serializes r.resolved_model.model_dump() without exclude_none, so the instance-segmentation fast path emits model_package_id/backend/quantization: null where the pydantic+orjson path would drop them — a small cross-endpoint inconsistency.
  • The changelog calls ResolvedModelMetadata "public", but it is not re-exported from inference_models/inference_models/__init__.py (only defined in entities.py).

Reviewed at HEAD: 01f0d92

@leeclemnet

Copy link
Copy Markdown
Contributor Author

Is it intended that resolved_model is absent for models served by legacy classes when USE_INFERENCE_MODELS=True (the non-Windows default)?

Not intended. Fixed in f8c8c6e

@leeclemnet leeclemnet added the claude-review Use to trigger AI review manually label Sep 17, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🤖 Claude review started at commit f8c8c6e4b94f734d1056e697e10d0884a2f5e539.

New commits are not auto-reviewed. Add the claude-review label to request a re-review — the label is consumed when the review starts, so just add it again next time.

@github-actions github-actions Bot removed the claude-review Use to trigger AI review manually label Sep 17, 2026
@github-actions

Copy link
Copy Markdown
Contributor

😎 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>
@PawelPeczek-Roboflow PawelPeczek-Roboflow added the claude-review Use to trigger AI review manually label Sep 18, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🤖 Claude review started at commit 523bb936cfa2e5973e6ff80121b1493ac91c516a.

New commits are not auto-reviewed. Add the claude-review label to request a re-review — the label is consumed when the review starts, so just add it again next time.

@github-actions github-actions Bot removed the claude-review Use to trigger AI review manually label Sep 18, 2026
dkosowski87
dkosowski87 previously approved these changes Sep 18, 2026
@github-actions

Copy link
Copy Markdown
Contributor

😎 PR passes the vibe-check and trust-me-bro verification.

@PawelPeczek-Roboflow
PawelPeczek-Roboflow dismissed stale reviews from dkosowski87 and themself via 013ec33 September 18, 2026 16:12
@PawelPeczek-Roboflow
PawelPeczek-Roboflow merged commit f938e64 into main Sep 18, 2026
11 of 19 checks passed
@PawelPeczek-Roboflow
PawelPeczek-Roboflow deleted the feat/http-resolved-model-metadata branch September 18, 2026 16:36
PawelPeczek-Roboflow added a commit that referenced this pull request Sep 18, 2026
…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>
PawelPeczek-Roboflow added a commit that referenced this pull request Sep 19, 2026
* 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>
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.

3 participants