Add exhaustive facet count to facet search requests - #989
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesFacet Search Request
Estimated code review effort: 1 (Trivial) | ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.
🧹 Nitpick comments (1)
src/test/java/com/meilisearch/sdk/FacetSearchRequestTest.java (1)
18-25: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd coverage for explicit
false.
exhaustiveFacetCountusesBoolean, sonullmeans omitted andfalsemust remain serialized. The current test covers onlytrue. Add a test that asserts the key is present and its value isfalse.Proposed test
+ `@Test` + void serializesExplicitFalseExhaustiveFacetCount() { + FacetSearchRequest request = + new FacetSearchRequest("genres").setExhaustiveFacetCount(false); + + JSONObject json = new JSONObject(request.toString()); + + assertThat(json.has("exhaustiveFacetCount"), is(true)); + assertThat(json.getBoolean("exhaustiveFacetCount"), is(false)); + }🤖 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/test/java/com/meilisearch/sdk/FacetSearchRequestTest.java` around lines 18 - 25, Add a separate test alongside serializesExhaustiveFacetCount that creates a FacetSearchRequest with setExhaustiveFacetCount(false), parses its serialized JSON, and asserts exhaustiveFacetCount is present with a false value.
🤖 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.
Nitpick comments:
In `@src/test/java/com/meilisearch/sdk/FacetSearchRequestTest.java`:
- Around line 18-25: Add a separate test alongside
serializesExhaustiveFacetCount that creates a FacetSearchRequest with
setExhaustiveFacetCount(false), parses its serialized JSON, and asserts
exhaustiveFacetCount is present with a false value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5ee79792-2fa5-43e4-96b5-544eedd19262
📒 Files selected for processing (2)
src/main/java/com/meilisearch/sdk/FacetSearchRequest.javasrc/test/java/com/meilisearch/sdk/FacetSearchRequestTest.java
Co-Authored-By: Claude Opus 5 <[email protected]>
Summary
exhaustiveFacetCountfield toFacetSearchRequestfalse, which must serialize rather than be omittedCloses #843
Testing
./gradlew test— 117 tests, 0 failures, including JaCoCo coverage verificationbash ./scripts/lint.sh— passed, spotless reports no changes./gradlew integrationTest— completed withNO-SOURCE; no integration tests ran through that taskRun on Temurin 17, matching
sourceCompatibilityand CI. ThreeSettingsHandlerTestcases need aMeilisearch instance on
localhost:7700; they pass with one running and fail with a connectionerror without it.
Review follow-up
exhaustiveFacetCountis a boxedBoolean, sonullmeans "omit" whilefalsemust still besent. The original tests only covered
true, which would let a "falsy means omit" regression passunnoticed.
serializesExhaustiveFacetCountWhenFalsecloses that gap by asserting the key ispresent and its value is
false.AI assistance
AI assistance was used materially throughout. Codex helped inspect the repository and draft the
initial implementation and tests; Claude Code added the explicit-
falseregression test and ranthe verification listed above. The full diff was reviewed before submission. I can't attribute
every line to one tool or the other, so both are named rather than guessed at.
Summary by CodeRabbit