[#84] feat(validate): add predicate value validation for SIMILAR operator#85
Conversation
Add _check_value() method to ValidateHandler to validate that SIMILAR operator predicates use a SimilarValue object with at least one of vector, text, or blob set. This implements the previously defined but unused INVALID_VALUE validation issue code. - Check SIMILAR predicate value is SimilarValue (not bare list/scalar) - Check SimilarValue has at least one search input field set - Include correctionHint with proper SimilarValue format examples - Add 6 unit tests covering valid/invalid value scenarios Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR closes a validation gap in adp.validate by adding predicate value validation for the SIMILAR operator, so malformed predicate values are caught during validation rather than failing later during execution.
Changes:
- Add
ValidateHandler._check_value()and invoke it during QUERY validation to emitINVALID_VALUEissues for invalid SIMILAR predicate values. - Add unit tests covering SIMILAR predicates with bare list/scalar values, empty SimilarValue, and valid vector SimilarValue inputs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/adp_hypervisor/handlers/validate.py |
Adds _check_value() and integrates SIMILAR predicate value validation into query validation. |
tests/unit/handlers/test_validate.py |
Adds a new test suite for SIMILAR predicate value validation scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| correction_hint=( | ||
| "Use a SimilarValue object: " | ||
| '{"vector": [<floats>], "top": <int>} ' | ||
| "for vector search, or " | ||
| '{"text": "<query>", "top": <int>} for text ' | ||
| "similarity search." | ||
| ), |
There was a problem hiding this comment.
The correction_hint suggests using {"text": "<query>"} (and implies blob similarity) for SIMILAR, but SimilarValue.text is explicitly rejected by its model validator (“not yet supported”), and there’s no backend handling for blob similarity today. This makes the hint misleading and may prompt clients to send a value that fails at request parsing (JSON-RPC invalid params) rather than returning a ValidateResult issue. Update the hint to only describe supported inputs (currently vector, and maybe top/threshold/distanceFunction if supported).
There was a problem hiding this comment.
Fixed. The first hint now only suggests {"vector": [<floats>], "top": <int>}. The not-yet-supported branches direct users to use vector-based search instead.
| ), | ||
| ) | ||
| ) | ||
| elif pred.value.vector is None and pred.value.text is None and pred.value.blob is None: |
There was a problem hiding this comment.
Can we add tests for text and blob?
There was a problem hiding this comment.
Done. Added test_similar_with_text_only_reports_not_yet_supported and test_similar_with_blob_only_reports_not_yet_supported, both asserting INVALID_VALUE with "not yet supported" in the message.
- Remove SimilarValue model validator that rejected text at parse time; defer validation to ValidateHandler._check_value() for proper INVALID_VALUE issues instead of JSON-RPC errors. - Tighten _check_value() to require vector for SIMILAR: text-only and blob-only now report 'not yet supported'; empty SimilarValue reports missing search input. - Fix correction_hint to only suggest vector-based search. - Add tests for text-only, blob-only, and threshold-only SimilarValue. - Update SimilarValue parse-time tests to reflect removed validator. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
What changes were proposed in this pull request?
Add
_check_value()method toValidateHandlerthat validates predicate values for the SIMILAR operator:SimilarValueobject, not a bare list or scalar. ReportsINVALID_VALUEwith acorrectionHintshowing the correctSimilarValueformat.SimilarValuemust have at least one ofvector,text, orblobset. ReportsINVALID_VALUEwith guidance on available search input options.Why are the changes needed?
Currently,
ValidateHandler._validate_query()checks field existence and operator compatibility but does not validate predicate values. This creates a gap whereadp_validatereturnsvalid: truefor a SIMILAR predicate with a bare array (e.g.[0.1, 0.9, 0.3]), butadp_executethen fails withExecutionFailedErrorbecause the backend requires aSimilarValueobject.The spec already defines
INVALID_VALUEinValidationIssueCodefor this purpose, but it was not implemented.Fix: #84
Does this PR introduce any user-facing change?
Yes.
adp_validatenow returnsINVALID_VALUEvalidation issues withcorrectionHintwhen:SimilarValuevalue (e.g. bare array, scalar)SimilarValuehas none ofvector,text, orblobsetPreviously these would pass validation silently and fail at execution.
How was this patch tested?
Added 6 unit tests in
TestPredicateValueValidation:test_similar_with_bare_list_reports_invalid_valuetest_similar_with_empty_similar_value_reports_invalid_valuetest_similar_with_valid_vector_valuetest_similar_with_valid_vector_and_toptest_similar_with_scalar_value_reports_invalid_valuetest_similar_value_check_with_bare_predicateAll 632 unit tests pass locally. CI checks (ruff, black, mypy) all pass.