MemoryStore.Search decodes and unmarshals every row before keeping topK #195

Merged
claude merged 3 commits from task/643-memorystore-search-decodes-and-unmarshal into master 2026-08-07 00:09:51 +02:00
Contributor

Closes Vikunja #643.

Acceptance criteria and quality gate are in TASK.md on this branch.
Review the review, not the diff — leave comments and the agent will apply them via task start 643.

Closes Vikunja #643. Acceptance criteria and quality gate are in `TASK.md` on this branch. Review the review, not the diff — leave comments and the agent will apply them via `task start 643`.
claude added 3 commits 2026-08-07 00:05:33 +02:00
MemoryStore.Search is on the per-turn recall path and had no benchmark, so
any claim about its cost was an argument rather than a measurement.

Seeds a store with rows the shape recall actually stores — 384-wide
vectors, the resident embedder's width, and a meta blob carrying the note
text — at 1000 and 10000 rows. 10000 is the ceiling the type doc claims a
full scan is fine at.

Measured as it stands: 5.3ms and 24k allocs at 1000 rows, 70.6ms and 240k
allocs at 10000.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YMNNEkYx1mZFtHNrFk7uqb
Search decoded the vector blob into a []float32 and JSON-unmarshalled the
meta map for every row, then sorted all N and threw away everything past
topK. Meta only ever matters for a survivor, and the sort answered a
question a bounded heap answers cheaper.

The scan still visits every row — that is what picks the winners. What it
no longer does is allocate for a row it is about to discard. dotBlob reads
the vector out of its stored bytes, so scoring costs nothing; a row is
copied and its meta unmarshalled only once it has entered the topK.

At 10000 rows and topK 10: 70.6ms to 26.8ms, 58MB to 17.5MB, 240k allocs
to 60k.

Recall is unchanged where it is measured. recall+onnx scores 22/32 with
recall@1 70.4% and recall@3 85.2%, identical to before.
TestMemoryStoreSearchMatchesNaive pins the ranking against the full-sort
implementation it replaced, and TestDotBlobMatchesDot pins bit-identical
scores, which the 0.008 gate margin demands.

One behaviour did move: ties. sort.Slice is not stable, so equal scores
were ordered arbitrarily; the heap now keeps the earliest. Under the real
embedder an exact tie is a duplicate vector and nothing moved. Under the
hash embedder the eval's floor uses, everything ties at 0 and that run's
recall@3 went 74.1% to 81.5% — a number that measures tie order, not
retrieval. recall@1 and false recall, the two the eval asserts, are
unchanged on both runs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YMNNEkYx1mZFtHNrFk7uqb
Owner's call, 07-08-2026. A completion summary written into the
description on the way out is lost anyway, and the durable record is the
commit messages and the merged PR.

Written during the V-641 session and left uncommitted; it rides this
branch rather than being dropped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YMNNEkYx1mZFtHNrFk7uqb
claude merged commit a4630b9314 into master 2026-08-07 00:09:51 +02:00
Sign in to join this conversation.
No Reviewers
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: kami/Maven#195