Skip to content

Conversation

@kumarUjjawal
Copy link
Contributor

@kumarUjjawal kumarUjjawal commented Nov 3, 2025

Pull Request

Related issue

Fixes #664

What does this PR do?

PR checklist

Please check if your PR fulfills the following requirements:

  • Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
  • Have you read the contributing guidelines?
  • Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

Summary by CodeRabbit

  • New Features
    • Results now include a flag indicating whether facet counts are exact or estimated, so callers can know when displayed facet totals are definitive.
    • Added an optional query setting and a builder-style method to request exhaustive (exact) facet counts when performing searches, giving users control over accuracy vs. performance.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 3, 2025

Walkthrough

Added optional exhaustive facet counting: SearchQuery can request exhaustive facet counts via exhaustiveFacetCount, and SearchResults exposes the server's exhaustiveFacetCount in exhaustive_facet_count: Option<bool>; a builder method sets the query field and SearchQuery::new initializes it to None.

Changes

Cohort / File(s) Summary
Search API — exhaustive facet flag
src/search.rs
Added pub exhaustive_facet_count: Option<bool> to SearchQuery<'a, Http> (serialized as exhaustiveFacetCount) and initialized it to None in SearchQuery::new(). Added builder pub fn with_exhaustive_facet_count<'b>(&'b mut self, exhaustive: bool) -> &'b mut SearchQuery<'a, Http> to set the flag. Added pub exhaustive_facet_count: Option<bool> to SearchResults<T> to surface the response field.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Client code
    participant Builder as SearchQuery builder
    participant HTTP as HTTP layer
    participant Server as Meilisearch server

    rect rgb(240,248,255)
    Client->>Builder: create SearchQuery
    Builder->>Builder: with_exhaustive_facet_count(true)
    end

    rect rgb(245,255,240)
    Builder->>HTTP: serialize query (includes exhaustiveFacetCount)
    HTTP->>Server: POST /search { ..., "exhaustiveFacetCount": true }
    Server-->>HTTP: 200 OK { ..., "exhaustiveFacetCount": true, "facetsDistribution": {...} }
    HTTP-->>Client: deserialize -> SearchResults.exhaustive_facet_count = Some(true)
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Single-file change with consistent pattern.
  • Review focus: correct serde rename to exhaustiveFacetCount, builder signature/borrow lifetimes, and that SearchResults deserializes the response field.

Possibly related PRs

Poem

🐰
I hopped through queries, keen and neat,
Toggling counts with eager feet.
Now facets tell their truth at last,
No guesses left from records past.
I nibble code — exactness sweet. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "Add exhaustive facet search" is concise and clearly describes the main change introduced in this PR. It directly aligns with the primary objective of implementing support for exhaustive facet count functionality as part of the Meilisearch v1.14 update. The title is specific enough that a teammate reviewing the commit history would understand the feature being added, and it accurately reflects the code changes that introduce the exhaustive_facet_count field and associated builder method to the search query and results structures.
Linked Issues Check ✅ Passed The pull request successfully implements the requirement stated in linked issue #664 to "Update the search method to handle the exhaustiveFacetCount parameter." The implementation adds the exhaustive_facet_count field to both the SearchQuery struct (allowing clients to request exhaustive facet counts) and the SearchResults struct (allowing responses to indicate whether counts are exhaustive). The builder method with_exhaustive_facet_count provides a convenient API for setting this parameter, and the serialization is properly configured to use the camelCase name exhaustiveFacetCount as expected by the Meilisearch API. All coding requirements from the linked issue are met.
Out of Scope Changes Check ✅ Passed All changes in this pull request are directly scoped to implementing exhaustive facet search support as defined in linked issue #664. The modifications are focused exclusively on adding the exhaustiveFacetCount parameter handling through struct fields, initialization updates, and builder method implementation. No unrelated changes, refactoring, or functionality outside the stated objective have been introduced. The changeset is minimal and purposeful, containing only what is necessary to support the new feature.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e90a3c2 and 1036295.

📒 Files selected for processing (1)
  • src/search.rs (4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-12T13:28:23.700Z
Learnt from: LukasKalbertodt
Repo: meilisearch/meilisearch-rust PR: 625
File: src/search.rs:368-370
Timestamp: 2025-06-12T13:28:23.700Z
Learning: In the Meilisearch Rust client, `SearchQuery` serializes its per-query federation settings under the key `federationOptions`; only the top-level multi-search parameter is named `federation`.

Applied to files:

  • src/search.rs

@codecov
Copy link

codecov bot commented Nov 3, 2025

Codecov Report

❌ Patch coverage is 88.09524% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.13%. Comparing base (e90a3c2) to head (53a95b7).

Files with missing lines Patch % Lines
src/search.rs 88.09% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #726      +/-   ##
==========================================
- Coverage   86.18%   86.13%   -0.06%     
==========================================
  Files          20       20              
  Lines        6263     6305      +42     
==========================================
+ Hits         5398     5431      +33     
- Misses        865      874       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/search.rs (1)

1992-2026: Strengthen the test assertion to verify the actual value.

The test correctly handles forward compatibility by accepting BadRequest errors from servers that don't support this field yet. However, when the field is present, the test only verifies it's Some(...) without checking the actual boolean value.

Apply this diff to verify the returned value matches the request:

         match res {
             Ok(results) => {
-                assert!(results.exhaustive_facet_count.is_some());
+                assert_eq!(results.exhaustive_facet_count, Some(true), 
+                    "Expected exhaustive_facet_count to be Some(true) when requested");
                 Ok(())
             }

This ensures the server actually honors the request rather than just returning any value.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 29b9877 and 53a95b7.

📒 Files selected for processing (1)
  • src/search.rs (6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-12T13:28:23.700Z
Learnt from: LukasKalbertodt
Repo: meilisearch/meilisearch-rust PR: 625
File: src/search.rs:368-370
Timestamp: 2025-06-12T13:28:23.700Z
Learning: In the Meilisearch Rust client, `SearchQuery` serializes its per-query federation settings under the key `federationOptions`; only the top-level multi-search parameter is named `federation`.

Applied to files:

  • src/search.rs
🧬 Code graph analysis (1)
src/search.rs (3)
src/client.rs (4)
  • None (1446-1446)
  • None (1525-1525)
  • index (424-426)
  • new (54-66)
src/indexes.rs (26)
  • client (186-188)
  • client (232-234)
  • client (320-322)
  • client (376-378)
  • client (428-430)
  • client (473-475)
  • client (522-525)
  • client (556-558)
  • client (634-636)
  • client (700-702)
  • client (970-972)
  • client (1038-1040)
  • client (1089-1091)
  • client (1133-1135)
  • client (1186-1188)
  • client (1245-1247)
  • index (2171-2171)
  • index (2188-2189)
  • index (2226-2226)
  • index (2298-2298)
  • index (2326-2326)
  • index (2352-2352)
  • index (2379-2379)
  • new (81-89)
  • new (1783-1790)
  • new (1994-2000)
src/documents.rs (8)
  • index (62-64)
  • index (161-161)
  • index (350-350)
  • index (487-487)
  • setup_test_index (423-451)
  • new (89-94)
  • new (209-218)
  • new (367-372)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: integration-tests
🔇 Additional comments (6)
src/search.rs (6)

118-120: LGTM! Field correctly implements exhaustive facet count in search results.

The field name, type, and documentation are appropriate. With the struct-level #[serde(rename_all = "camelCase")] attribute, this will correctly serialize/deserialize as exhaustiveFacetCount.


414-419: LGTM! Query parameter correctly configured for exhaustive facet counting.

The field is properly configured with serde attributes to skip serialization when unset, and the documentation clearly explains its purpose and default behavior.


466-466: LGTM! Proper initialization in constructor.

The field is correctly initialized to None, consistent with other optional query parameters.


735-742: LGTM! Builder method correctly implements the pattern.

The method follows the established builder pattern and is consistent with the existing FacetSearchQuery::with_exhaustive_facet_count method.


1113-1113: LGTM! Import required for the new test.

The ErrorCode import is needed for error handling in the test_search_with_exhaustive_facet_count test.


2028-2048: LGTM! Excellent serialization test.

This unit test provides valuable verification that the field serializes to the correct camelCase name exhaustiveFacetCount, which is critical for API compatibility with Meilisearch v1.14. The test would catch any naming regressions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[v1.14] Add exhaustive facet search

1 participant