Repository navigation
Conversation
Co-authored-by: OpenAI Codex <codex@openai.com>
|
👋 Thanks for the pull request! Here is how automated Claude review works here, so you spend credits (and reviewer time) wisely. 🚧 Right now this is a draft, so automated Claude review is paused — nothing is being spent yet. Mark it Ready for review to trigger it. 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.
|
Co-authored-by: OpenAI Codex <codex@openai.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
|
🤖 Claude review started at commit New commits are not auto-reviewed. Add the |
| ) | ||
| self._model: OWLv2HF = singleton_instance.model | ||
|
|
||
| def run_tensor_native_inference(self, images, **kwargs) -> List[Detections]: |
There was a problem hiding this comment.
Medium: the tensor-native workflow path still fails because this adapter has no class_names attribute.
With ENABLE_TENSOR_DATA_REPRESENTATION=True, the object detection tensor blocks (workflows/roboflow_workflows/core_steps/models/roboflow/object_detection/v{1,2,3}_tensor.py) call run_tensor_native_inference and then immediately call self._model_manager.get_class_names(model_id) (v3_tensor.py, right after the inference call). That resolves to ModelManager.get_class_names (inference/core/managers/base.py:705-706), which returns model.class_names. InferenceModelsOwlV2Adapter never sets class_names, and Model/BaseInference don't define it either.
So a workflow that runs an uploaded OWLv2 model through the OD block in tensor mode now gets past inference and then fails with AttributeError. Before this PR it failed earlier with NotImplementedError. This method looks like it was added for exactly that path, so the path should work end to end.
Suggested fix: in __init__, expose the saved labels, e.g. self.class_names = list(self._model._class_names or []) (ideally through a public property on OWLv2HF). Also add a test that drives the tensor block, or at least get_class_names, for an OWLv2 package with saved prompts.
| class_names=None, | ||
| **kwargs, | ||
| ) -> List[Detections]: | ||
| threshold = confidence |
There was a problem hiding this comment.
Medium: string confidence modes crash the explicit-classes open-vocabulary path.
When class_names is None (the base OWLv2 model or a package without saved prompts, called with explicit classes), threshold stays as the raw confidence. Line 359 then evaluates scores > score_thresholds. For confidence="best" or "default", that is a tensor-vs-string comparison and raises TypeError.
This is newly reachable on the v2 stack. The new OWLv2HF infer action in inference_model_manager/.../registry_defaults.py uses _K_OD, which declares confidence as type "confidence" (string modes allowed). The fallback it replaces, OpenVocabularyObjectDetectionModel, used _P_FLOAT_CONFIDENCE. So a request like {"images": ..., "classes": ["dog"], "confidence": "best"} against owlv2/owlv2-large-patch14-ensemble used to be rejected by validation and now reaches the model and fails as a server error.
Suggested fix: always resolve through ConfidenceFilter, using the prompts as names when there are no saved labels:
threshold = ConfidenceFilter(
confidence=confidence,
recommended_parameters=self.recommended_parameters,
default_confidence=INFERENCE_MODELS_OWLV2_DEFAULT_CONFIDENCE,
).get_threshold(class_names or [])(get_threshold returns a scalar when there is no per-class map.) Alternatively, keep _P_FLOAT_CONFIDENCE on the action when no saved prompts exist. Add a unit test for confidence="best" / "default" with explicit classes and no saved prompts.
Claude review summaryStatus: blocked. 3 Medium findings must be fixed before sign-off. Skills: review-inference-models-pkg, review-legacy-models-registries, review-topic-legacy-new-stack-parity, review-topic-prediction-integrity, review-topic-backward-compat-and-versioning, review-topic-test-hygiene No dedicated surface skill covers Findings
Tests to add
Legacy→new-stack parity: 1 legacy hunk checked ( Minor doubts (non-blocking):
Commands that informed this review: New commits are not auto-reviewed. Add the Reviewed at HEAD: 7c83453 |
Release coordination
Reviewed at HEAD: 7c83453 |
Co-authored-by: OpenAI Codex <codex@openai.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
|
🤖 Claude review started at commit New commits are not auto-reviewed. Add the |
Claude review summaryStatus: all earlier findings are fixed in code. No new blocking findings in the delta ( Skills: review-inference-models-pkg, review-legacy-models-registries, review-topic-legacy-new-stack-parity, review-topic-prediction-integrity, review-topic-backward-compat-and-versioning, review-topic-test-hygiene No dedicated surface skill covers Earlier findings
Legacy→new-stack parity: 1 legacy hunk checked ( Minor doubts (non-blocking):
Commands that informed this review: Reviewed at HEAD: 0d06c1f |
Release coordination (updated)
Reviewed at HEAD: 0d06c1f |
|
😎 PR passes the vibe-check and trust-me-bro verification. |
|
Maintainer review requested. Final approval and merge remain in GitHub. |
Co-authored-by: OpenAI Codex <codex@openai.com>
MTaraszewski
left a comment
There was a problem hiding this comment.
Independent review of b43d3dbdcd8cc762caa5fce04a4f461ffe3398c0: three reproducible findings below.
Validation: 56 existing focused tests passed across saved-prompt behavior, reference-example coercion, embedding caches, the legacy adapter, and registry defaults. Separate behavioral probes reproduced all three findings. The reference-example and integer-confidence controls passed against the OWLv2 implementation from base revision 9d75f3baf45647a7d164a595af5fba2666b4f26c.
Verification used CPU and a tiny synthetic OWLv2 checkpoint. The class-filter and confidence probes replaced only the neural-network forward pass with known scores while preserving preprocessing, thresholding, NMS, and response handling. GPU and hosted staging were not exercised.
| def pre_process( | ||
| self, | ||
| images: Union[torch.Tensor, List[torch.Tensor], np.ndarray, List[np.ndarray]], | ||
| input_color_format=None, |
There was a problem hiding this comment.
[P1] Preserve the v2 reference-example action
Adding input_color_format to pre_process makes inference_model_manager.marshalling.with_decoded_color_format inject that keyword into reference-example requests too: its capability check inspects both the selected method and pre_process. infer_with_reference_examples does not accept the keyword, so the existing v2 serving path now raises TypeError: OWLv2HF.infer_with_reference_examples() got an unexpected keyword argument 'input_color_format' before inference.
Reproduced through ModelManager.process(..., action="infer_with_reference_examples", wire_marshalling=True) with encoded PNG input. The same request succeeds with the base OWLv2 code. The probe substitutes model acquisition and uses OpenCV to supply the production decoder's BGR uint8 representation; model-manager marshalling and action dispatch are real.
Please make the reference-example path correctly handle the color-format argument, or restrict injection to entry points that support it, and add a regression test through the serving boundary.
| confidence=confidence, | ||
| iou_threshold=iou_threshold, | ||
| max_detections=max_detections, | ||
| **kwargs, |
There was a problem hiding this comment.
[P2] Honor class_filter on image-only detection requests
The new image-only path forwards class_filter through **kwargs into OWLv2HF, whose preprocessing/postprocessing ignores it. make_response then returns every detected class, so ordinary object-detection requests violate their filtering contract.
Reproduced with a real ObjectDetectionInferenceRequest containing a base64 PNG, confidence=0.1, and class_filter=["label-b"]: adapter.infer_from_request(request) returns both label-a and label-b. Only neural-network forward was substituted with two known detections; request handling, preprocessing, NMS, and response serialization remained real.
Please apply the filter while constructing the legacy response, following the generic object-detection adapter, and cover it with a request-level regression test.
| if class_names is not None: | ||
| threshold = ConfidenceFilter( | ||
| confidence=confidence, | ||
| recommended_parameters=self.recommended_parameters, | ||
| default_confidence=INFERENCE_MODELS_OWLV2_DEFAULT_CONFIDENCE, | ||
| ).get_threshold(class_names) |
There was a problem hiding this comment.
[P2] Preserve integer confidence overrides
The new ConfidenceFilter path silently ignores integer thresholds: _resolve_fallback_threshold recognizes only float values as numeric overrides, so an integer falls through to the recommended/default confidence.
For a package with saved labels and recommended confidence 0.5, I reproduced model.infer(image, classes=["a", "b"], confidence=1) returning detections scored 0.9 and 0.8, while the otherwise identical call with confidence=1.0 returns none. The base OWLv2 implementation treats both numeric forms consistently.
Please normalize numeric thresholds before passing them to ConfidenceFilter, or extend the helper's numeric handling, and test integer/float equivalence on the saved-label path.
Keep color-format injection on the text-query action, honor legacy class filters, and normalize integer confidence overrides. Co-authored-by: OpenAI Codex <codex@openai.com>
|
Comments should be addressed in a865ac8 |
|
Marking as a draft as I clarify more about the intended usecase for this feature |
|
will target next week's release instead of this one. |
Note
Code and PR description are LLM-written. If this PR has been marked Ready For Review, code has
been reviewed and tested in staging by Kai
Diff breakdown
Approximate added and removed lines.
Description
Reuse
OWLv2HF, the existing OWLv2 request adapter, and architectureowlv2for uploaded full weights. Saved labels and prompts enable ordinary image-only detection requests. Explicit text queries and reference-example requests retain their existing paths. The model manager permits omitted classes only for OWLv2, and the model rejects requests without either explicit or saved prompts. Confidence filtering runs once before NMS for both scalar and per-class thresholds.Companion PRs: exporter #981, SDK #569, platform #18540. No separate custom model class or model type remains.
Type of change
How has this change been tested?
At
a865ac8, T4 reruns passed for direct inference, batches, inline and saved Workflows, class filters, and the Auto Label request format. All 18 comparisons againstb43d3dbmatched exactly in normal and tensor modes.The few-shot matrix matched pre-PR
9d75f3bin all 37 cases: 35 successful results, including 28 nonempty cases, and two matching preexisting errors. Box and confidence differences were zero. Encoded images, typed/dictionary references, batches, cold/warm caches, confidence boundaries, and detection limits were covered. A separate authenticated manager load and the in-process v2 parser passed.Negative-only requests still fail, and negative examples did not suppress detections in these cases. Both behaviors predate this PR. The earlier two-file staging upload preserved all 418 tensors. Hosted Auto Label jobs, annotation writeback, and customer-model accuracy remain unverified.
Any specific deployment considerations
Kai handles releases. Release #3294 prepares a new unpublished package version and synchronized runtime pins. In GitHub Actions, publish that version through Publish Inference Models Wheels to PyPi, then build both Build and Push CPU Container and Build and Push GPU Container from
kai/owlv2-staging-release. Use the same newcustom_tagand enableforce_push.The v2 serving stack also needs the model-manager registry change released with that stack. The regular image builds alone do not update it. Coordinate with the active rf-serving staging branch before deployment. Nothing was published or deployed by the agent.