Add support for Dynamic Search Rules#1238
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds Meilisearch Dynamic Search Rules support: new models and config path, four Client CRUD methods (list, get, create-or-update, delete), tests exercising all flows with a fixture toggling the feature, and documentation code samples. ChangesDynamic Search Rules API Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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 `@tests/client/test_client_dynamic_search_rules_meilisearch.py`:
- Around line 8-14: test_get_dynamic_search_rules_empty can be flaky because
dynamic rules created by other tests are not cleaned up; before asserting empty,
call client.get_dynamic_search_rules() and delete any existing rules to make the
test deterministic (e.g., iterate over the results from
client.get_dynamic_search_rules() and call the client delete method for each
rule such as client.delete_dynamic_search_rule(rule["name"]) or the appropriate
delete API on your client) then re-query with client.get_dynamic_search_rules()
and assert results is an empty list.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e80c737b-b318-4793-bd03-f9e75d18b8d0
📒 Files selected for processing (5)
.code-samples.meilisearch.yamlmeilisearch/client.pymeilisearch/config.pymeilisearch/models/dynamic_search_rule.pytests/client/test_client_dynamic_search_rules_meilisearch.py
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 `@tests/conftest.py`:
- Around line 359-370: The PATCH calls in the enable_dynamic_search_rules
fixture currently ignore responses; update both requests.patch invocations (the
one before yield and the one after yield) to capture their return values (e.g.,
resp = requests.patch(...)) and call resp.raise_for_status() immediately after
each call so any HTTP errors surface and prevent leaving the server in the wrong
state.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f57de259-5131-46d4-8f7f-8094dfd3606d
📒 Files selected for processing (4)
meilisearch/client.pymeilisearch/models/dynamic_search_rule.pytests/client/test_client_dynamic_search_rules_meilisearch.pytests/conftest.py
🚧 Files skipped from review as they are similar to previous changes (3)
- meilisearch/models/dynamic_search_rule.py
- meilisearch/client.py
- tests/client/test_client_dynamic_search_rules_meilisearch.py
|
@Strift I've fixed the CI failures:
|
|
@Strift Hi, when you have time, could you please take another look at this PR? Thanks! |
Strift
left a comment
Strift
left a comment
There was a problem hiding this comment.
This is great!
Please note the API for get_dynamic_search_rules() should accept offset, limit, and filter in the POST body; see docs.
Can you add this extra param? You can take example from the other list methods in the SDK, e.g., get_keys(parameters=None), get_indexes(parameters=None), get_tasks(parameters=None).
Accept offset, limit, and filter in the POST body, matching the pattern used by get_keys, get_indexes, and get_tasks.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/client/test_client_dynamic_search_rules_meilisearch.py (1)
36-37:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd count verification to the active filter test.
The filter test correctly asserts that all returned rules have
active=True, but it doesn't verify that exactly 2 active rules are returned (the count created by this test). If other tests leave active rules behind, the assertion passes without confirming the filter worked as intended.🔍 Suggested enhancement
# Test with filter on active status rules = client.get_dynamic_search_rules({"filter": {"active": True}}) +assert len(rules.results) == 2, "Expected 2 active rules from this test" assert all(r.active is True for r in rules.results)🤖 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 `@tests/client/test_client_dynamic_search_rules_meilisearch.py` around lines 36 - 37, The test for the active filter in the get_dynamic_search_rules call verifies that all returned rules have active=True, but fails to verify the count of returned rules. Add an assertion after the existing assertion to confirm that exactly 2 active rules are returned (since the test creates 2 active rules), preventing false positives if other tests leak active rules into the system.
🤖 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 `@tests/client/test_client_dynamic_search_rules_meilisearch.py`:
- Around line 32-33: The assertion that len(rules.results) == 2 after calling
client.get_dynamic_search_rules() with offset=1 assumes exactly 3 rules exist in
total, which is not guaranteed by test isolation. Before the offset assertion,
first verify the total count by calling client.get_dynamic_search_rules()
without offset and asserting that it returns exactly 3 results, ensuring the
pagination test makes deterministic assertions only when the required
precondition is met. This guards against prior tests that may have created rules
without proper cleanup.
---
Duplicate comments:
In `@tests/client/test_client_dynamic_search_rules_meilisearch.py`:
- Around line 36-37: The test for the active filter in the
get_dynamic_search_rules call verifies that all returned rules have active=True,
but fails to verify the count of returned rules. Add an assertion after the
existing assertion to confirm that exactly 2 active rules are returned (since
the test creates 2 active rules), preventing false positives if other tests leak
active rules into the system.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1c55d783-1ed5-44cc-beb6-f59f06da7de8
📒 Files selected for processing (2)
meilisearch/client.pytests/client/test_client_dynamic_search_rules_meilisearch.py
🚧 Files skipped from review as they are similar to previous changes (1)
- meilisearch/client.py
Prevents flaky test failure by checking the precondition (3 rules exist) before asserting offset behavior.
|
@Strift Thanks for the suggestion! This has been addressed in the latest commit.
A test for the new parameter support has also been added. |
|
Tests are failing again |
|
@Strift The CI failure was caused by |
2 participants
Add support for Dynamic Search Rules
Summary
Add support for Meilisearch v1.41's Dynamic Search Rules feature. This introduces four new methods on the
Clientclass for managing dynamic search rules, along with the corresponding models, tests, and code samples.Changes
New files
meilisearch/models/dynamic_search_rule.py— Pydantic models forDynamicSearchRuleandDynamicSearchRuleResultstests/client/test_client_dynamic_search_rules_meilisearch.py— 7 test cases covering CRUD operationsModified files
meilisearch/config.py— Addeddynamic_search_rules = "dynamic-search-rules"path constantmeilisearch/client.py— Added 4 methods:get_dynamic_search_rules,get_dynamic_search_rule,create_or_update_dynamic_search_rule,delete_dynamic_search_rule.code-samples.meilisearch.yaml— Added 4 code samples:list_dynamic_search_rules_1,get_dynamic_search_rule_1,patch_dynamic_search_rule_1,delete_dynamic_search_rule_1New methods
get_dynamic_search_rules()/dynamic-search-rulesget_dynamic_search_rule(uid)/dynamic-search-rules/{uid}create_or_update_dynamic_search_rule(uid, options)/dynamic-search-rules/{uid}delete_dynamic_search_rule(uid)/dynamic-search-rules/{uid}Related issue
Closes #1227
Summary
This PR adds Meilisearch v1.41 Dynamic Search Rules support to the Python SDK, allowing SDK users to pin documents at the top of search results via four new
Clientmethods. It introduces dedicated Pydantic models, adds comprehensive CRUD test coverage (including list pagination/filters), enables the feature in tests via an experimental-features fixture, and updates documentation code samples.Changes
New Methods in
Clientget_dynamic_search_rules(parameters: Optional[Mapping[str, Any]] = None)→ List rules via POST/dynamic-search-rules, sending an empty body when no parameters are provided; supportsoffset,limit, andfilterin the request body.get_dynamic_search_rule(uid)→ Retrieve a rule via GET/dynamic-search-rules/{uid}create_or_update_dynamic_search_rule(uid, options)→ Create/update via PATCH/dynamic-search-rules/{uid}delete_dynamic_search_rule(uid)→ Delete via DELETE/dynamic-search-rules/{uid}and return the HTTP status codeNew Models
Added
meilisearch/models/dynamic_search_rule.py:DynamicSearchRule(uidplus optionaldescription,priority,active,conditions, andactions)DynamicSearchRuleResults(resultsplus pagination fields:offset,limit,total)Test Coverage
Updated
tests/client/test_client_dynamic_search_rules_meilisearch.pywith 7 tests covering:offset/limit/filter(includingactivefiltering)conditions/actionsuid)204and subsequent not-found behavior)Also includes explicit assertions to reduce flaky pagination behavior.
Test Configuration
enable_dynamic_search_rulesfixture intests/conftest.pyto toggleexperimental-features.dynamicSearchRuleson/off around the tests.Configuration
Config.Paths.dynamic_search_rules = "dynamic-search-rules"inmeilisearch/config.py.Documentation / Code Samples
Updated
.code-samples.meilisearch.yamlwith:list_dynamic_search_rules_1get_dynamic_search_rule_1patch_dynamic_search_rule_1delete_dynamic_search_rule_1Related
Closes
#1227