Fix hybrid search metadata filters and CI failure detection - #252
Conversation
|
Warning Review limit reached
Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe change adds a shared pytest-summary validator for CI and updates three workflow checks. It also narrows hybrid-search SQL rewriting to quoted ChangesPytest summary validation
Hybrid-search SQL rewrite
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR fixes a SQL rewrite regression in hybrid search metadata filters (caused by an overly-broad backtick/JSON_EXTRACT regex) and hardens CI failure detection by replacing a brittle tail|grep ... || exit 0 pattern with a dedicated pytest-summary validation script, with unit tests covering both behaviors.
Changes:
- Tighten the
DBMS_HYBRID_SEARCH.GET_SQLpost-processing to unquote only fully backtick-quotedJSON_EXTRACT(...)expressions. - Replace CI’s pytest “last-line grep” fallback with
.github/scripts/check-pytest-summary.shand add unit tests for summary parsing. - Add regression tests ensuring adjacent identifier backticks are preserved during the SQL rewrite.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/pyseekdb/client/client_base.py |
Adds a constrained regex + helper to unquote only quoted JSON_EXTRACT(...) expressions in GET_SQL output. |
tests/unit_tests/test_hybrid_search_sql_rewrite.py |
Regression tests for the SQL rewrite to ensure identifiers remain properly quoted. |
.github/workflows/ci.yml |
Switches pytest-summary validation from inline greps to the new script. |
.github/scripts/check-pytest-summary.sh |
New “fail-closed” pytest summary validator to catch failures even when pytest exit codes are masked. |
tests/unit_tests/test_ci_pytest_summary.py |
Unit tests for the CI summary validation script behavior. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pyseekdb/client/client_base.py`:
- Around line 83-85: Update _QUOTED_JSON_EXTRACT_EXPRESSION_PATTERN so it
matches only a single JSON_EXTRACT call whose closing parenthesis, allowing
optional surrounding parentheses and whitespace, terminates the expression; do
not let trailing operators or other content inside the backticks match. Add a
regression test confirming `JSON_EXTRACT(metadata, '$.category') + (1)` remains
quoted and is not unquoted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6da4191a-7789-435d-be47-400e79a8f760
📒 Files selected for processing (5)
.github/scripts/check-pytest-summary.sh.github/workflows/ci.ymlsrc/pyseekdb/client/client_base.pytests/unit_tests/test_ci_pytest_summary.pytests/unit_tests/test_hybrid_search_sql_rewrite.py
| _QUOTED_JSON_EXTRACT_EXPRESSION_PATTERN = re.compile( | ||
| r"`(?P<expression>\s*\(*\s*JSON_EXTRACT\s*\([^`]*\)\s*\)*\s*)`", | ||
| re.IGNORECASE, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Restrict the matcher to one JSON_EXTRACT expression.
Line 84 lets [^]*consume text after theJSON_EXTRACTcall. For example, ``JSON_EXTRACT(metadata, '$.category') + (1)` `` matches and is unquoted. That content can be an ordinary backtick-quoted identifier.
Validate that the matching closing parenthesis terminates JSON_EXTRACT, after optional outer parentheses. Add a regression test that preserves this input.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pyseekdb/client/client_base.py` around lines 83 - 85, Update
_QUOTED_JSON_EXTRACT_EXPRESSION_PATTERN so it matches only a single JSON_EXTRACT
call whose closing parenthesis, allowing optional surrounding parentheses and
whitespace, terminates the expression; do not let trailing operators or other
content inside the backticks match. Add a regression test confirming
`JSON_EXTRACT(metadata, '$.category') + (1)` remains quoted and is not unquoted.
Backport #252 to release/1.4.0.
Summary
DBMS_HYBRID_SEARCH.GET_SQLrewrite to backtick-quotedJSON_EXTRACT(...)expressionspylibseekdb, but fail on positivefailed/error(s)counts and on missing successful summariesFixes #251.
Root cause
The previous regex could start at the closing backtick of one ordinary identifier and end at the opening backtick of another whenever an unquoted
JSON_EXTRACTappeared between them. That produced malformed SQL for metadata-filtered hybrid searches.The integration tests already caught this regression, but the CI fallback required a literal
=======in pytest's final line. When the summary format did not contain that exact marker, the|| exit 0branch marked the job successful even with failures.Validation
pytest tests/unit_tests/test_hybrid_search_sql_rewrite.py tests/unit_tests/test_ci_pytest_summary.py -q— 11 passedpytest tests/integration_tests/test_collection_hybrid_search.py -k embedded -q— 7 passedprek run -a— all checks passedLocal full-unit-suite note: 463 tests passed and 318 skipped; 6 existing sentence-transformer tests failed because the local
transformersinstallation imports an unavailablehuggingface_hub.is_offline_mode. This is unrelated to the changed files.Summary by CodeRabbit
Bug Fixes
Tests