Reviewing inference-models-pkg changes
Scope
Load when a PR changes any of:
inference_models/inference_models/**— model backends;models/base/*.pyprediction entities + base model classes;models/common/(onnx.py,trt.py,torch.py,rle_utils.py,roboflow/);models/auto_loaders/**;weights_providers/**;runtime_introspection/**;errors.py;configuration.py;constants.py;entities.py.inference_models/pyproject.toml,inference_models/uv.lock.inference_models/docs/**(changelog.md,errors/,models/,api-reference/,contributors/,how-to/).
OUT of scope (defer to sibling skills): inference/core/** model adapters, workflow blocks, the HTTP API, and the ENABLE_TENSOR_DATA_REPRESENTATION _tensor block pivot. On a cross-cutting PR, review only the inference_models side here.
Review checklist
Severity tags: BLOCK = must fix before merge; FLAG = raise it; NIT = optional.
- BLOCK — Functional change but no entry under
## Unreleasedininference_models/docs/changelog.md. Do not ask the contributor to bump a version. See Required companions. - BLOCK — Deps changed in
pyproject.tomlbutuv.locknot regenerated, or shared server deps not mirrored intorequirements/*.txt. - BLOCK — New model/backend not registered in
REGISTERED_MODELS, or missingfrom_pretrained(**kwargs), resolvableLazyClass, integration tests, ordocs/models/*. See Standards §1–2. - BLOCK — Prediction-entity field shape/semantics (
xyxy/class_id/confidence/mask) changed non-additively, or a backend /to_supervision()/docs/api-reference/*left un-updated. See Standards §3. - BLOCK —
errors.pyclass renamed/removed without updating theinference/corestep-error → HTTP mapping. See Standards §5. - BLOCK — Global torch state leak (autocast / dtype / grad context escaping its scope), or a global-mutating loader (
torch.jit.load/script) not serialized behind the shared lock. See Standards §4, §7. - FLAG — New load-time param passed positionally instead of threaded as an optional
**kwargsforwarded fromAutoModel/manager. See Standards §1. - FLAG — New/renamed error class missing its
help_urlanchor ordocs/errors/*section. See Standards §5. - FLAG — New env var not
INFERENCE_MODELS_*-named, not validated (InvalidEnvVariable), or not indocs/how-to/environment-variables.md. See Standards §6. - FLAG — ONNX/TRT batch bounds not respecting both min and max, missing TRT flags (
engine_host_code_allowed), or no graceful handling of inconsistent artefacts. See Standards §8. - FLAG — Runtime-introspection change can hard-crash negotiation on a missing CUDA/Jetson probe, or has no unit test. See Standards §9.
- FLAG — Third-party (
supervision/transformers/flash-attn) constructor/kwarg use notinspect.signature-guarded (relies on a pinned version instead). See Standards §3, §10. - NIT — New integration/GPU assertion uses brittle exact-equality instead of tolerant bounds (
>=, loosened tolerance). See Standards §11.
Not blocking
- Do NOT demand a speed-verification or prediction-correctness artefact from the diff — those live in the PR description / attached evidence, not the tree (see Required companions). Confirm they were provided; do not block on their absence from the diff.
- Do NOT block a doc-only or test-only PR for a missing changelog entry.
- Do NOT ask contributors to change
inference_models/pyproject.tomlorinference/core/version.py; maintainers version releases separately. - Do NOT treat a
supervision-constructor guard as an inference_models concern — that guard lives in the workflows viz block (#2467); on this surface #2467 was only additivekeypoints_detection.pyenrichment. - Do NOT block a clean revert (#2241, #2087) for removing registry/CI/deps/changelog together — that is the correct rollback shape.
Standards
from_pretrained+**kwargscontract. Every model class is loaded viaAutoModel.from_pretrained(models/auto_loaders/core.py) and must implement its ownfrom_pretrainedclassmethod accepting**kwargs, so negotiation-derived params can be forwarded. New load-time params are threaded as optional kwargs, not positional (e.g. the TorchScript lock in #2373,rf_detr_max_input_resolutionin #2145). Canonical:docs/contributors/adding-model.md.Register the model. New arch/task/backend combos land in
REGISTERED_MODELS(models/auto_loaders/models_registry.py) keyed by(ModelArchitecture, TaskType, BackendType)→LazyClass(module_name, class_name)orRegistryEntry. Feature-gated backends setRegistryEntry.supported_model_features/required_model_features(e.g.{"nms_fused"}). Auto-loader ranking (models/auto_loaders/ranking.py) puts ONNX above Torch (TRT=7,ONNX=6,TORCH=5) — do not silently reorder it (#2047).Prediction-entity shape stability. Entities live in
models/base/*.py—Detections(object_detection.py),InstanceDetections(instance_segmentation.py),KeyPoints(keypoints_detection.py),ClassificationPrediction/MultiLabelClassificationPrediction(classification.py). Each is a torch-tensor dataclass with ato_supervision()consumed byinference/coreadapters and Workflows. Do not mutatexyxy/class_id/confidence/masksemantics; add new representations asUnion/opt-in (RLE masks added asUnion[torch.Tensor, InstancesRLEMasks]+InstanceSegmentationMaskFormat = Literal["dense","rle"], #2260;KeyPointsgainedcovariance/detection_confidenceadditively, changelog0.29.7). Any field change touches every backend +to_supervision()+docs/api-reference/*.Device / dtype discipline. Tensors stay on-device until
to_supervision()does.cpu().numpy(). No leaked global autocast — a bf16torch.autocastthat poisoned the whole process was a real bug (#2363, inSam3TrackerPredictor.__init__, which entered autocast and never exited). Cast processor inputs to the resolved per-device dtype (Jetson bfloat16 fix, changelog0.29.4).Errors carry
help_urlto a real docs anchor.errors.pyis a tree underBaseInferenceModelsError(help_urloptional). New classes get adocs/errors/<page>.mdsection and are raised withhelp_url="https://inference-models.roboflow.com/errors/<page>/#<lowercased-classname>"(seeconfiguration.py, which raisesInvalidEnvVariablewith theruntime-environmentanchor). Theinference/corehandlers (workflows/execution_engine/v1/step_error_handlers.py,interfaces/http/error_handlers.py) map specific classes —ModelNotFoundError,UnauthorizedModelAccessError,ModelPackageRestrictedError,ModelPackageAlternativesExhaustedError— to HTTP/workflow errors; renaming/removing one breaks that mapping (#2180 wired violated-env-constraint reporting through it).Env-var config is validated and documented. New env flags use
INFERENCE_MODELS_*naming, are read/validated inconfiguration.py(raisingInvalidEnvVariableon bad values), and are documented indocs/how-to/environment-variables.md+ changelog (Triton RF-DETR flags, changelog0.29.6).Concurrency safety for global-mutating loaders.
torch.jit.load/torch.jit.scriptmutate a process-global, non-thread-safe registry and must be serialized viatorchscript_global_lock(models/common/torch.py), threaded from the model manager (#2373;lock=Noneis a no-op for single-threaded use). Any new global-state loader needs equivalent serialization.ONNX/TRT edge-cases. Batch fast paths respect both
min_batch_sizeandmax_batch_size— a fixed-batch model got under-sized batches when only max was checked (#2112). TRT artefacts are inconsistent: output-name verification is skipped for known-bad artefacts (#1790 YOLO-NAS) andengine_host_code_allowedis set where required (#1791 YOLOv10). Watch RF-DETR class remapping / TRT post-processing and cache-path sanitisation (#2075/#1619/#2029).Runtime introspection degrades gracefully. CUDA/Jetson/L4T detection (
runtime_introspection/core.py) is best-effort; a failed probe must not crash negotiation (CUDA-version fix #2515;JetsonTypeResolutionError, #1796). New device support must not change platform defaults (#1436 kept MPS non-default on macOS).Third-party API drift is guarded, not pinned in code.
transformers/flash-attndrift silently broke OwlV2 / GLM-OCR (#2257/#2266); guard HF-backed models against version drift. supervision-constructor changes belong to the workflows viz block, not here (#2467).Tolerant test assertions. New integration/e2e/GPU assertions use tolerant comparisons (
>=, loosened tolerance), not exact box-count/class-set equality, which flakes on GPU/ONNX (#2026, #1646, #1649, #1809, #1638, #1843). Keypoint/segmentation slicing uses model-declared slot counts, not-1inference (#1626 broke multi-class keypoints with5 + num_classes/view(..., -1, 3); fix uses fixed6:offset +_key_points_slots_in_prediction).
Required companions
Block a functional change unless it carries these contributor-owned companions (condition → required file):
- Any functional change → an entry under
## Unreleasedininference_models/docs/changelog.md, usingAdded/Changed/Fixed/Removedas appropriate. pyproject.tomldeps changed →inference_models/uv.lockregenerated (uv sync); shared server deps also mirrored intorequirements/*.txt(#2047, #2449 security bumps, #2415/#2510 dep updates).- New error class →
docs/errors/<page>.mdsection +help_urlanchor. - New model →
REGISTERED_MODELSentry +docs/models/<model>.md+ a license file for the model-family dir, listed indocs/models/index.md. - New env var →
docs/how-to/environment-variables.mdentry + changelog. - New model → tests in
inference_models/tests/:integration_tests/models/test_<model>_predictions_<backend>.py(mandatory — "the most important tests" perwriting-tests.md),e2e_platform_tests/(AutoModel path), andunit_tests/for pure logic (ranking, negotiation, post-processing, introspection).
Non-diff-checkable — verify via the PR description / attached evidence, not the diff (block approval if absent from the PR, not from the tree):
- Model registration acknowledgement → new-model registration details must be acknowledged by code-owners and confirmed done in the PR's GH comments.
- Speed verification → new-model contributions must attach speed evidence on NVIDIA L4 GPU and MacBook (MacBook optional if unsupported), comparing our implementation vs. the original. Absence blocks approval.
- Prediction correctness → new or substantial model changes must show predictions are reasonable — ideally integration tests asserting meaningful prediction qualities; for modifications, existing tests suffice or manual evidence against the unchanged code. Absence blocks approval.
Release notice
For every functional inference_models change, add a non-blocking top-level
review notice addressed to maintainers: inference-models requires a version
bump for release. This notice is required even when the contributor supplied
the changelog entry. Maintainers choose the version, update
inference_models/pyproject.toml and related lock-step pins/lockfiles, move the
entries into the final version section, and leave a fresh ## Unreleased
section for subsequent contributions.
Key files & entry points
models/auto_loaders/—core.py(AutoModel.from_pretrained),models_registry.py(REGISTERED_MODELS),auto_negotiation.py,ranking.py,model_cache_paths.py.models/base/*.py— prediction entities + base model classes +to_supervision().models/common/—onnx.py,trt.py,torch.py(torchscript_global_lock),rle_utils.py,roboflow/post_processing.py.errors.py,configuration.py,constants.py,entities.py;runtime_introspection/core.py;weights_providers/{core,roboflow,trt_manifest,entities}.py.inference_models/pyproject.toml,inference_models/uv.lock.inference_models/docs/{changelog.md,errors/,models/,api-reference/,contributors/adding-model.md,contributors/writing-tests.md,how-to/environment-variables.md}.- Core-side error mapping (out of scope to change here, but the contract to protect):
inference/core/workflows/execution_engine/v1/step_error_handlers.py,inference/core/interfaces/http/error_handlers.py.
Reference PRs
- #2373 — serialize TorchScript load/script behind a global lock.
- #2363 — eject leaked bf16 autocast from
Sam3TrackerPredictor. - #2467 — additive
keypoints_detection.pyenrichment (supervision-kwarg guard itself is out of scope). - #2112 — ONNX batch fast path respects min+max bounds.
- #1626 — multi-class keypoint post-processing slot indexing.
- #2260 — additive RLE mask representation + version/changelog companions.
- #2145 — RF-DETR max input resolution +
ModelPackageRestrictedError+ errors doc. - #2180 — report model-loading failures from violated env constraints (error → core mapping).
- #2047 — AutoLoader ranks ONNX over Torch (ranking + unit tests + docs).
- #2515 — graceful CUDA-version acquisition in runtime introspection.
- #2449 — security dep bumps with mirrored requirements + uv.lock + changelog.
Related topic skills
When the PR also exhibits these cross-cutting concerns, load the matching topic skill too (see each skill's description for the trigger):
review-topic-prediction-integrityreview-topic-backward-compat-and-versioningreview-topic-concurrency-and-resource-safetyreview-topic-external-contract-and-silent-fallbackreview-topic-test-hygiene