Skip to content

Conversation

@ErrorProne
Copy link

@ErrorProne ErrorProne commented Nov 16, 2025

Pull Request

Related issue

What does this PR do?

  • Adds the Hybrid search option to IndexSearchRequest which is used in multi search queries
  • This is basically a clone of the hybrid implementation in the SearchRequest class

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

    • Added hybrid search configuration to requests, allowing users to provide semantic ratio and embedder settings for combined keyword + semantic search; hybrid settings are included in request serialization.
  • Tests

    • Added tests verifying hybrid configuration is correctly represented in serialized request output and stringified form across multiple parameter combinations.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 16, 2025

Walkthrough

Adds a protected Hybrid field to IndexSearchRequest, updates its toString() to serialize a "hybrid" JSON entry when present (using hybrid.toJSONObject()), updates constructor Javadoc defaults, and adds unit tests validating JSON output for several hybrid configurations.

Changes

Cohort / File(s) Summary
IndexSearchRequest update
src/main/java/com/meilisearch/sdk/IndexSearchRequest.java
Added protected Hybrid hybrid, imported com.meilisearch.sdk.model.Hybrid, updated constructor Javadoc default mention, and extended toString() to include a "hybrid" JSON property (uses hybrid.toJSONObject() when non-null, otherwise null).
Unit tests for hybrid serialization
src/test/java/com/meilisearch/sdk/IndexSearchRequestTest.java
Added IndexSearchRequestTest with three tests: toStringWithHybridUsingBuilder(), toStringWithHybridAndOtherParameters(), and toStringWithHybridOnlyEmbedder() asserting exact JSON strings and verifying Hybrid getters (semanticRatio, embedder).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Test as Test
  participant Req as IndexSearchRequest
  participant Hybrid as Hybrid

  Note over Test,Req: Build or instantiate IndexSearchRequest (may include Hybrid)
  Test->>Req: call toString()
  alt hybrid != null
    Req->>Hybrid: hybrid.toJSONObject()
    Hybrid-->>Req: JSONObject (embedder, semanticRatio, ...)
    Req-->>Test: JSON string includes "hybrid": JSONObject
  else hybrid == null
    Req-->>Test: JSON string includes "hybrid": null
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Inspect public API surface: added protected field may need getters/setters or constructor exposure.
  • Verify JSON formatting/ordering in toString() and the brittle exact-string tests.

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • brunoocasali

Poem

🐰 I nibble code beneath the moonlit sky,

A Hybrid hop and JSON lullaby,
Fields align with tidy, gentle care,
Tests applaud the structure that we share,
A rabbit scans the diff and gives a joyful try.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding Hybrid parameter support to IndexSearchRequest for multi-search functionality, which is directly reflected in the code changes.
✨ 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: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 45861af and fbd8ccc.

📒 Files selected for processing (2)
  • src/main/java/com/meilisearch/sdk/IndexSearchRequest.java (3 hunks)
  • src/test/java/com/meilisearch/sdk/IndexSearchRequestTest.java (1 hunks)
🔇 Additional comments (5)
src/main/java/com/meilisearch/sdk/IndexSearchRequest.java (2)

3-3: LGTM!

The import is necessary for the new hybrid field and follows the existing import pattern.


42-42: LGTM!

The field declaration is consistent with other protected fields in the class, and the Lombok annotations will automatically generate the necessary getters and setters.

src/test/java/com/meilisearch/sdk/IndexSearchRequestTest.java (3)

11-26: Verify test expectations match actual JSON output.

This test validates the builder pattern and getter methods. The getter assertions (lines 24-25) are good practice. However, the expected JSON string at line 20 may not match the actual output due to the serialization concerns flagged in IndexSearchRequest.java (lines 115-117).

Specifically:

  • The expected string contains only q and hybrid fields
  • The actual output will likely include additional fields like offset, limit, etc., with null values due to the .put() calls in the toString() method

This concern is addressed by the verification script in the previous review comment for IndexSearchRequest.java.


28-45: Field ordering in expected JSON may not match actual output.

The expected JSON string at line 43 shows the field order as q, hybrid, offset, limit. However, in the implementation (IndexSearchRequest.java lines 115-117), hybrid is added after all other fields, so the actual output will have hybrid at the end, not between q and offset.

This concern is addressed by the verification script in the review comment for IndexSearchRequest.java (lines 115-117).


47-56: Good test coverage for partial Hybrid configuration.

This test validates the scenario where only the embedder is set (without semanticRatio), which is valuable for ensuring partial configuration works correctly. The use of constructor and setter methods (instead of builder) also provides good test coverage.

However, the same JSON output concerns apply here as in the other tests (see comments for lines 11-26 and 28-45).

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: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7c85e6d and ad390e6.

📒 Files selected for processing (2)
  • src/main/java/com/meilisearch/sdk/IndexSearchRequest.java (3 hunks)
  • src/test/java/com/meilisearch/sdk/IndexSearchRequestTest.java (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/test/java/com/meilisearch/sdk/IndexSearchRequestTest.java
🔇 Additional comments (3)
src/main/java/com/meilisearch/sdk/IndexSearchRequest.java (3)

3-3: LGTM!

The import is necessary for the new Hybrid field and follows the existing import structure.


42-42: LGTM!

The field declaration follows the established pattern for protected optional fields in this class.


50-50: LGTM!

The Javadoc update correctly documents the default value of the new hybrid field.

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.

1 participant