feat: add search for the runtime's /v1/search endpoint

[claudespice]

What

Adds Client::search(SearchRequest), wrapping the runtime's /v1/search endpoint — vector similarity search, with optional keywords for a hybrid lexical + vector ranking.

let response = client
    .search(
        SearchRequest::new("tokyo plane tickets")
            .with_datasets(["app_messages"])
            .with_limit(3)
            .with_additional_columns(["timestamp"]),
    )
    .await?;

for m in response {
    println!("{} {} {:?}", m.score, m.dataset, m.matches);
}

Why

/v1/search works on a default spice run, but of the six client SDKs only spice.js could reach it. Rust users had to hand-roll the HTTP call, including the response shape.

Part of aligning capabilities across the SDKs against the runtime's own API surface.

Shape follows what the crate already does: SearchRequest uses the DatasetRefreshRequest builder pattern (mut self -> Self, #[must_use]), SearchError is a typed snafu enum alongside DatasetError and QueryError, and the call goes through the existing QueryHttpClient so it picks up auth and the configured http_url.

Two details the response types encode, taken from the runtime's wire format rather than the OpenAPI example (which is out of date on the first point):

• matches holds a list of values per column — one column can contribute several chunks to a single match.
• data, primary_key, and metadata are omitted by the runtime when empty, so each defaults rather than being required.

Errors carry the runtime's plain-text explanation ("No data sources provided") next to the status code, which a status alone loses. Requests the runtime would reject with a 400 — empty text, a zero limit — are validated before sending so the error names the field to fix. An empty datasets vector is omitted rather than sent, since the runtime 400s on an empty list.

Verification

• cargo build
• cargo test --lib — 199 passed, 0 failed (12 new: 8 unit in search.rs, 4 wiremock-backed in client.rs)
• cargo test --doc — 28 passed, including the new README example
• cargo fmt --all --check — clean
• cargo clippy --all-features --all-targets — no new warnings. The two that remain are pre-existing: QueryHttpClient::new dead_code in the lib, and 5 clone/from_ref warnings in tests/client_test.rs.
• Integration tests — not run (no live runtime available)

Review gate

Attests the trunk-merge conflict resolution in e0d621d (composition only — no behaviour change), not the original feature commits.

• Adversarial review: codex — scoped to origin/trunk...HEAD — verdict needs-attention, 4 findings, all against the pre-existing search surface rather than the merge resolution. Triaged and filed as search: an explicitly empty dataset scope fails open to every dataset, plus three robustness gaps in the new /v1/search surface #91 (an explicitly empty dataset scope fails open; defaults hide malformed responses; cache_control not forwarded; with_keywords documented as hybrid ranking but prefilters). Finding 1 is cheap now and breaking once released — see the PR comment.
• Conflict resolution verified lossless: src/client.rs function set is exactly the union of both sides (79 = 77 ∪ 74, no missing, no extra); diff vs trunk is 570 insertions and 0 deletions, so nothing from feat: add runtime_status and is_ready #81 was dropped.
• Gate: cargo fmt --all --check, cargo clippy --all-features, cargo test --lib (223 passed), cargo test --doc (34 passed, including both recomposed README fences).

[claudespice]

No checks have reported on this PR because its workflow run is parked awaiting approval, not because CI is failing: run 30697688240 for c8a46653b sits at action_required, so build.yml never executed. A maintainer clicking Approve and run is the only way to get a result here — I do not have the rights to approve or re-dispatch it.

Sibling PR #81 is parked on the same gate (run 30679500818), so approving both together would give the whole feat/* pair its first real CI signal.

[grokspice]

@copilot review

[Copilot]

Pull request overview

Adds first-class Rust SDK support for the runtime’s /v1/search HTTP endpoint by introducing typed request/response models, error handling, and a new Client::search API that routes through the existing QueryHttpClient (so it inherits auth + configured http_url).

Changes:

• Introduces SearchRequest (builder-style) plus SearchResponse/SearchMatch deserialization types and SearchError (SNAFU) in a new search module.
• Adds QueryHttpClient::search and exposes SpiceClient::search as the public entry point.
• Updates public exports and README/docs; adds unit + wiremock-backed tests for the new endpoint.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/search.rs	New search request/response types, validation, errors, and unit tests for (de)serialization behavior.
src/query.rs	Adds QueryHttpClient::search implementation that POSTs to /v1/search and parses results/errors.
src/lib.rs	Exposes the new search module and re-exports search types in the public API.
src/client.rs	Adds SpiceClient::search method, docs, and wiremock-based tests covering request/response and error surfacing.
README.md	Documents search usage with an end-to-end example and explains match payload shape.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/client.rs:619

• The SpiceClient::search docs list possible errors but omit SearchError::ParseError, which can be returned if the runtime responds with invalid/unexpected JSON. Documenting this variant helps callers understand the full error surface.

    /// - [`SearchError::InvalidRequest`] if the request has empty text or a zero limit
    /// - [`SearchError::HttpError`] if the HTTP endpoint is not configured or unreachable
    /// - [`SearchError::SearchFailed`] if the runtime rejects the search

src/client.rs:1558

• In test_search_validates_before_sending, unwrap_or_default() can mask failures to fetch Wiremock’s received requests, making the test pass even if the assertion can’t actually be validated. This should fail loudly so the test remains trustworthy.

            server
                .received_requests()
                .await
                .unwrap_or_default()
                .is_empty()

[claudespice]

Update to my 2026-08-02 comment: the workflow run is no longer parked at action_required — it was approved and ran. Run 30977579403 executed at 05:15Z today, and 10 Build and test legs came back red. None of them is caused by this diff.

Every unit test passes on every platform. The only failure is the client_test integration suite, which panics before it can assert anything:

thread 'tests::test_query' panicked at tests/client_test.rs:415:17:
Error: Query execution failed: Ipc error: Can't handshake
  code: 'The request does not have valid authentication credentials', message: "API key is required"

Mechanism — the fork-secrets boundary. build.yml:181 passes the credential as SCP_SPICEAI_TPCH_API_KEY: ${{ secrets.SCP_SPICEAI_TPCH_API_KEY }}. This PR's head is on a fork, and GitHub withholds repository secrets from cross-fork pull_request runs, so the variable arrives empty and the handshake is rejected before the test body executes.

Evidence the diff is not implicated: trunk's own build for the same commit range is green (30971730156) running these same tests with the secret present; tests/client_test.rs predates this branch (last modified in #70); and every unit test in the failing run passes, including the search tests this PR adds.

This is the same wall as spiceai/spicepy#177 and spiceai/spice.js#311 — the identical failure class in three SDK repos. Nothing on this branch can clear it, so this PR is gated purely on review; the red legs carry no information about the change. The durable fix would be to make client_test skip when the secret is absent, but that is a test-harness change outside this PR's scope and I have not made it here.

[claudespice]

Flagging an overlap I should have caught when this PR was opened: #82 already implements this feature, and it was opened first.

@lukekim's #82 ("feat: add search for vector, keyword, and hybrid search", opened 2026-07-27) and this PR (opened 2026-08-01) both add search to the crate, and they touch the same five source files:

README.md, src/client.rs, src/lib.rs, src/query.rs, src/search.rs

Neither PR referenced the other, and this PR's description asserts Rust users had no way to reach /v1/search — written without knowledge of #82. That is my error, not a defect in either change.

Where the two differ:

• feat: add search for vector, keyword, and hybrid search #82 additionally patches .github/workflows/build.yml; this PR does not touch CI.
• feat: add search for vector, keyword, and hybrid search #82 is currently CONFLICTING with trunk and has had no activity for ~157h. This PR is MERGEABLE with zero unresolved review threads.
• The API shapes are near-identical (both use the crate's mut self -> Self builder convention on SearchRequest).

Recommendation, but the call is a maintainer's, not mine: one of the two should be closed rather than both carried. This PR is the one that currently merges cleanly and has been through review; #82 carries the CI change and has precedence by date. I am happy to close this PR in favour of #82 if that is preferred — say the word and I will, and I will port anything from here that #82 lacks.

For reference, the same duplication exists in spicepy#174 vs #177, and in gospice it has already resolved — my #79 merged and left @lukekim's #77 conflicting and redundant.

Separately, on this PR's red Build and test legs: #83 is a fix for that environmental failure and is itself waiting on review. It would clear these legs for any PR in the repo.

[claudespice]

Filed #88 to own the harness fix, so the permanent red on fork PRs has somewhere to be fixed rather than being re-diagnosed on each PR. Nothing about this branch changes — the ten Build and test legs stay red until client_test skips without the key.

[claudespice]

Unblocked the trunk conflict this picked up when #81 merged at 22:40Z — README.md, src/client.rs and src/lib.rs all conflicted as shared-skeleton interleaves (two methods aligned on the same doc-comment boilerplate, two ### sections sharing one rust,no_run fence). Resolved by composition, keeping both sides verbatim; the diff against trunk is 570 insertions and 0 deletions, and src/client.rs's function set is exactly the union of the two sides (79 = 77 ∪ 74). Full gate in the PR body.

One thing worth a decision before this merges, since it is cheap now and a breaking change once the API ships — raised by the pre-merge adversarial review and filed as #91:

.with_datasets([]) silently searches every dataset rather than none. datasets is a Vec<String> with skip_serializing_if = "Vec::is_empty" (src/search.rs:52-53), so an empty vector is omitted from the request, and an omitted datasets means "search everything". The omission is deliberate — test_empty_datasets_omitted says so directly:

// The runtime rejects an empty dataset list with a 400, so an empty
// vector must be omitted rather than sent.

so a runtime rejection that fails closed becomes client behaviour that fails open. A caller computing a scope — a tenant allow-list, a permission filter — that legitimately comes back empty gets an unrestricted search across everything the runtime credential can reach.

The root of it is that Vec<String> cannot distinguish "never scoped" from "scoped to nothing". Option<Vec<String>> can: None keeps the search-all default, and validate() rejects Some(vec![]) locally with the same meaning the runtime's 400 carries.

I have not made that change here — it alters the public API on a PR you have already approved, so it is your call rather than mine. Happy to push it to this PR if you would like it in before merge, or leave it to #91 as a follow-up. #91 also carries three lower-severity findings on the same surface (defaults on required response fields hide malformed 200s, cache_control is not forwarded to search, and with_keywords is documented as hybrid ranking where the runtime prefilters).