From d5afec8cdbe12a316a1d3b322f272cd12966587f Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Fri, 24 Jul 2026 12:30:18 -0700 Subject: [PATCH] fix: health ring never fakes a learning backslide across projects The health-history ring is machine-global (kit.json) but learningRows is project-local (the sync cwd's .claude-flow/neural/stats.json), and an absent store was recorded as a literal 0. A sync run from a project with no learning store therefore compared 0 against another project's real count and fired a false 'learning rows shrank 75 -> 0' alarm. Two-part fix, mirroring the admin page's honesty rule (unknown is never a fabricated zero): - sync stamps each snapshot with its project (cwd) and records learningRows: null when the store is absent - detectRegression compares learningRows only against the most recent prior entry from the SAME project with a KNOWN count; other projects, legacy unstamped entries, and unknown readings are never baselines. Machine-global metrics (nativeSlots, drift, security) still compare the last two entries as before. The alarm message now names the project so a real backslide says where it happened. --- src/commands/sync.mjs | 7 +++- src/lib/health-history.mjs | 55 ++++++++++++++++++------- tests/health-history.test.cjs | 77 +++++++++++++++++++++++++++++++++-- 3 files changed, 120 insertions(+), 19 deletions(-) diff --git a/src/commands/sync.mjs b/src/commands/sync.mjs index f39e31e1..2d9545cf 100644 --- a/src/commands/sync.mjs +++ b/src/commands/sync.mjs @@ -218,7 +218,12 @@ export async function run({ flags, pkgRoot }) { const stats = readJson(path.join(paths.projectClaudeFlowDir(cwd), 'neural', 'stats.json')); appendToConfig(cfg, { ts: Math.floor(Date.now() / 1000), - learningRows: stats?.patternsLearned ?? 0, + // learningRows is PROJECT-local (this cwd's learning store) in a MACHINE- + // global ring, so stamp the project and record null (unknown) when the + // store is absent — a fabricated 0 would let a sync run from a store-less + // project fake a "learning shrank" alarm against another project's count. + project: cwd, + learningRows: Number.isFinite(stats?.patternsLearned) ? stats.patternsLearned : null, // Count NATIVE bindings (incl. the aqe slot), not directories: a location // flipping native→WASM must move this number or the regression detector // named "native agentdb slots dropped" can never fire; and a benign tree diff --git a/src/lib/health-history.mjs b/src/lib/health-history.mjs index 725bf92c..15498f2a 100644 --- a/src/lib/health-history.mjs +++ b/src/lib/health-history.mjs @@ -3,18 +3,31 @@ // last two and alarms on any backslide (learning shrank, native agentdb slots // dropped, drift regressed current→outdated, security present→absent). // +// The ring is MACHINE-global (kit.json) but learningRows is PROJECT-local (read +// from the sync cwd's .claude-flow/neural/stats.json), so entries carry the +// project they were recorded from and the learning comparison only ever pairs +// entries from the SAME project. An absent learning store records null (unknown), +// never a fabricated 0 — the same honesty rule the admin page lives by — so a +// sync run from a store-less project can never fake a "learning shrank" alarm. +// // The core (append / summarize / detectRegression) is PURE — no file I/O. The // loadRing / appendToConfig shims only read/mutate a plain cfg object so the // caller can persist via saveKitConfig; they have no side effects beyond the cfg. // // An entry looks like: -// { ts, learningRows, nativeSlots, driftOutdated: bool, securityPresent: bool } +// { ts, project, learningRows: number|null, nativeSlots, driftOutdated: bool, securityPresent: bool } const DEFAULT_CAP = 30; /** Coerce a possibly-missing numeric field to a finite number (default 0). */ const num = (v) => (Number.isFinite(v) ? v : 0); +/** A finite count, or null for unknown — never a fabricated 0. */ +const numOrNull = (v) => (Number.isFinite(v) ? v : null); + +/** Last path segment for display (handles / and \ so messages read the same on Windows). */ +const projLabel = (p) => String(p).split(/[\\/]/).filter(Boolean).pop() ?? String(p); + /** * Append `entry` to `ring`, returning a NEW array capped at `cap` entries. * Oldest entries past the cap are dropped (FIFO). Never mutates the input. @@ -24,10 +37,12 @@ export function append(ring, entry, cap = DEFAULT_CAP) { return next.length > cap ? next.slice(next.length - cap) : next; } -/** Project an entry down to just the tracked scalar fields. */ +/** Project an entry down to just the tracked scalar fields. learningRows and + * project keep null for "unknown" — an absent learning store is not a zero. */ export function summarize(entry = {}) { return { - learningRows: num(entry.learningRows), + project: typeof entry.project === 'string' && entry.project ? entry.project : null, + learningRows: numOrNull(entry.learningRows), nativeSlots: num(entry.nativeSlots), driftOutdated: Boolean(entry.driftOutdated), securityPresent: Boolean(entry.securityPresent), @@ -35,11 +50,14 @@ export function summarize(entry = {}) { } /** - * Compare the last two entries of `ring` and return an array of regressions: - * { metric, from, to, message } - * Regressions: learningRows shrank, nativeSlots dropped, drift current→outdated, - * security present→absent. Recoveries (the reverse) are never flagged. Fewer than - * two entries → []. Missing numeric fields count as 0; missing bools as falsy. + * Compare the newest entry of `ring` against its baselines and return an array + * of regressions: { metric, from, to, message }. + * + * Machine-global metrics (nativeSlots, drift, security) compare the last two + * entries. learningRows is project-local, so its baseline is the most recent + * PRIOR entry from the SAME project with a KNOWN count — entries from other + * projects, legacy entries with no project stamp, and unknown (null) readings + * are never compared. Recoveries are never flagged. Fewer than two entries → []. */ export function detectRegression(ring) { if (!Array.isArray(ring) || ring.length < 2) return []; @@ -47,13 +65,20 @@ export function detectRegression(ring) { const curr = summarize(ring[ring.length - 1]); const out = []; - if (curr.learningRows < prev.learningRows) { - out.push({ - metric: 'learningRows', - from: prev.learningRows, - to: curr.learningRows, - message: `learning rows shrank ${prev.learningRows} → ${curr.learningRows}`, - }); + if (curr.learningRows != null && curr.project != null) { + let base = null; + for (let i = ring.length - 2; i >= 0; i--) { + const s = summarize(ring[i]); + if (s.project === curr.project && s.learningRows != null) { base = s; break; } + } + if (base && curr.learningRows < base.learningRows) { + out.push({ + metric: 'learningRows', + from: base.learningRows, + to: curr.learningRows, + message: `learning rows shrank ${base.learningRows} → ${curr.learningRows} (${projLabel(curr.project)})`, + }); + } } if (curr.nativeSlots < prev.nativeSlots) { out.push({ diff --git a/tests/health-history.test.cjs b/tests/health-history.test.cjs index 817520e9..6bf1323d 100644 --- a/tests/health-history.test.cjs +++ b/tests/health-history.test.cjs @@ -29,7 +29,7 @@ function eq(a, b, msg) { assert(A === B, (msg || 'not equal') + `\n got: ${A}\n expected: ${B}`); } const entry = (o = {}) => ({ - ts: 1000, learningRows: 10, nativeSlots: 5, driftOutdated: false, securityPresent: true, ...o, + ts: 1000, project: '/p/alpha', learningRows: 10, nativeSlots: 5, driftOutdated: false, securityPresent: true, ...o, }); // ── append: cap behavior + immutability ────────────────────────────────────── @@ -75,7 +75,8 @@ test('append onto undefined/missing ring treats it as empty', () => { console.log('\nsummarize'); test('summarize projects an entry to the tracked scalar fields', () => { - const s = summarize({ ts: 9, learningRows: 3, nativeSlots: 2, driftOutdated: true, securityPresent: false, junk: 'x' }); + const s = summarize({ ts: 9, project: '/p/a', learningRows: 3, nativeSlots: 2, driftOutdated: true, securityPresent: false, junk: 'x' }); + eq(s.project, '/p/a'); eq(s.learningRows, 3); eq(s.nativeSlots, 2); eq(s.driftOutdated, true); @@ -83,6 +84,13 @@ test('summarize projects an entry to the tracked scalar fields', () => { assert(!('junk' in s), 'summarize drops untracked fields'); }); +test('summarize keeps null (unknown) for absent learningRows/project — never a fabricated 0', () => { + const s = summarize({ ts: 9 }); + eq(s.learningRows, null, 'absent learning store is unknown, not 0'); + eq(s.project, null, 'legacy entry has no project stamp'); + eq(s.nativeSlots, 0, 'machine-global counts still default to 0'); +}); + // ── detectRegression: every branch ─────────────────────────────────────────── console.log('\ndetectRegression (every branch)'); @@ -181,11 +189,74 @@ test('only the LAST two entries are compared', () => { eq(detectRegression(ring), []); }); -test('missing numeric fields are treated as 0 (no spurious regression, no crash)', () => { +test('bare entries (no fields at all) → no spurious regression, no crash', () => { const ring = [{ ts: 1 }, { ts: 2 }]; eq(detectRegression(ring), []); }); +// ── detectRegression: per-project learning comparison + unknown semantics ──── +console.log('\ndetectRegression (per-project learning, unknown ≠ 0)'); + +test('a sync from a project with NO learning store (null) never alarms against another project\'s count', () => { + // the real-world false positive this guards: sync ran in project beta (no + // store) right after a sync in alpha had recorded 75 — 75 → 0 is a lie. + const ring = [ + entry({ project: '/p/alpha', learningRows: 75 }), + entry({ project: '/p/beta', learningRows: null }), + ]; + eq(detectRegression(ring), []); +}); + +test('a KNOWN count in one project is never compared against a different project\'s count', () => { + const ring = [ + entry({ project: '/p/alpha', learningRows: 75 }), + entry({ project: '/p/beta', learningRows: 3 }), // beta has no prior entry → no baseline + ]; + eq(detectRegression(ring), []); +}); + +test('learning baseline skips other projects and unknowns to find the same project\'s last KNOWN count', () => { + const ring = [ + entry({ project: '/p/alpha', learningRows: 20 }), // ← the baseline + entry({ project: '/p/beta', learningRows: 999 }), // other project, skipped + entry({ project: '/p/alpha', learningRows: null }), // unknown reading, skipped + entry({ project: '/p/alpha', learningRows: 12 }), + ]; + const r = detectRegression(ring); + eq(r.length, 1); + eq(r[0].metric, 'learningRows'); + eq(r[0].from, 20); + eq(r[0].to, 12); +}); + +test('legacy entries with no project stamp are never used as a learning baseline', () => { + const ring = [ + entry({ project: undefined, learningRows: 75 }), // pre-fix entry + entry({ project: '/p/alpha', learningRows: 3 }), + ]; + eq(detectRegression(ring), []); +}); + +test('the learning message names the project (last path segment, either separator)', () => { + const posix = detectRegression([entry({ learningRows: 9 }), entry({ learningRows: 4 })]); + assert(posix[0].message.includes('(alpha)'), 'posix path label: ' + posix[0].message); + const win = detectRegression([ + entry({ project: 'C:\\dev\\gamma', learningRows: 9 }), + entry({ project: 'C:\\dev\\gamma', learningRows: 4 }), + ]); + assert(win[0].message.includes('(gamma)'), 'windows path label: ' + win[0].message); +}); + +test('machine-global metrics still compare across projects (a native-slot drop is machine truth)', () => { + const ring = [ + entry({ project: '/p/alpha', nativeSlots: 8 }), + entry({ project: '/p/beta', nativeSlots: 3 }), + ]; + const r = detectRegression(ring); + eq(r.length, 1); + eq(r[0].metric, 'nativeSlots'); +}); + // ── loadRing / appendToConfig (thin cfg shims) ─────────────────────────────── console.log('\nloadRing / appendToConfig');