-
Notifications
You must be signed in to change notification settings - Fork 103
Add queryVector to search responses and support index renaming #704
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds index renaming support (via swap-indexes with optional rename and via index update with new uid) and exposes queryVector in search results. Introduces lifetime adjustments to several query builders. Updates docs with a new REST example and adds tests for renaming and queryVector handling. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant SDK as Rust SDK
participant MS as Meilisearch
rect rgb(235,245,255)
note right of Dev: Rename via swap-indexes
Dev->>SDK: swap_indexes([("indexA","indexB")], rename=true)
SDK->>MS: POST /swap-indexes [{ indexes: ["indexA","indexB"], rename: true }]
MS-->>SDK: Task enqueued/completed
SDK-->>Dev: Task info
end
rect rgb(245,235,255)
note right of Dev: Rename via index update
Dev->>SDK: IndexUpdater::new("indexA").with_uid("indexB").update()
SDK->>MS: PATCH /indexes/indexA { uid: "indexB" }
MS-->>SDK: Task enqueued/completed
SDK-->>Dev: Task info
end
sequenceDiagram
autonumber
actor Dev as Developer
participant SDK as Rust SDK
participant MS as Meilisearch
rect rgb(235,255,245)
note right of Dev: Search with retrieveVectors
Dev->>SDK: index.search().with_retrieve_vectors(true).with_vector(qv).execute()
SDK->>MS: POST /indexes/:uid/search { retrieveVectors: true, vector: [...] }
MS-->>SDK: { hits: [...], queryVector: [...] }
SDK-->>Dev: SearchResults { query_vector: Some([...]) }
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
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.
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 (6)
src/client.rs (1)
1358-1387
: Rename via swap test looks solid; consider asserting source no longer exists.Optional: after asserting the target exists, also verify the old uid is gone to fully cover the rename path.
@@ - let new_index = client.get_index(&to).await?; + let new_index = client.get_index(&to).await?; assert_eq!(new_index.uid, to); + // Optional: old uid should no longer resolve + assert!(client.get_raw_index(&from).await.is_err());src/search.rs (2)
124-134
: Add alias for older servers that return "vector" at the top level.Earlier engine responses echoed the request vector under "vector". Adding alias = "vector" keeps the SDK backward-compatible while supporting new names like "queryVector". (github.com)
#[serde( rename = "queryVector", alias = "query_vector", alias = "queryEmbedding", alias = "query_embedding", + alias = "vector", skip_serializing_if = "Option::is_none" )] pub query_vector: Option<Vec<f32>>,
2030-2066
: Test is good; avoid noisy stderr dump in CI.Consider guarding the raw-response eprintln! behind an env flag to keep CI logs clean while preserving the helpful debug path.
- if results.query_vector.is_none() { + if std::env::var("MSDK_DEBUG_RAW_SEARCH").ok().as_deref() == Some("1") + && results.query_vector.is_none() + { use crate::request::Method; let url = format!("{}/indexes/{}/search", index.client.get_host(), index.uid); let raw: serde_json::Value = index .client .http_clientsrc/indexes.rs (3)
279-281
: Good API polish:search()
now returnsSearchQuery<'_, Http>
.Consistent with other query builders. Consider a follow-up sweep to explicitly use lifetimed types in request turbofish where present (e.g.,
&SearchQuery<'_, Http>
) for clarity, though not strictly required if CI is green.
1836-1841
: Name clarity: provide an aliaswith_new_uid
to avoid ambiguity.
with_uid
can be misread as setting the current uid. Add a clearer alias without breaking changes./// Define a new `uid` to rename the index. pub fn with_uid(&mut self, new_uid: impl AsRef<str>) -> &mut IndexUpdater<'a, Http> { self.new_uid = Some(new_uid.as_ref().to_string()); self } + + /// Alias for `with_uid` with clearer intent. + pub fn with_new_uid(&mut self, new_uid: impl AsRef<str>) -> &mut IndexUpdater<'a, Http> { + self.with_uid(new_uid) + }
2236-2268
: Strengthen rename test assertions and cleanup.Assert the old index no longer exists post-rename and defensively clean it up if it still does.
// New index should exist let new_index = client.get_index(&to).await?; assert_eq!(new_index.uid, to); + // Old index should no longer exist + let old_index = client.get_index(&from).await; + assert!(old_index.is_err(), "old uid still resolves after rename"); + // cleanup new_index .delete() .await? .wait_for_completion(&client, None, None) .await?; + + // defensive cleanup if rename semantics change + if let Ok(idx) = client.get_index(&from).await { + idx.delete() + .await? + .wait_for_completion(&client, None, None) + .await?; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.code-samples.meilisearch.yaml
(1 hunks)src/client.rs
(5 hunks)src/documents.rs
(3 hunks)src/indexes.rs
(5 hunks)src/search.rs
(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-12T13:28:23.700Z
Learnt from: LukasKalbertodt
PR: meilisearch/meilisearch-rust#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/client.rs
🧬 Code graph analysis (4)
src/search.rs (3)
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
(2166-2166)index
(2183-2184)index
(2221-2221)index
(2281-2281)index
(2309-2309)index
(2335-2335)index
(2362-2362)new
(81-89)new
(1783-1790)new
(1989-1995)src/client.rs (3)
index
(422-424)new
(52-64)get_host
(223-225)src/request.rs (3)
query
(39-47)request
(71-91)request
(151-163)
src/client.rs (2)
meilisearch-test-macro/src/lib.rs (1)
meilisearch_test
(14-201)src/indexes.rs (16)
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)
src/documents.rs (3)
src/client.rs (2)
new
(52-64)index
(422-424)src/indexes.rs (10)
new
(81-89)new
(1783-1790)new
(1989-1995)index
(2166-2166)index
(2183-2184)index
(2221-2221)index
(2281-2281)index
(2309-2309)index
(2335-2335)index
(2362-2362)src/search.rs (6)
new
(39-41)new
(424-456)new
(743-748)new
(995-1006)index
(723-723)index
(2047-2050)
src/indexes.rs (2)
examples/cli-app/src/main.rs (1)
search
(41-62)src/search.rs (8)
SearchQuery
(1844-1847)SearchQuery
(1859-1862)SearchQuery
(1874-1877)SearchQuery
(1889-1891)new
(39-41)new
(424-456)new
(743-748)new
(995-1006)
🔇 Additional comments (11)
.code-samples.meilisearch.yaml (1)
97-101
: Rename via PATCH: correct body key is "uid" (not "indexUid").This sample matches the API: PATCH /indexes/{uid} accepts a body with { "uid": "NEW_UID" }. The linked issue text mentions "indexUid", which would be incorrect here—please keep the sample as-is and consider updating the issue description to avoid confusion. (meilisearch.com)
src/client.rs (4)
30-32
: Expose optional SwapIndexes.rename — aligns with API.Field name and wire format match the server (“rename”: boolean, default false). Skipping serialization when None is ideal. (meilisearch.com)
207-207
: Return type now includes two lifetimes — verify no downstream breakage.The signature change to MultiSearchQuery<', ', Http> should be source-compatible, but it’s a public API tweak; please run a quick compile check against known external usages.
537-537
: Doc example updated to include rename: None — good defaulting.Including the field in the literal helps keep examples compiling after adding the new struct field.
1255-1255
: Tests use rename: None explicitly — fine and future-proof.Explicit None avoids emitting the field on the wire and mirrors server default behavior.
src/indexes.rs (3)
1783-1789
: Constructor lifetime return looks good.Returning
IndexUpdater<'_, Http>
aligns with other builders and prevents accidental dangling references.
1989-1995
: Good:IndexesQuery::new
now returnsIndexesQuery<'_, Http>
.Consistent lifetime ergonomics across query builders.
1776-1779
: Leave serde rename as"uid"
Meilisearch v1.18 PATCH /indexes/{index_uid} expects the request body field for renaming to be"uid"
(response tasks useindexUid
), so the existing serde(rename = "uid") is correct. (meilisearch.com)Likely an incorrect or invalid review comment.
src/documents.rs (3)
89-94
: LGTM:DocumentQuery::new
returnsDocumentQuery<'_, Http>
.Matches the pattern in other modules; no behavioral change.
203-211
: LGTM:DocumentsQuery::new
lifetime return.Consistent and idiomatic; examples already pass a borrowed
Index
.
335-340
: LGTM:DocumentDeletionQuery::new
lifetime return.No concerns.
There was a problem hiding this 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 (2)
src/search.rs (1)
124-134
: Expose queryVector with broad aliasing — consider narrowing one alias.Field addition looks correct and backward-compatible. One nit: aliasing to "vector" is very broad and could accidentally deserialize unrelated fields if the server ever adds a top-level "vector". Safe to drop that alias.
Apply:
#[serde( rename = "queryVector", alias = "query_vector", alias = "queryEmbedding", alias = "query_embedding", - alias = "vector", skip_serializing_if = "Option::is_none" )]
src/indexes.rs (1)
1836-1845
: Rename builder methods are clear; minor ergonomics thought.Both with_uid and with_new_uid read well. Optional: document that only one of primary_key/new_uid needs to be set and how conflicts are handled (last write wins).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/client.rs
(5 hunks)src/indexes.rs
(5 hunks)src/search.rs
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/client.rs
🧰 Additional context used
🧬 Code graph analysis (2)
src/search.rs (3)
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/client.rs (3)
index
(422-424)new
(52-64)get_host
(223-225)src/request.rs (3)
query
(39-47)request
(71-91)request
(151-163)
src/indexes.rs (2)
examples/cli-app/src/main.rs (1)
search
(41-62)src/search.rs (8)
SearchQuery
(1845-1848)SearchQuery
(1860-1863)SearchQuery
(1875-1878)SearchQuery
(1890-1892)new
(39-41)new
(425-457)new
(744-749)new
(996-1007)
🔇 Additional comments (6)
src/search.rs (1)
2031-2069
: Solid coverage for queryVector; debug block is handy.Test reads well and asserts expected dimensionality. The optional raw dump behind MSDK_DEBUG_RAW_SEARCH is a nice touch for diagnosing server responses.
src/indexes.rs (5)
279-281
: Return type lifetime annotation for search() looks good.This brings the signature in line with other builders returning borrowing queries.
1776-1789
: Confirm server expects "uid" (not "indexUid") for rename.You serialize the rename as {"uid": ""} via #[serde(rename = "uid")]. Some docs/sdks mention "indexUid" for PATCH /indexes/:uid. Please verify the exact field name for Meilisearch 1.18.0 and adjust if needed (this is a wire-compat issue).
Would you like me to check the Meilisearch 1.18 API docs and open a follow-up patch if it differs?
1783-1789
: Constructor lifetime tweak LGTM.Returning IndexUpdater<'_, Http> is consistent with the other API changes.
1994-2000
: IndexesQuery::new lifetime annotation LGTM.Matches the pattern used elsewhere.
2241-2285
: Rename via update is well-tested; nice cleanup.Test asserts both new existence and old absence, with defensive cleanup. Looks good.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #704 +/- ##
==========================================
+ Coverage 85.63% 85.96% +0.33%
==========================================
Files 19 19
Lines 5853 5950 +97
==========================================
+ Hits 5012 5115 +103
+ Misses 841 835 -6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@curquiza Am I going in the right direction, would appreciate your feedback. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bors merge
704: Add queryVector to search responses and support index renaming r=curquiza a=kumarUjjawal # Pull Request Add `queryVector` to search responses and support index renaming. ## Related issue Fixes #703 ## What does this PR do? As discussed in the open issue we wanted to update the SDK such that when search requests include the `retrieveVectors` parameters, API responses include a `queryVector` field. ## PR checklist Please check if your PR fulfills the following requirements: - [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)? - [x] Have you read the contributing guidelines? - [x] Have you made sure that the title is accurate and descriptive of the changes? Thank you so much for contributing to Meilisearch! <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - New Features - Rename an index via update (set a new UID). - Optional rename behavior when swapping indexes. - Search results can include the query vector when requested. - Documentation - Added REST example demonstrating index rename via PATCH. - Breaking Changes - Public query-return types now include lifetimes; integrators may need small code updates. <!-- end of auto-generated comment: release notes by coderabbit.ai --> 708: Update version for the next release (v0.30.0) r=curquiza a=meili-bot _This PR is auto-generated._ The automated script updates the version of meilisearch-rust to a new version: "v0.30.0" <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - New Version - Package version bumped to 0.30.0. - Chores - Updated internal dependency to align with 0.30.0 release. - Documentation - Updated README and templates to reference meilisearch-sdk 0.30.0. - Refreshed code samples/config snippets to use the new dependency version. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Kumar Ujjawal <[email protected]> Co-authored-by: Clémentine <[email protected]> Co-authored-by: meili-bot <[email protected]>
This PR was included in a batch that successfully built, but then failed to merge into main. It will not be retried. Additional information: {"message":"Changes must be made through a pull request.","documentation_url":"https://docs.github.com/articles/about-protected-branches","status":"422"} |
Pull Request
Add
queryVector
to search responses and support index renaming.Related issue
Fixes #703
What does this PR do?
As discussed in the open issue we wanted to update the SDK such that when search requests include the
retrieveVectors
parameters, API responses include aqueryVector
field.PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!
Summary by CodeRabbit
New Features
Documentation