From 74e039f546942a324dff3232fbdc6d347dc9fc94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8D=83=E5=A4=8F?= Date: Tue, 1 Sep 2026 21:20:27 +0800 Subject: [PATCH] fix(recall): cold pool selects by demonstrated reuse, not by self-rated importance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cold pool produces the "oh, I just remembered something" surface: when a recall comes back short, 25% of the time it adds 1-3 rows that are old, untouched, and still worth seeing. "Still worth seeing" was implemented as `importance >= 8`. Measured on a 9301-row library: that term excluded 441 of 1124 eligible rows, and the excluded set was led by the single most-recalled memory in the database — access_count=1879, importance=5. Also in there: acc=721, 634, 604, 529, 495, all rated 5-7 at write time. The gate was doing the opposite of its stated purpose. It filtered out the rows with the strongest evidence of being worth remembering, because someone typed a number when writing them and never revisited it. importance is 89% saturated at >=7 across the library, so as a selector it carries almost no signal — and where it does discriminate, it discriminates wrong. The decay floor already does this job on earned signal. decay_score is w(age) * reuseBoost(access_count), so a row cold for 30 days only still holds decay_score >= 0.3 if it was reused heavily. Measured on the same library, every row reaching the pool had access_count >= 8, average 135. Selecting by reuse is what "still important" was always trying to approximate — we just had a proxy sitting right there and used a self-report instead. Dropping the term widens the pool 683 -> 1124 and admits exactly the rows with the best track record. ## Index The index definition lived in three places that all had to agree: schema.sql (fresh DBs), migrations/003 (canonical), and an inline copy in index.mjs (old-DB upgrades). All three led on importance and were partial on `importance >= 8`, so with that term gone from the query SQLite could no longer use any of them and would have fallen back to a scan. All three now lead on last_accessed, which is what the pool range-filters. The index.mjs migration block re-runs on every initMemory(), so the DROP is guarded on the old definition actually being present — an unguarded drop would rebuild the index at every process start, at a cost that grows with the library. Caught by reading the surrounding block rather than by a test; then verified both directions: fresh DB: run 1 -> 0 rebuilds, run 2 -> 0 (all three definitions agree) old DB: run 1 -> 1 rebuild, run 2 -> 0 (upgrades once, then never) ## Also - split the query out as selectColdPoolCandidates so it is testable without fighting Math.random() and ORDER BY RANDOM() ## Verification Red-then-green checked properly rather than assumed. With `importance >= 8` restored, exactly the one assertion that matters fails (5 pass / 1 fail) — the other five stay green, so they are not accidentally coupled to it. Full suite green: cold-pool 6 · recall-contract 14 · ranking-importance 5 · anchor-pinned 4 · level-migration 10 · supersede-shrink 24 · recall-endpoint 17 · query-rewrite 6 · memory-health 68 · locations 51 · provenance 25 · encoding-damage 18 · injection-hygiene 14 · hooks 12 · series-detection 11. [prediction] 修复: 冷池复现从"半个库里随机捞"变成"复用记录最强的那批里随机捞", 长期没碰但真正常用的条目重新有机会浮上来 [prediction] 风险: 池子从 683 涨到 1124,单条被抽中的概率下降约 40%。若实际感受 变成"总冒出不相干的",说明 decay_score>=0.3 这个地板对本库太松,调地板而不是 把 importance 加回来 [prediction] 验证: 下次 recall 返回 surfaced_random 时,命中条目的 access_count 应普遍 >= 8;若出现 access_count 很低的条目,说明 decay 公式或地板需要复核 Co-Authored-By: 千夏 --- .github/workflows/ci.yml | 5 ++ cold-pool-gate.test.mjs | 99 +++++++++++++++++++++++++ index.mjs | 65 ++++++++++++---- migrations/003-add-decay-and-priors.sql | 11 ++- schema.sql | 9 ++- 5 files changed, 169 insertions(+), 20 deletions(-) create mode 100644 cold-pool-gate.test.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ad8216b..13743a1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -113,5 +113,10 @@ jobs: TOKENMEM_DB_PATH: ${{ runner.temp }}/mneme-ci-query-rewrite.db run: node query-rewrite.test.mjs + - name: Integration test (cold pool selects by reuse, not self-rated importance) + env: + TOKENMEM_DB_PATH: ${{ runner.temp }}/mneme-ci-cold-pool.db + run: node cold-pool-gate.test.mjs + - name: Adapter test (codex-recovery-proxy fault injection) run: node adapters/codex-recovery-proxy/proxy.test.mjs diff --git a/cold-pool-gate.test.mjs b/cold-pool-gate.test.mjs new file mode 100644 index 0000000..d5e426a --- /dev/null +++ b/cold-pool-gate.test.mjs @@ -0,0 +1,99 @@ +// End-to-end: cold-pool candidate selection is by demonstrated reuse, not self-rating +// Run: node cold-pool-gate.test.mjs +// +// The cold pool produces the "oh, I just remembered something" surface: when a +// recall comes back short, 25% of the time it adds 1-3 rows that are old, +// untouched, and still worth seeing. +// +// "Still worth seeing" was implemented as importance >= 8 — a number the author +// types at write time. Measured on a 9301-row library (2026-09-01), that gate +// excluded 441 rows from a 1124-row pool, and the excluded set was led by the +// single most-recalled memory in the whole database (access_count = 1879, +// importance = 5). The decay floor the pool already applies is the real filter: +// a 30-day-cold row only holds decay_score >= 0.3 if it was reused a lot, so +// every row reaching the pool has access_count >= 8 by construction. + +import { initMemory, selectColdPoolCandidates, closeMemory } from './index.mjs' +import Database from 'better-sqlite3' +import { resolve, dirname } from 'node:path' +import { fileURLToPath } from 'node:url' +import { existsSync } from 'node:fs' + +initMemory() + +const __dirname_at = dirname(fileURLToPath(import.meta.url)) +const DB_PATH = process.env.TOKENMEM_DB_PATH + || (existsSync(resolve(__dirname_at, 'engram.db')) + ? resolve(__dirname_at, 'engram.db') + : resolve(__dirname_at, 'tokenmem.db')) + +const SIMPLE_EXT_PATH = resolve(__dirname_at, 'lib', 'libsimple-windows-x64', 'simple') +function tryLoadSimple(db) { + try { + if (existsSync(SIMPLE_EXT_PATH + '.dll') || existsSync(SIMPLE_EXT_PATH)) { + db.loadExtension(SIMPLE_EXT_PATH) + } + } catch {} +} + +const db = new Database(DB_PATH) +tryLoadSimple(db) + +let pass = 0, fail = 0 +const ok = (name, cond) => { + if (cond) { pass++; console.log(`✓ ${name}`) } + else { fail++; console.log(`✗ ${name}`) } +} + +const DAY = 86400_000 +const now = Date.now() +const cold = now - 45 * DAY // comfortably past the 30d staleness cutoff +const warm = now - 2 * DAY +const TAG = `coldpool-${process.pid}` + +const insert = db.prepare(` + INSERT INTO memories (id, content, summary, importance, access_count, + last_accessed, created_at, decay_score, memory_level, memory_type) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, 'semi_abstract', 'long_term') +`) +const add = (id, imp, acc, last, decay) => + insert.run(`${TAG}-${id}`, `cold pool fixture ${id}`, `fixture ${id}`, imp, acc, last, cold, decay) + +// The row the old gate wrongly excluded: heavily reused, self-rated low. +add('proven-but-low-rated', 5, 1879, cold, 0.9) +// The row the old gate admitted on its rating alone. +add('rated-high', 9, 12, cold, 0.5) +// Must stay out: not cold yet. +add('too-warm', 10, 400, warm, 0.9) +// Must stay out: decayed below the floor. +add('below-decay-floor', 10, 400, cold, 0.1) + +const mine = (rows) => new Set( + rows.filter(r => String(r.id).startsWith(TAG)).map(r => String(r.id).slice(TAG.length + 1))) + +// take is deliberately large: selection is ORDER BY RANDOM(), so asserting on a +// small sample would be flaky. We assert on eligibility, not on which row won. +const picked = mine(selectColdPoolCandidates(db, { take: 5000, nowMs: now })) + +ok('a heavily reused memory is eligible even when its importance is low', + picked.has('proven-but-low-rated')) +ok('a high self-rating still gets in — this widens the pool, it does not invert it', + picked.has('rated-high')) +ok('the staleness cutoff still holds', !picked.has('too-warm')) +ok('the decay floor still holds — it is the real relevance filter', + !picked.has('below-decay-floor')) + +const all = selectColdPoolCandidates(db, { take: 5000, nowMs: now }) +const drop = all[0]?.rowid +const rest = selectColdPoolCandidates(db, { take: 5000, nowMs: now, excludeRowids: [drop] }) +ok('excluded rowids are honoured so the surface cannot repeat a hit', + drop != null && !rest.some(r => r.rowid === drop)) + +ok('take caps the result size', selectColdPoolCandidates(db, { take: 1, nowMs: now }).length === 1) + +db.prepare(`DELETE FROM memories WHERE id LIKE ?`).run(`${TAG}-%`) +db.close() +closeMemory() + +console.log(`\n${fail ? 'FAIL' : 'PASS'}: ${pass} passed / ${fail} failed`) +process.exit(fail ? 1 : 0) diff --git a/index.mjs b/index.mjs index e17878c..c2773e5 100644 --- a/index.mjs +++ b/index.mjs @@ -348,7 +348,22 @@ export function initMemory() { log('Migration: added prior_versions column') } catch {} try { - db.exec(`CREATE INDEX IF NOT EXISTS idx_mem_surface_pool ON memories(importance, last_accessed, decay_score) WHERE deleted_at IS NULL AND superseded_by IS NULL AND importance >= 8`) + // Partial index for the cold pool. Rebuilt 2026-09-01: the old definition led + // on importance and was partial on `importance >= 8`. With that term gone from + // the pool query, SQLite can no longer use it and falls back to a scan. The + // new one leads on last_accessed, which is what the query range-filters. + // + // This whole migration block re-runs on every initMemory(), so the DROP is + // guarded on the old definition actually being present — otherwise every + // process start would rebuild the index, and that cost grows with the library. + const surfaceIdx = db.prepare( + `SELECT sql FROM sqlite_master WHERE type = 'index' AND name = 'idx_mem_surface_pool'` + ).get() + if (surfaceIdx?.sql && /importance/i.test(surfaceIdx.sql)) { + db.exec(`DROP INDEX idx_mem_surface_pool`) + log('Migration: rebuilt idx_mem_surface_pool (was keyed on importance)') + } + db.exec(`CREATE INDEX IF NOT EXISTS idx_mem_surface_pool ON memories(last_accessed, decay_score) WHERE deleted_at IS NULL AND superseded_by IS NULL`) } catch {} // migration 008 (v2.6): is_anchor / is_pinned scarcity-as-structure layer. // Ombre-Brain inspired: importance 1-10 is a weak prior that gets inflated @@ -1941,7 +1956,7 @@ export function recallMemories(opts = {}) { let result = filtered.slice(0, limit) // migration 003: surfaced_random — when result < limit, 25% chance of pulling - // 1-3 records from the cold pool (importance >= 8 AND 30d untouched AND decay >= 0.3) + // 1-3 records from the cold pool (30d untouched AND decay >= 0.3 — see selectColdPoolCandidates) // Skipped when called internally by recallMemoriesHybrid (avoid double-surfacing) if (!opts._internal && queryText && result.length < limit) { const surfaced = surfaceRandomMemories(db, result.map(r => r.rowid), limit - result.length, now) @@ -2635,21 +2650,34 @@ export async function buildMemoryContext(opts = {}) { // ── migration 003: surfaced_random pool ───────────────────── // When recall result count < limit, with 25% probability, surface 1-3 records -// from the cold pool: importance >= 8 AND last_accessed < (now - 30d) -// AND decay_score >= 0.3 AND deleted_at IS NULL AND superseded_by IS NULL. +// from the cold pool: last_accessed < (now - 30d) AND decay_score >= 0.3 +// AND deleted_at IS NULL AND superseded_by IS NULL. // Models the "I just remembered something" feeling — covers cold-recall blind -// spots and lets long-decayed but still-important memories resurface. +// spots and lets long-decayed but still-useful memories resurface. +// +// 2026-09-01: dropped the importance >= 8 term. It was meant to read as "still +// important", but importance is self-rated at write time and 89% of the library +// sits at >= 7, so it was not selecting for importance — it was excluding 441 of +// 1124 eligible rows on the basis of a number nobody calibrated. The excluded set +// was led by the single most-recalled memory in the database (access_count=1879, +// importance=5). +// +// The decay floor already does this job, and does it on earned signal: decay is +// w(age) * reuseBoost(access_count), so a row that has been cold for 30 days only +// still holds decay_score >= 0.3 if it was reused heavily. Measured on the same +// library, every row reaching the pool had access_count >= 8 (avg 135). Selecting +// by reuse is what "still worth remembering" was always trying to approximate. const SURFACE_RANDOM_PROB = 0.25 const SURFACE_RANDOM_MAX = 3 const SURFACE_AGE_MS = 30 * 86400_000 const SURFACE_DECAY_FLOOR = 0.3 -const SURFACE_IMPORTANCE_MIN = 8 -function surfaceRandomMemories(db, excludeRowids, slotsAvailable, nowMs) { - if (slotsAvailable <= 0) return [] - if (Math.random() > SURFACE_RANDOM_PROB) return [] - const cutoff = nowMs - SURFACE_AGE_MS - const take = Math.min(SURFACE_RANDOM_MAX, slotsAvailable) +/** + * The cold-pool query, split out from the probability roll so it can be tested + * without fighting Math.random() or ORDER BY RANDOM(). + */ +export function selectColdPoolCandidates(db, { excludeRowids, take, nowMs } = {}) { + const cutoff = (nowMs ?? Date.now()) - SURFACE_AGE_MS const excludeClause = excludeRowids?.length ? `AND rowid NOT IN (${excludeRowids.map(() => '?').join(',')})` : '' @@ -2658,13 +2686,12 @@ function surfaceRandomMemories(db, excludeRowids, slotsAvailable, nowMs) { SELECT rowid, * FROM memories WHERE deleted_at IS NULL AND superseded_by IS NULL - AND importance >= ? AND last_accessed < ? AND decay_score >= ? ${excludeClause} ORDER BY RANDOM() LIMIT ? - `).all(SURFACE_IMPORTANCE_MIN, cutoff, SURFACE_DECAY_FLOOR, ...(excludeRowids || []), take) + `).all(cutoff, SURFACE_DECAY_FLOOR, ...(excludeRowids || []), take ?? SURFACE_RANDOM_MAX) return rows.map(r => ({ ...r, score: 0, @@ -2673,11 +2700,21 @@ function surfaceRandomMemories(db, excludeRowids, slotsAvailable, nowMs) { recall_source: 'surfaced_random', // callers can distinguish from query matches })) } catch (e) { - log(`surfaceRandomMemories failed: ${e.message}`) + log(`selectColdPoolCandidates failed: ${e.message}`) return [] } } +function surfaceRandomMemories(db, excludeRowids, slotsAvailable, nowMs) { + if (slotsAvailable <= 0) return [] + if (Math.random() > SURFACE_RANDOM_PROB) return [] + return selectColdPoolCandidates(db, { + excludeRowids, + take: Math.min(SURFACE_RANDOM_MAX, slotsAvailable), + nowMs, + }) +} + // ── Memory Management ─────────────────────────────────────── /** Clean up expired memories (both TTL path and supersede chain) */ diff --git a/migrations/003-add-decay-and-priors.sql b/migrations/003-add-decay-and-priors.sql index 1f0a2a6..e370c9a 100644 --- a/migrations/003-add-decay-and-priors.sql +++ b/migrations/003-add-decay-and-priors.sql @@ -20,10 +20,15 @@ ALTER TABLE memories ADD COLUMN decay_score REAL NOT NULL DEFAULT 1.0; -- 2. prior_versions -- paper trail; on supersede, push old content/summary/ts into this array ALTER TABLE memories ADD COLUMN prior_versions TEXT NOT NULL DEFAULT '[]'; --- 3. Index for surfaced_random cold pool (small subset; index accelerates RANDOM() sampling) +-- 3. Index for surfaced_random cold pool (index accelerates RANDOM() sampling). +-- 2026-09-01: was ON (importance, last_accessed, decay_score) partial on +-- `importance >= 8`. The pool no longer filters on importance -- it selects by +-- staleness + decay, which is reuse-derived rather than self-rated -- so the old +-- index could not serve the query. Leads on last_accessed, which the pool +-- range-filters. index.mjs carries a matching guarded rebuild for existing DBs. CREATE INDEX IF NOT EXISTS idx_mem_surface_pool - ON memories(importance, last_accessed, decay_score) - WHERE deleted_at IS NULL AND superseded_by IS NULL AND importance >= 8; + ON memories(last_accessed, decay_score) + WHERE deleted_at IS NULL AND superseded_by IS NULL; -- Verify: -- PRAGMA table_info(memories); diff --git a/schema.sql b/schema.sql index 94c8db1..2f9e475 100644 --- a/schema.sql +++ b/schema.sql @@ -93,10 +93,13 @@ CREATE INDEX IF NOT EXISTS idx_mem_superseded_by ON memories(superseded_by) WHERE superseded_by IS NOT NULL AND deleted_at IS NULL; --- v2.1: Surfaced-random cold pool index (importance >= 8 AND 30d untouched AND decay >= 0.3) +-- v2.1: Surfaced-random cold pool index (30d untouched AND decay >= 0.3). +-- 2026-09-01: dropped the importance leg. The pool selects by staleness + decay +-- (reuse-derived) rather than by self-rated importance, so an importance-keyed +-- partial index could not serve the query. CREATE INDEX IF NOT EXISTS idx_mem_surface_pool - ON memories(importance, last_accessed, decay_score) - WHERE deleted_at IS NULL AND superseded_by IS NULL AND importance >= 8; + ON memories(last_accessed, decay_score) + WHERE deleted_at IS NULL AND superseded_by IS NULL; -- FTS5 virtual table (full-text search) -- Default tokenize='unicode61' (built-in, zero-dependency — boots on any SQLite,