fix(server): widen memory embedding test fixture to halfvec(4000) - #382
fix(server): widen memory embedding test fixture to halfvec(4000)#382dpage wants to merge 1 commit into
Conversation
The chat_memories fixture in the tools package still declared its embedding column as vector(3), a leftover from before the collector migration v6 widening. The memory store pads every embedding to embedding.MaxDimensions, which is 4000, on both the insert and the query paths, so Postgres rejected the padded value with "expected 3 dimensions, not 4000" and the two embedding integration tests failed on any host that actually has pgvector installed. The column now reads halfvec(4000), matching the sibling fixture in the memory package; halfvec is required rather than a wider vector because pgvector caps the vector type at 2000 dimensions, which is precisely why the original widening chose halfvec. The stale comments claiming that the mock's three-element response matches the column type have been corrected to explain the padding instead, whilst the mock itself continues to return three elements.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
1 participant
The problem
The
chat_memoriesfixture used by the embedding integration tests inserver/src/internal/tools/memory_tools_embedding_db_test.gostill declared itsembedding column as
vector(3), which was correct back when the mocked Geminiendpoint's three-element response went into the database verbatim, but has been
wrong since the collector's migration v6 widened the embedding columns to
halfvec(4000)for multi-provider support.memory.Storenow pads everyembedding out to
embeddingpkg.MaxDimensions, which is 4000, on the insert pathin
server/src/internal/memory/store.goand again on the query path, soPostgres rejected the padded value outright with
ERROR: expected 3 dimensions, not 4000 (SQLSTATE 22000). The sibling fixture in the memory package wasupdated at the time and reads
halfvec(4000)already; this one was missed.The upshot is that both
TestStoreMemoryGeneratesEmbeddingIntegrationandTestRecallMemoriesGeneratesQueryEmbeddingIntegrationfailed on any host wherepgvector is actually installed, which includes the development host.
Why halfvec rather than a wider vector
Simply bumping the column to
vector(4000)is not an option, because pgvectorcaps the
vectortype at 2000 dimensions;halfvecexists precisely to carrywider embeddings at half precision, and that is exactly why the original
widening chose it. The fixture now matches the production schema and the
memory-package fixture.
Why CI never caught this
These two tests have never actually executed in CI. The workflow's Postgres
service is a plain
postgres:<version>image with no pgvector available, soCREATE EXTENSION IF NOT EXISTS vectorfails, the helper callst.Skipf("pgvector extension unavailable"), and the run reports green whilstquietly skipping the whole seam. A follow-up PR adds pgvector to the CI Postgres
service so that this path is genuinely gated; no workflow files are touched
here, keeping the two concerns separate.
Scope
This is a fixture and comment correction only: no production code changes, no
test restructuring, and no new tests. The stale comments asserting that the
mock's three-element vector matches the column type have been reworded to
explain the padding, since the mock returning three elements remains correct and
should stay.
Verification
Run on the development host, where pgvector is installed, against the local
loopback Postgres:
gofmt -lon the touched file is clean, andgo build ./...succeeds.Both tests now PASS rather than skip:
The whole
internal/toolspackage is green, and coverage rises from 49.3% to51.1% now that these two tests execute rather than skip.
make lintinserverreports 0 issues.🤖 Generated with Claude Code
https://claude.ai/code/session_015nPEZyXgN1zK7EuEXhfZvW