feat: add SearchAsync for vector, keyword, and hybrid search - #22
Open
lukekim wants to merge 4 commits into
Open
feat: add SearchAsync for vector, keyword, and hybrid search#22lukekim wants to merge 4 commits into
lukekim wants to merge 4 commits into
Conversation
Wraps POST /v1/search, which was previously unreachable from .NET without hand-rolling the HTTP call. spice.js is the only other SDK that exposes it. Only Text is required; Datasets, Limit, Where, AdditionalColumns and Keywords are optional. Supplying Keywords pre-filters the embedding column with a lexical search before the vector search, making the search hybrid. SearchMatch maps the runtime's wire format, including the `_score` field name and the objects the runtime omits when empty; those default to empty dictionaries so callers can read them without a null check. System.Text.Json was already resolved transitively via Apache.Arrow.Adbc; it is now referenced explicitly at the version that dependency already selects, since netstandard2.0 has no in-box System.Text.Json.
There was a problem hiding this comment.
Pull request overview
Adds first-class .NET SDK support for the Spice runtime /v1/search endpoint via SpiceClient.SearchAsync, including strongly-typed request/response models, HTTP wiring, unit tests for the wire contract, and README documentation so callers can perform vector/keyword/hybrid search without hand-rolling HTTP.
Changes:
- Introduces
SpiceClient.SearchAsyncandISpiceHttpClient.SearchAsyncto expose/v1/search. - Adds
Spice.Searchtypes (SearchRequest,SearchResponse,SearchMatch) with JSON wire-format handling (e.g.,_score, omitted empty objects). - Adds unit tests for request/response serialization and documents usage in README; pins
System.Text.Jsonexplicitly.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| SpiceTest/SearchTest.cs | Adds tests for request serialization and response deserialization wire contract. |
| Spice/src/SpiceClient.cs | Exposes new SearchAsync API surface on the public client. |
| Spice/src/Search/SearchTypes.cs | Defines public search request/response/match DTOs with JSON attributes. |
| Spice/src/Http/SpiceHttpClient.cs | Implements /v1/search POST, JSON (de)serialization, and error extraction. |
| Spice/src/Http/ISpiceHttpClient.cs | Adds SearchAsync to the HTTP client interface. |
| Spice/Spice.csproj | Adds explicit System.Text.Json package reference. |
| README.md | Documents SearchAsync usage and explains request/response fields. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
AuthenticateAsync passed stream.GetTrailers() as an argument, so trailers were
read eagerly — before the handshake had completed. gRPC only makes trailers
available once a call finishes, so any handshake that fails throws
System.InvalidOperationException: Can't get the call trailers because the
call has not completed successfully.
which replaces the gRPC status describing the actual problem. Every integration
test in CI reports this, and it says nothing about what to fix.
Now the token is read from the response headers first, trailers are consulted
only if the headers carry none, and a failure to read them is caught rather
than propagated. The resulting SpiceException names the likely cause and
carries the originating RpcException as InnerException.
Reproduced against a TLS endpoint that is not a Flight service:
before: InvalidOperationException: Can't get the call trailers ...
after: SpiceException: Failed to authenticate: the runtime returned no
authorization token. Check that the API key is valid for this endpoint.
3 tasks
… failing member Two issues from review: - System.Text.Json was referenced unconditionally, pushing a dependency constraint onto net8.0+ consumers that already have it in-box. Now conditioned on netstandard2.0, which is the target that actually needs it. netstandard2.0 still resolves 9.0.9, so there is no NU1605 downgrade. - Argument validation reported nameof(request) when it was request.Text that was empty, pointing callers at the wrong member.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds
SpiceClient.SearchAsync, wrapping the runtime'sPOST /v1/search— vector similarity, keyword, and hybrid search over datasets with an embedding column.Only
Textis required. SettingKeywordspre-filters the embedding column with a lexical search before the vector search runs, making the search hybrid.New public types in
Spice.Search:SearchRequest,SearchResponse,SearchMatch.Why
/v1/searchworks on a defaultspice runand is a distinctive Spice capability, but .NET users had no way to reach it short of hand-rolling the HTTP call. spice.js is currently the only SDK that exposes it.Two wire-format details the types handle so callers don't have to:
_score, notscoredata,primary_keyandmetadataare omitted entirely when empty, so they default to empty dictionaries rather than null — readable without a guardOn a failed search the runtime's own
{"error": "..."}message is surfaced rather than a bare status code, since that message names what the caller needs to fix.Part of aligning search support across the SDKs.
Notes for review
System.Text.Jsonis now an explicitPackageReference. It was already resolved transitively throughApache.Arrow.Adbc(which requires>= 9.0.9); this pins the same version explicitly because the search path depends on it directly, andnetstandard2.0has no in-boxSystem.Text.Json. Pinning below 9.0.9 fails the build withNU1605.ReadAsStringAsyncis#if-split — theCancellationTokenoverload doesn't exist onnetstandard2.0, and the analyzers treatCA2016as an error on the modern targets.SpiceClient.csand the HTTP client. This branch adds new members rather than changing existing ones, but it will want a merge rather than a fast-forward if those land first.Verification
dotnet build -c Release— all four targets,netstandard2.0includeddotnet test— 125 passed, 0 failed on each of net8.0 / net9.0 / net10.0, including 10 new tests inSearchTest.cscovering request serialization (wire field names,limit: 0, empty collections) and response deserialization (_score, omitted objects, missing keys)spice run: the search request reaches/v1/search, and afailure surfaces the runtime's own message (
"Search cannot be run on <dataset> because it has no embeddings or full text search indexes.") rather than a bare status codeembedding column and a loaded embedding model, which this environment has no credentials for. The 33 skipped tests are the existing suites that require one.