Let a note answer a question (#373) #27

2026-07-31T11:34:02+02:00

claude commented

2026-07-31 11:34:02 +02:00

One commit. Vikunja #373 — a note you asked her to remember could never be recalled.

The bug, proven rather than assumed. The memory search was nested inside the failure branch of the notes-only gate:

if !ConfidentScores(noteScores, minScore, minMargin) {   // notes gate FAILED
    if hits, err := memStore.Search(...); err == nil {
        if text, ok := bestRecall(hits, minScore, minMargin) {  // SAME gate, SAME numbers

Both indexes hold the same notes with the same vectors — a note is written to both on save, and the query side embeds once and hands the same vector to both searches. Both scorers are a dot product over normalised vectors, so a note scores bit-identically in the two places. So reaching the inner check means the outer one already failed, and the inner one is arithmetically guaranteed to fail too: if the best memory hit is a note it is the same top note, and its runner-up can only be closer, so the margin can only be smaller. Facts are the only rows the notes table doesn't hold — so a fact was the only thing that could ever come back. bestRecall was fine; its caller's precondition was the bug.

The fix: run the memory search first. One search over notes and facts, one gate. A winning note gets phrased in her voice; a fact is read back as stored. The notes-only pass stays behind it for older notes the vector index doesn't hold.

No guard was loosened. The set of questions that get answered is exactly the same union as before — only which pass gets first look changed. This matters: making notes recallable by making recall guess more would have traded one bug for a worse one.

Numbers (before → after):

	before	after
hash (CI ratchet) recall@1	36.0%	37.0%
e5 recall@1	72.0%	70.4%
e5 answered after gate	68.0%	66.7%
false recall (both)	0/5 hash, 1/5 e5	unchanged

The percentages move because two mixed note+fact cases were added to the fixture (denominator 25 → 27), not because the fix cost recall. One of the new cases — a note beating facts — is exactly what was impossible before and now passes end to end. The other fails, and it fails on the embedder ranking a note distractor above the right fact, not on the gate.

Tests: a fixed-vector embedder so scores are exact, then a clearly-best note answers (this fails on the old code), a better-matching fact answers, and no-clear-best still stays silent.

Three things found and left, one of which is a landmine:

Fixture id collision. Filler notes use ids f1..f12, and the two store backends disagree about a duplicate id — sqlite upserts, the in-memory one appends. A case that reuses a filler id would score differently on the two backends and look like a real difference. Hit during this work and dodged by renaming; the fixture guard checks ids within a case but not against the fillers.
The gate is nearly a no-op under e5. The right-note and must-stay-silent score bands overlap above 0.55, so the 0.008 margin is the only thing separating them — one case ranks first with a margin of 0.001. Already a known finding; this PR doesn't make it better or worse.
The other half of #373: note ranking is still cosine only, with no recency and no signal from the kind of note.

One commit. Vikunja #373 — a note you asked her to remember could never be recalled. **The bug, proven rather than assumed.** The memory search was nested *inside* the failure branch of the notes-only gate: ```go if !ConfidentScores(noteScores, minScore, minMargin) { // notes gate FAILED if hits, err := memStore.Search(...); err == nil { if text, ok := bestRecall(hits, minScore, minMargin) { // SAME gate, SAME numbers ``` Both indexes hold the same notes with the same vectors — a note is written to both on save, and the query side embeds once and hands the same vector to both searches. Both scorers are a dot product over normalised vectors, so a note scores *bit-identically* in the two places. So reaching the inner check means the outer one already failed, and the inner one is arithmetically guaranteed to fail too: if the best memory hit is a note it is the same top note, and its runner-up can only be closer, so the margin can only be smaller. Facts are the only rows the notes table doesn't hold — so **a fact was the only thing that could ever come back.** `bestRecall` was fine; its caller's precondition was the bug. **The fix: run the memory search first.** One search over notes and facts, one gate. A winning note gets phrased in her voice; a fact is read back as stored. The notes-only pass stays behind it for older notes the vector index doesn't hold. **No guard was loosened.** The set of questions that get answered is exactly the same union as before — only which pass gets first look changed. This matters: making notes recallable by making recall guess more would have traded one bug for a worse one. **Numbers** (before → after): | | before | after | |---|---|---| | hash (CI ratchet) recall@1 | 36.0% | 37.0% | | e5 recall@1 | 72.0% | 70.4% | | e5 answered after gate | 68.0% | 66.7% | | false recall (both) | 0/5 hash, 1/5 e5 | unchanged | The percentages move because two mixed note+fact cases were added to the fixture (denominator 25 → 27), not because the fix cost recall. One of the new cases — a note beating facts — is exactly what was impossible before and now passes end to end. The other fails, and it fails on the embedder ranking a note distractor above the right fact, not on the gate. **Tests:** a fixed-vector embedder so scores are exact, then a clearly-best note answers (this fails on the old code), a better-matching fact answers, and no-clear-best still stays silent. **Three things found and left, one of which is a landmine:** 1. **Fixture id collision.** Filler notes use ids `f1..f12`, and the two store backends disagree about a duplicate id — sqlite upserts, the in-memory one appends. A case that reuses a filler id would score differently on the two backends and look like a real difference. Hit during this work and dodged by renaming; the fixture guard checks ids within a case but not against the fillers. 2. **The gate is nearly a no-op under e5.** The right-note and must-stay-silent score bands overlap above 0.55, so the 0.008 margin is the only thing separating them — one case ranks first with a margin of 0.001. Already a known finding; this PR doesn't make it better or worse. 3. **The other half of #373:** note ranking is still cosine only, with no recency and no signal from the kind of note.

claude added 2 commits 2026-07-31 11:34:03 +02:00

Let a note win a recall, not only a fact (#373 ) 98ee701e03

The memory pass ran only after the notes-only gate had already rejected
the same note at the same score. Notes and facts share one vector index,
so a note that failed there failed again — the branch could only ever
return a fact.

Now the memory pass runs first: one search over everything Maven
remembers, one gate, and the memory that clearly matches best answers
(a note gets phrased, a fact is read back). The notes-only pass stays
behind it for notes the vector index does not hold. No threshold moved,
so the set of questions answered is unchanged — only which memory
answers them.

Fixture gained two mixed note+fact cases, so the answerable count goes
25 -> 27: hash recall@1 36.0% -> 37.0% (ratchet 0.32 unchanged, comment
updated), e5 recall@1 72.0% -> 70.4%, false recall still 1/5.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ

Merge the note recall fix 892330eb84

kami commented

2026-07-31 20:22:16 +02:00

Superseded by #47, which landed this whole stack on master as one reviewed integration merge. This PR head is an ancestor of master — its commits are in, nothing here is lost. Closing as merged-by-proxy rather than merged, since the merge came in through #47.

Review threads on this PR were answered or acted on before the merge; the Russian wording fixes went in as #48.

Superseded by #47, which landed this whole stack on master as one reviewed integration merge. This PR head is an ancestor of master — its commits are in, nothing here is lost. Closing as merged-by-proxy rather than merged, since the merge came in through #47. Review threads on this PR were answered or acted on before the merge; the Russian wording fixes went in as #48.

kami closed this pull request

2026-07-31 20:22:17 +02:00

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: kami/Maven#27