Skip to content

feat(retrieval): Phase 6 — integration suite (real valkey-search)#245

Merged
jamby77 merged 1 commit into
masterfrom
feature/retrieval-sdk-phase6-integration
Jun 21, 2026
Merged

feat(retrieval): Phase 6 — integration suite (real valkey-search)#245
jamby77 merged 1 commit into
masterfrom
feature/retrieval-sdk-phase6-integration

Conversation

@jamby77

@jamby77 jamby77 commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Phase 6 — Integration suite (real Valkey + valkey-search)

Stacked on #243 (Phase 5) — base is the Phase 5 branch. Final phase of the Retrieval SDK.

What's new

Retriever.integration.test.ts — skip-guarded end-to-end suite against a real valkey-bundle (search module), VALKEY_URL default redis://:devpassword@localhost:6384. Deterministic sha256 fakeEmbed (8-dim).

Covers the full SDK lifecycle:

  • createIndex (idempotent — called twice) → upsertvector query returns the upserted doc
  • full hit shape (id/text/fields/finite score)
  • query by precomputed vector (embedFn bypass + Float32 serialization)
  • TAG and NUMERIC filters narrow correctly
  • register/unregister discovery marker round-trip on __betterdb:caches
  • health snapshot · delete removes (polls for HNSW propagation) · dropIndex cleans up

Reliability

HNSW indexing/deletion is asynchronous, so beforeAll polls until every doc is queryable and the delete test polls until propagation completes — no fixed sleeps, no order-dependence. Verified green and non-flaky across multiple live runs (9 integration tests; full package suite 101/101). Added iovalkey as a devDependency (the library client is caller-supplied; iovalkey is only used by the test).

Review-driven changes

A pre-PR review flagged indexing-lag races (filter/delete/hit-shape tests), a NaN-passes-typeof score check, and missing FT-capability skip-probe — all fixed; added precomputed-vector + register coverage.


Note

Low Risk
Test-only changes with no production SDK or runtime dependency changes beyond lockfile resolution metadata.

Overview
Adds end-to-end integration tests for Retriever against a live Valkey instance with the search module (VALKEY_URL, default localhost:6384). The suite skips when Valkey or FT.* is unavailable (FT._LIST probe).

Coverage exercises index lifecycle (idempotent createIndex), upsert, text and precomputed vector queries, TAG/NUMERIC filters, discovery register/unregister, health, delete, and dropIndex. Setup and assertions use polling instead of fixed sleeps to tolerate async HNSW indexing and deletion.

iovalkey is added as a devDependency for the test client only; runtime SDK behavior is unchanged.

Reviewed by Cursor Bugbot for commit ce5813d. Bugbot is set up for automated code reviews on this repo. Configure here.

@jamby77 jamby77 force-pushed the feature/retrieval-sdk-phase5-observability branch from 98fc992 to e1c6b81 Compare June 16, 2026 10:01
@jamby77 jamby77 force-pushed the feature/retrieval-sdk-phase6-integration branch from a7ef1e0 to 94a8bf3 Compare June 16, 2026 10:08
@jamby77 jamby77 force-pushed the feature/retrieval-sdk-phase5-observability branch from e1c6b81 to 7d97a4d Compare June 18, 2026 06:36
@jamby77 jamby77 force-pushed the feature/retrieval-sdk-phase6-integration branch from 94a8bf3 to bd4a678 Compare June 18, 2026 06:36

@KIvanow KIvanow left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving. Solid end-to-end coverage: graceful FT._LIST skip, pollUntil over fixed sleeps for HNSW propagation, NaN-safe score assertion, and a clean unique-per-run index. Two non-blocking notes for a follow-up:

  1. The suite shares and mutates one index, so it is order-dependent despite the "no order-dependence" note. All tests run against the index built in beforeAll, and the delete test (line 176) permanently removes doc:2, which the NUMERIC-filter test (line 135) and health test rely on. It is green today because Vitest preserves in-file order, but under --sequence.shuffle or concurrent the delete could race ahead and break the earlier assertions. Worth isolating the destructive case (its own doc or its own index, like the dropIndex test) or softening the wording.

  2. The iovalkey 'error' handler is only attached on the skip path (line 62). If the connection drops mid-suite in the non-skip case, an unhandled error event can crash the runner. Attaching it right after construction would harden it.

@jamby77 jamby77 force-pushed the feature/retrieval-sdk-phase6-integration branch from bd4a678 to 7a8e651 Compare June 19, 2026 13:16
@jamby77 jamby77 force-pushed the feature/retrieval-sdk-phase5-observability branch from ac1cd7d to d857536 Compare June 21, 2026 14:13
Base automatically changed from feature/retrieval-sdk-phase5-observability to master June 21, 2026 14:16
- Add Retriever.integration.test.ts (skip-guarded, VALKEY_URL default 6384)
- Cover create index, upsert, vector query returning the upserted doc,
  TAG and NUMERIC filters, delete, health, and drop end-to-end
- Add iovalkey devDependency for the integration client
@jamby77 jamby77 force-pushed the feature/retrieval-sdk-phase6-integration branch from 7a8e651 to ce5813d Compare June 21, 2026 14:17
@jamby77 jamby77 merged commit 0687d26 into master Jun 21, 2026
3 checks passed
@jamby77 jamby77 deleted the feature/retrieval-sdk-phase6-integration branch June 21, 2026 14:20
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 21, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants