From 394dbd4dbfb92b3ae463d05bfd3b37fc2fb67eec Mon Sep 17 00:00:00 2001 From: MXAntian Date: Thu, 20 Aug 2026 14:20:53 +0800 Subject: [PATCH] fix(recall): importance filters, it does not rank MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A self-rated field that does not predict use was moving results 2-4 places. It had already been demoted once — from a heavy weight down to 0.05 in the hybrid path and 0.1 in the FTS one — on the reasoning that a weak prior is harmless. Measured against RRF's actual spacing, it is not. Adjacent ranks are separated by ~0.0026, so: imp 8 vs 7 -> 0.0050 ~2 rank positions imp 9 vs 7 -> 0.0100 ~4 rank positions imp 10 vs 3 -> 0.0350 ~17 rank positions A top-8 injection is decided inside that range: four places is the difference between a row reaching the model and not. And the leverage was being spent on noise. On a live 8.6k-row store, rows that have ever been recalled average importance 7.70; rows never recalled average 7.36. A 0.34 gap on a 1-10 scale, with 90% of the corpus at >=7 — self-rated, saturated, uncorrelated with use. The comment sitting next to the term already said "don't let self-rated importance drive ranking"; 0.05 was still enough to drive it. importance keeps the jobs it is actually good at: the min_importance filter, surface-cold thresholds, and display. Filtering on it is a caller stating a floor. Ranking on it is the store guessing. FTS weights renormalized across the three surviving signals (0.61/0.22/0.17) so the score keeps its 0-1 shape. The test is calibrated to the failure regime, not to an obvious case. A first version gave the low-importance row a large relevance edge, which survived the tilt — so it passed with and without the fix and proved nothing. It now ties relevance and gives the low-importance row a single prior access, worth ~0.0455, deliberately less than the ~0.07 an importance gap of 10-vs-3 was buying. Verified red before the change and green after. level-rank-offset.test.mjs pins the composite as source text, so its regex moved with the formula; its actual assertion (no level-weight multiplier) is unchanged. 325 passed / 0 failed. Co-authored-by: 千夏 Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 5 ++ index.mjs | 24 ++++++-- level-rank-offset.test.mjs | 5 +- ranking-importance.integration.test.mjs | 73 +++++++++++++++++++++++++ 4 files changed, 102 insertions(+), 5 deletions(-) create mode 100644 ranking-importance.integration.test.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 84326cc..f7000b8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -58,6 +58,11 @@ jobs: TOKENMEM_DB_PATH: ${{ runner.temp }}/mneme-ci-injection-hygiene.db run: node injection-hygiene.integration.test.mjs + - name: Integration test (ranking — importance filters, does not rank) + env: + TOKENMEM_DB_PATH: ${{ runner.temp }}/mneme-ci-ranking-importance.db + run: node ranking-importance.integration.test.mjs + - name: Integration test (encoding-damage detection, both write paths) env: TOKENMEM_DB_PATH: ${{ runner.temp }}/mneme-ci-encoding-damage.db diff --git a/index.mjs b/index.mjs index 370752b..e17878c 100644 --- a/index.mjs +++ b/index.mjs @@ -1889,7 +1889,6 @@ export function recallMemories(opts = {}) { const ftsScore = opts._legacyFtsScoring ? (row.fts_rank ? Math.min(1, Math.abs(row.fts_rank) / 10) : 0) : normalizedFtsScore(row.fts_rank, maxFtsMagnitude) - const importanceScore = row.importance / 10 // migration 004 (v2.2): age based on event_time when set, else created_at fallback. // Lets temporal queries ("what did I do last June?") match by when the event happened, // not when it was recorded. @@ -1902,7 +1901,10 @@ export function recallMemories(opts = {}) { const accessScore = Math.min(1, Math.log1p(row.access_count || 0) / Math.log1p(20)) const levelWeight = LEVEL_WEIGHT[row.memory_level] || 1.0 - const baseScore = (ftsScore * 0.55) + (accessScore * 0.2) + (timeScore * 0.15) + (importanceScore * 0.1) + // Dropped here too — same measurement, and this path weighted it twice as + // heavily as the hybrid one. Weights renormalized across the three surviving + // signals so the score keeps its 0-1 shape. + const baseScore = (ftsScore * 0.61) + (accessScore * 0.22) + (timeScore * 0.17) // migration 003: decay_score as multiplier (periodically updated by runDecayCycle) // Defaults to 1.0 for records that haven't been through a decay cycle — backward compatible const decay = (row.decay_score != null) ? row.decay_score : 1.0 @@ -2198,7 +2200,6 @@ export async function recallMemoriesHybrid(opts = {}) { dropped: Math.max(0, rrfScores.size - fusedCandidates.length), }) let merged = fusedCandidates.map(({ row, rrf, sources }) => { - const importanceScore = row.importance / 10 // migration 004 (v2.2): age based on event_time when set, else created_at fallback const effectiveTime = row.event_time != null ? row.event_time : row.created_at const age = now - effectiveTime @@ -2214,7 +2215,22 @@ export async function recallMemoriesHybrid(opts = {}) { // recalled->access++->ranked-higher loop), now the primary structural tiebreak. const freqScore = Math.min(1, Math.log1p(row.access_count || 0) / Math.log1p(20)) const decay = (row.decay_score != null) ? row.decay_score : 1.0 - const score = (rrf * 10 + freqScore * 0.10 + timeScore * 0.06 + importanceScore * 0.05) * decay + // importance is NOT in this sum, deliberately. It was demoted to 0.05 rather + // than removed, on the reasoning that a weak prior is harmless. Measured + // against RRF's actual spacing it is not: adjacent ranks differ by ~0.0026, + // so 0.05 buys imp8-over-imp7 two places and imp9-over-imp7 four. In a top-8 + // injection, four places decides whether a row is seen at all. + // + // And the field it spent that leverage on carries no signal: on a live + // 8.6k-row store, rows ever recalled average importance 7.70 and rows never + // recalled 7.36 — a 0.34 gap on a 1-10 scale, 90% of the corpus at >=7. A + // self-rated number that does not predict use should not move results past + // genuine retrieval evidence. + // + // importance keeps what it is good at: the min_importance filter, + // surface-cold thresholds, display. Filtering on it is a caller stating a + // floor; ranking on it is the store guessing. + const score = (rrf * 10 + freqScore * 0.10 + timeScore * 0.06) * decay const temporalMetadata = temporalWindow ? { temporal_match: isInTemporalWindow(row, temporalWindow) } : {} diff --git a/level-rank-offset.test.mjs b/level-rank-offset.test.mjs index 8e49e1a..36c52cb 100644 --- a/level-rank-offset.test.mjs +++ b/level-rank-offset.test.mjs @@ -22,7 +22,10 @@ try { const offsetMatch = hybridSource.match(/const LEVEL_RANK_OFFSET = (\{[^\n]+\})/) const rrfMatch = source.match(/const RRF_K = (\d+)/) const contributionMatch = hybridSource.match(/const contribution = 1 \/ \(RRF_K \+ idx \+ 1 \+ offset\)/) - const scoreMatch = hybridSource.match(/const score = \(rrf \* 10 \+ freqScore \* 0\.10 \+ timeScore \* 0\.06 \+ importanceScore \* 0\.05\) \* decay/) + // Pins the composite's shape. importanceScore left the sum deliberately: a + // self-rated field measured to be uncorrelated with use was buying 2-4 rank + // positions against RRF's ~0.0026 spacing. See ranking-importance.integration.test.mjs. + const scoreMatch = hybridSource.match(/const score = \(rrf \* 10 \+ freqScore \* 0\.10 \+ timeScore \* 0\.06\) \* decay/) const levelWeightMatch = hybridSource.match(/const levelWeight = LEVEL_WEIGHT\[row\.memory_level\] \|\| 1\.0/) check('hybrid fusion declares the intended level rank offsets', diff --git a/ranking-importance.integration.test.mjs b/ranking-importance.integration.test.mjs new file mode 100644 index 0000000..b546de2 --- /dev/null +++ b/ranking-importance.integration.test.mjs @@ -0,0 +1,73 @@ +// Importance filters; it does not rank. +// +// It used to do both. The ranking half was demoted to a small weight rather than +// removed, on the reasoning that a weak prior is harmless. Measured against RRF's +// actual spacing it is not: adjacent ranks differ by ~0.0026, so 0.05 moves an +// importance-8 row two places past an importance-7 row, and an importance-9 row +// four. A top-8 injection is decided inside that range. +// +// The field was spending that leverage on nothing. On a live 8.6k-row store, rows +// ever recalled averaged importance 7.70 and rows never recalled 7.36 — a 0.34 gap +// on a 1-10 scale with 90% of the corpus at >=7. Self-rated, saturated, and +// uncorrelated with use. +// +// The fixture below is calibrated to the failure regime rather than to an obvious +// case: an earlier version gave the low-importance row a large relevance edge, +// which survived the tilt, so the test passed with and without the fix and proved +// nothing. Here relevance is a tie and the low-importance row's only advantage is +// one prior access — worth ~0.0455, deliberately less than the ~0.07 that an +// importance gap of 10-vs-3 was buying. Restore the importance term and this goes +// red. +// +// Covers the FTS path (no embedding config in a temp DB, so hybrid falls back). +// +// Run: node ranking-importance.integration.test.mjs +import { initMemory, closeMemory, storeMemory, recallMemories } from './index.mjs' +import Database from 'better-sqlite3' + +const DB_PATH = process.env.TOKENMEM_DB_PATH +if (!DB_PATH) { console.error('FATAL: set TOKENMEM_DB_PATH'); process.exit(2) } + +let pass = 0, fail = 0 +const check = (label, cond, detail = '') => { + if (cond) { pass++; console.log(`✓ ${label}`) } + else { fail++; console.log(`✗ ${label}${detail ? ' — ' + detail : ''}`) } +} + +initMemory() +const marker = `zzrank${Math.floor(Math.random() * 1e6)}` + +const used = storeMemory({ + content: `${marker} calibration runbook alpha`, + importance: 3, memoryLevel: 'semi_abstract', memoryType: 'long_term', +}) +const rated = storeMemory({ + content: `${marker} calibration runbook bravo`, + importance: 10, memoryLevel: 'semi_abstract', memoryType: 'long_term', +}) +{ + const db = new Database(DB_PATH) + db.prepare('UPDATE memories SET access_count = 1 WHERE rowid = ?').run(used) + db.prepare('UPDATE memories SET access_count = 0 WHERE rowid = ?').run(rated) + db.close() +} + +const hits = recallMemories({ query: marker, limit: 10 }) +const ids = hits.map(h => String(h.rowid)) +check('both rows retrieved', ids.includes(String(used)) && ids.includes(String(rated)), JSON.stringify(ids)) + +check('evidence of use outranks a higher self-rating', + ids.indexOf(String(used)) < ids.indexOf(String(rated)), + `used(imp3,acc1)@${ids.indexOf(String(used))} rated(imp10,acc0)@${ids.indexOf(String(rated))}`) + +const filtered = recallMemories({ query: marker, limit: 10, minImportance: 8 }) +const fids = filtered.map(h => String(h.rowid)) +check('min_importance still excludes below the floor', + !fids.includes(String(used)) && fids.includes(String(rated)), JSON.stringify(fids)) +check('every row returned under a floor satisfies it', + filtered.every(h => (h.importance || 0) >= 8), JSON.stringify(filtered.map(h => h.importance))) +check('importance still travels on the result', typeof hits[0]?.importance === 'number') + +closeMemory() +console.log(`\n${fail === 0 ? 'PASS' : 'FAIL'}: ${pass} passed / ${fail} failed`) +process.exit(fail === 0 ? 0 : 1)