Support filter conditions in dynamic search rules - #936
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #936 +/- ##
==========================================
- Coverage 89.78% 88.87% -0.91%
==========================================
Files 59 93 +34
Lines 1449 1996 +547
==========================================
+ Hits 1301 1774 +473
- Misses 148 222 +74 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/Contracts/DynamicSearchRule.php`:
- Around line 28-30: Define a named FilterValue PHPStan type alias in
DynamicSearchRule.php covering the supported scalar JSON values, then update the
FilterCondition.values annotation to use array<string, FilterValue> instead of
array<string, mixed>. Keep toArray() and fromArray() aligned with the narrowed
alias.
- Around line 36-39: Update the RawDynamicSearchRule PHPStan shape so
lastUpdatedAt accepts an explicit null in addition to a non-empty string and
omission, matching the server model’s optional timestamp representation. Add or
update the relevant fixture to cover a payload where lastUpdatedAt is explicitly
null.
🪄 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 Plus
Run ID: c61f953e-0487-4bf9-a62e-c7af61b6ae4b
📒 Files selected for processing (3)
src/Contracts/DynamicSearchRule.phptests/Contracts/DynamicSearchRuleTest.phptests/Contracts/UpdateDynamicSearchRuleQueryTest.php
| * @phpstan-type FilterCondition array{ | ||
| * values: array<string, mixed> | ||
| * } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo files of interest =="
git ls-files | rg '(^|/)DynamicSearchRule\.php$|composer\.(json|lock)$|phpstan' || true
echo
echo "== DynamicSearchRule outline/contents =="
if [ -f src/Contracts/DynamicSearchRule.php ]; then
wc -l src/Contracts/DynamicSearchRule.php
sed -n '1,220p' src/Contracts/DynamicSearchRule.php | cat -n
fi
echo
echo "== search for FilterCondition and values usages =="
rg -n "FilterCondition|FilterValue|values:\s*array<string,\s*mixed>|dynamic_search|dynamicSearch|toArray" -S .
echo
echo "== composer PHPStan config =="
if [ -f composer.json ]; then
sed -n '1,220p' composer.json | cat -n
fi
if [ -f phpstan.neon ] || [ -f phpstan.neon.dist ] || [ -f phpstan.neon.baseline ] || [ -f phpstan-baseline.neon ]; then
rg -n "filterCondition|FilterCondition|array<string, mixed>|strict_types|defineArrayShape|type" phpstan.neon* composer.json 2>/dev/null || true
fiRepository: meilisearch/meilisearch-php
Length of output: 30500
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== DynamicSearchRulesFilter =="
if [ -f src/Contracts/DynamicSearchRulesFilter.php ]; then
wc -l src/Contracts/DynamicSearchRulesFilter.php
cat -n src/Contracts/DynamicSearchRulesFilter.php
fi
echo
echo "== UpdateDynamicSearchRuleQuery dynamic search filter area =="
sed -n '80,150p' src/Contracts/UpdateDynamicSearchRuleQuery.php | cat -n
echo
echo "== DynamicSearchRules tests relevant area =="
sed -n '120,150p' tests/Endpoints/DynamicSearchRulesTest.php | cat -n
echo
echo "== Fetch Meilisearch v1.51 dynamic_search_rules.rs relevant snippets =="
python3 - <<'PY'
import urllib.request, re
url = "https://raw.githubusercontent.com/meilisearch/meilisearch/v1.51.0/crates/meilisearch-types/src/dynamic_search_rules.rs"
try:
text = urllib.request.urlopen(url, timeout=15).read().decode()
except Exception as e:
print(f"FETCH_ERROR: {type(e).__name__}: {e}")
raise SystemExit(0)
patterns = [
r"enum\s+DynamicSearchFilter[A-Za-z0-9_]*\b.*?(?:^$)",
r"struct\s+.*?DynamicSearch[A-Za-z0-9_]*\b.*?(?:^$)",
r"type\s+Dynamic[A-Za-z0-9_]*Value\b.*?(?:^$)",
]
print("URL:", url)
for pat in patterns:
print(f"\n-- matches for {pat[:80]} --")
matches = re.findall(pat, text, flags=re.M|re.S)
print("count", len(matches))
for m in matches[:10]:
print(m.replace("\n\n", "\n")[:1200])
PYRepository: meilisearch/meilisearch-php
Length of output: 4344
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== PHPStan config =="
cat -n phpstan.dist.neon
echo
echo "== Static parse-ish evidence for FilterCondition shape and toArray doc =="
python3 - <<'PY'
from pathlib import Path
p = Path("src/Contracts/DynamicSearchRule.php")
text = p.read_text()
print("FilterCondition has array<string, mixed>:", "values: array<string, mixed>" in text)
print("FilterValue alias present:", "`@phpstan-type` FilterValue" in text)
print("toArray returns RawDynamicSearchRule:", "`@return` RawDynamicSearchRule" in text and "return $this->raw;" in text)
PYRepository: meilisearch/meilisearch-php
Length of output: 852
Replace array<string, mixed> with a bounded filter-value type.
FilterCondition.values currently widens PHPStan analysis. Since dynamic search filter entries can be scalar JSON values, add a named FilterValue alias and use it here so toArray()/fromArray() stay precisely typed under the src/Contracts/**/*.php guideline.
Proposed shape
+ * `@phpstan-type` FilterValue scalar|array<array-key, mixed>|null
* `@phpstan-type` FilterCondition array{
- * values: array<string, mixed>
+ * values: array<string, FilterValue>
* }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * @phpstan-type FilterCondition array{ | |
| * values: array<string, mixed> | |
| * } | |
| * `@phpstan-type` FilterValue scalar|array<array-key, mixed>|null | |
| * `@phpstan-type` FilterCondition array{ | |
| * values: array<string, FilterValue> | |
| * } |
🤖 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/Contracts/DynamicSearchRule.php` around lines 28 - 30, Define a named
FilterValue PHPStan type alias in DynamicSearchRule.php covering the supported
scalar JSON values, then update the FilterCondition.values annotation to use
array<string, FilterValue> instead of array<string, mixed>. Keep toArray() and
fromArray() aligned with the narrowed alias.
Sources: Coding guidelines, MCP tools
| * @phpstan-type RawDynamicSearchRule array{ | ||
| * uid: non-empty-string, | ||
| * description?: string|null, | ||
| * lastUpdatedAt?: non-empty-string, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate target and related files =="
fd -a 'DynamicSearchRule\.php$|dynamic.*search.*rule|search_rules' . | sed 's#^\./##'
echo
echo "== Inspect DynamicSearchRule target =="
if [ -f src/Contracts/DynamicSearchRule.php ]; then
wc -l src/Contracts/DynamicSearchRule.php
cat -n src/Contracts/DynamicSearchRule.php
fi
echo
echo "== Search for fixture/usages =="
rg -n "lastUpdatedAt|DynamicSearchRule|RawDynamicSearchRule" tests src -S
echo
echo "== Composer/phpstan references =="
rg -n '"phpstan/phpstan"|phpstan' composer.json composer.lock || trueRepository: meilisearch/meilisearch-php
Length of output: 17060
🌐 Web query:
meilisearch v1.51 dynamic_search_rules.rs lastUpdatedAt skip_serializing_if
💡 Result:
In Meilisearch, the file crates/meilisearch-types/src/dynamic_search_rules.rs (and associated modules in milli) manages dynamic search rules [1][2][3]. The serde attribute skip_serializing_if = "Option::is_none" is a standard Rust Serde pattern used throughout these structs to ensure that optional fields (like priority or certain query conditions) are omitted from the serialized JSON output when they contain no value [4][5]. Regarding your query: 1. File Context: The dynamic_search_rules.rs file defines the schema for dynamic search rules, which allow you to influence search results based on specific conditions and actions [4][5]. 2. Serializing Behavior: The use of #[serde(default, skip_serializing_if = "Option::is_none")] on fields like priority is intended to keep the API response payloads clean by excluding null fields [4][5]. 3. Regarding lastUpdatedAt: There is no field named lastUpdatedAt inherently present in the dynamic_search_rules.rs source code in the Meilisearch repository [4][2][3]. If you are encountering this field in relation to Meilisearch, it is likely part of a custom implementation, a third-party plugin (such as those used for Jellyfin or other integrations), or a different data model that may be mistaken for a core Meilisearch dynamic search rule field [6][7][8]. If you are seeing errors or unexpected behavior involving lastUpdatedAt and Meilisearch, you may be looking at an external API response or a client-side wrapper that adds metadata fields not present in the native Meilisearch dynamic search rule schema [7].
Citations:
- 1: https://fossies.org/linux/meilisearch/crates/meilisearch-types/src/dynamic_search_rules.rs
- 2: meilisearch/meilisearch@0551db4
- 3: meilisearch/meilisearch@873969d
- 4: meilisearch/meilisearch@dbf6921
- 5: meilisearch/meilisearch@v1.40.0...v1.41.0
- 6: https://kotlin-sdk.jellyfin.org/dokka/jellyfin-model/org.jellyfin.sdk.model.api/-group-info-dto/last-updated-at.html
- 7: Plugin filters on type but never registers it as filterable: searches return 400 arnesacnussem/jellyfin-plugin-meilisearch#121
- 8: https://docs.rs/jellyfin_api/latest/src/jellyfin_api/lib.rs.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Fetch exact v1.51 dynamic_search_rules.rs =="
curl -fsSL https://raw.githubusercontent.com/meilisearch/meilisearch/v1.51.0/crates/meilisearch-types/src/dynamic_search_rules.rs \
| sed -n '250,330p'
echo
echo "== Fetch raw line context for lastUpdatedAt =="
curl -fsSL https://raw.githubusercontent.com/meilisearch/meilisearch/v1.51.0/crates/meilisearch-types/src/dynamic_search_rules.rs \
| rg -n "lastUpdatedAt|lastUpdated|updatedAt|skip_serializing_if|serde\\(.*default" || trueRepository: meilisearch/meilisearch-php
Length of output: 6246
🌐 Web query:
github.com/meilisearch/meilisearch v1.51 dynamic_search_rules.rs lastUpdatedAt
💡 Result:
The file dynamic_search_rules.rs is a component of the Meilisearch codebase that manages the definition, structure, and processing of dynamic search rules [1][2][3]. While there are multiple instances of files named dynamic_search_rules.rs within the repository (notably in crates/index-scheduler/src/, crates/meilisearch-types/src/, and crates/milli/src/ [3][4]), none of the public or standard definitions for the dynamic search rule structures in these files include a field explicitly named lastUpdatedAt [1][2]. The Meilisearch dynamic search rules feature underwent significant architectural changes around the v1.50.0 release [5][6]. The rules now handle fields such as precedence, active, conditions, and actions [1][2]. Changes to dynamic search rules are processed asynchronously via tasks, and responses for operations like PATCH or DELETE now return the associated task rather than the rule object itself [5][6]. If you are encountering a reference to lastUpdatedAt, it is likely part of an internal state management system, a specific database schema representation, or a custom implementation detail not present in the standard public API or primary rule definitions [1][7]. {confidence: 0.9}
Citations:
- 1: meilisearch/meilisearch@dbf6921
- 2: meilisearch/meilisearch@v1.40.0...v1.41.0
- 3: meilisearch/meilisearch@v1.49.0...v1.50.0
- 4: meilisearch/meilisearch@0551db4
- 5: chore(deps): update getmeili/meilisearch docker tag to v1.50.0 BaptTF/vps-infra#120
- 6: https://github.com/meilisearch/meilisearch/releases/tag/v1.50.0
- 7: https://github.com/meilisearch/meilisearch/blob/main/crates/index-scheduler/src/lib.rs
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Fetch upstream structs around DynamicSearchRule definitions =="
curl -fsSL https://raw.githubusercontent.com/meilisearch/meilisearch/v1.51.0/crates/meilisearch-types/src/dynamic_search_rules.rs \
| sed -n '1,140p'
echo
echo "== Fetch all lastUpdatedAt occurrences in v1.51 source =="
curl -fsSL https://raw.githubusercontent.com/meilisearch/meilisearch/v1.51.0/crates/meilisearch-types/src/dynamic_search_rules.rs \
| rg -n "lastUpdatedAt|lastUpdatedAt|lastUpdated" || true
echo
echo "== Check if v1.51 is latest or contains newer dynamic search rules in another file =="
curl -fsSL https://api.github.com/repos/meilisearch/meilisearch/git/matches/data?sha=v1.51.0&path=crates/meilisearch-types/src/dynamic_search_rules.rs \
| jq -r '.patch_url // empty' 2>/dev/null || true
git ls-data --tree v1.51.0 crates/meilisearch-types/src/dynamic_search_rules.rs || true
git ls-tree -r v1.51.0 --name-only crates/meilisearch-types/src/ | rg dynamic_search_rules || trueRepository: meilisearch/meilisearch-php
Length of output: 6134
Allow explicit null in RawDynamicSearchRule::lastUpdatedAt.
In the server model, lastUpdatedAt is Option<OffsetDateTime> with time::serde::rfc3339::option; this represents explicit null as well as an omitted field. Update the PHPStan shape and add an explicit-null fixture:
- * lastUpdatedAt?: non-empty-string,
+ * lastUpdatedAt?: non-empty-string|null,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * @phpstan-type RawDynamicSearchRule array{ | |
| * uid: non-empty-string, | |
| * description?: string|null, | |
| * lastUpdatedAt?: non-empty-string, | |
| * `@phpstan-type` RawDynamicSearchRule array{ | |
| * uid: non-empty-string, | |
| * description?: string|null, | |
| * lastUpdatedAt?: non-empty-string|null, |
🤖 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/Contracts/DynamicSearchRule.php` around lines 36 - 39, Update the
RawDynamicSearchRule PHPStan shape so lastUpdatedAt accepts an explicit null in
addition to a non-empty string and omission, matching the server model’s
optional timestamp representation. Add or update the relevant fixture to cover a
payload where lastUpdatedAt is explicitly null.
Source: MCP tools
1 participant
Summary
filteractivation condition to the dynamic search rule PHPStan shapelastUpdatedAtas a nullableDateTimeImmutableFixes #935
Validation
docker compose run --no-deps --rm package ./vendor/bin/phpunit tests/Contracts(170 tests, 238 assertions)lastUpdatedAtappears in both single-rule and list responsesI also attempted the full local suite in the repository Docker environment. It exceeded Composer's 300-second process timeout on the Windows bind mount before completion, so I am not representing that run as a pass; the focused contract suite above completed successfully.
AI disclosure
OpenAI Codex was used to draft the PHPDoc shape and tests. I reviewed the diff, checked it against the v1.51 server implementation, and verified it with the checks above.
Summary by CodeRabbit
New Features
Bug Fixes