fix(diagnostics): defensive guards on new validators (CodeRabbit #473 review)

CodeRabbit flagged two patterns in the per-store validators added in
the parent commit:

1. .trim() on .title / .narrative was unconditional — a corrupted row
   with title=null or title=42 would throw, abort the whole diagnose
   run, and silently skip every later category. Add typeof guards.

2. confidence range checks were `< 0 || > 1` which silently passes
   NaN and Infinity (NaN < 0 is false, NaN > 1 is false → "healthy").
   Add Number.isFinite(...) prefix so corrupted scored rows surface
   as warnings instead.

Applied across all 6 new validators: lesson confidence, summary title,
semantic confidence, crystal narrative, insight confidence.

Tests added in test/diagnostics.test.ts under "defensive row-shape
handling": NaN confidence on a lesson, null summary title (verifies
diagnose still completes and later categories still execute),
undefined crystal narrative, Infinity / NaN on insight + semantic.

34/34 tests pass.

Author: efenex

src/functions/diagnostics.ts

@@ -374,12 +374,21 @@ export function registerDiagnosticsFunction(sdk: ISdk, kv: StateKV): void {
         const live = lessons.filter((l) => !l.deleted);
         let lessonIssues = 0;
         for (const l of live) {
-          if (l.confidence < 0 || l.confidence > 1) {
+          // Number.isFinite rejects NaN / Infinity / non-numbers; a
+          // corrupted row passing those would silently survive the < / >
+          // range check (e.g. NaN < 0 is false, NaN > 1 is false, so the
+          // bad row would be "healthy") and skew memory_lesson_recall's
+          // scoring downstream. Surface as warning.
+          if (
+            !Number.isFinite(l.confidence) ||
+            l.confidence < 0 ||
+            l.confidence > 1
+          ) {
             checks.push({
               name: `lesson-bad-confidence:${l.id}`,
               category: "lessons",
               status: "warn",
-              message: `Lesson ${l.id} has confidence ${l.confidence} (expected 0..1)`,
+              message: `Lesson ${l.id} has confidence ${l.confidence} (expected finite number in 0..1)`,
               fixable: false,
             });
             lessonIssues++;
@@ -400,7 +409,10 @@ export function registerDiagnosticsFunction(sdk: ISdk, kv: StateKV): void {
         const summaries = await kv.list<SessionSummary>(KV.summaries);
         let summaryIssues = 0;
         for (const s of summaries) {
-          if (!s.title || !s.title.trim()) {
+          // typeof guard before .trim() — a corrupted row with title=null
+          // or title=42 would otherwise throw and abort the whole diagnose
+          // run before later categories get checked.
+          if (typeof s.title !== "string" || s.title.trim().length === 0) {
             checks.push({
               name: `summary-missing-title:${s.sessionId}`,
               category: "summaries",
@@ -426,12 +438,16 @@ export function registerDiagnosticsFunction(sdk: ISdk, kv: StateKV): void {
         const semantic = await kv.list<SemanticMemory>(KV.semantic);
         let semanticIssues = 0;
         for (const s of semantic) {
-          if (s.confidence < 0 || s.confidence > 1) {
+          if (
+            !Number.isFinite(s.confidence) ||
+            s.confidence < 0 ||
+            s.confidence > 1
+          ) {
             checks.push({
               name: `semantic-bad-confidence:${s.id}`,
               category: "semantic",
               status: "warn",
-              message: `Semantic fact ${s.id} has confidence ${s.confidence} (expected 0..1)`,
+              message: `Semantic fact ${s.id} has confidence ${s.confidence} (expected finite number in 0..1)`,
               fixable: false,
             });
             semanticIssues++;
@@ -478,7 +494,7 @@ export function registerDiagnosticsFunction(sdk: ISdk, kv: StateKV): void {
         const crystals = await kv.list<Crystal>(KV.crystals);
         let crystalIssues = 0;
         for (const c of crystals) {
-          if (!c.narrative || !c.narrative.trim()) {
+          if (typeof c.narrative !== "string" || c.narrative.trim().length === 0) {
             checks.push({
               name: `crystal-empty-narrative:${c.id}`,
               category: "crystals",
@@ -504,12 +520,16 @@ export function registerDiagnosticsFunction(sdk: ISdk, kv: StateKV): void {
         const insights = await kv.list<Insight>(KV.insights);
         let insightIssues = 0;
         for (const i of insights) {
-          if (i.confidence < 0 || i.confidence > 1) {
+          if (
+            !Number.isFinite(i.confidence) ||
+            i.confidence < 0 ||
+            i.confidence > 1
+          ) {
             checks.push({
               name: `insight-bad-confidence:${i.id}`,
               category: "insights",
               status: "warn",
-              message: `Insight ${i.id} has confidence ${i.confidence} (expected 0..1)`,
+              message: `Insight ${i.id} has confidence ${i.confidence} (expected finite number in 0..1)`,
               fixable: false,
             });
             insightIssues++;

test/diagnostics.test.ts

@@ -761,5 +761,107 @@ describe("Diagnostics Functions", () => {
       expect(result.checks.some((c) => c.category === "lessons")).toBe(true);
       expect(result.checks.some((c) => c.category === "summaries")).toBe(true);
     });
+
+    describe("defensive row-shape handling (CodeRabbit #473 review)", () => {
+      it("NaN/Infinity confidence on a lesson is flagged as warn, not silently passed", async () => {
+        await kv.set(KV.lessons, "lsn_nan", {
+          id: "lsn_nan", content: "x", context: "", confidence: NaN,
+          reinforcements: 0, source: "manual", sourceIds: [], tags: [],
+          createdAt: "", updatedAt: "", decayRate: 0.05,
+        });
+
+        const result = (await sdk.trigger("mem::diagnose", {
+          categories: ["lessons"],
+        })) as { checks: DiagnosticCheck[] };
+
+        const warn = result.checks.find((c) => c.name.startsWith("lesson-bad-confidence:"));
+        expect(warn?.status).toBe("warn");
+      });
+
+      it("non-string summary title doesn't throw — surfaces as warn", async () => {
+        await kv.set(KV.summaries, "ses_bad_title", {
+          sessionId: "ses_bad_title",
+          project: "p",
+          createdAt: "",
+          title: null as unknown as string, // simulate corrupted row
+          narrative: "n",
+          keyDecisions: [],
+          filesModified: [],
+          concepts: [],
+          observationCount: 1,
+        });
+
+        // The bug to guard against: the old code called .trim() unconditionally,
+        // which throws on null/number, which aborts the whole diagnose run and
+        // any later category check never executes. Verify diagnose completes
+        // AND surfaces the bad row.
+        const result = (await sdk.trigger("mem::diagnose", {
+          categories: ["summaries", "lessons"],
+        })) as { checks: DiagnosticCheck[]; success?: boolean };
+
+        expect(result.success).toBe(true);
+        const warn = result.checks.find((c) => c.name.startsWith("summary-missing-title:"));
+        expect(warn?.status).toBe("warn");
+        // Later category still ran:
+        expect(result.checks.some((c) => c.category === "lessons")).toBe(true);
+      });
+
+      it("non-string crystal narrative doesn't throw — surfaces as warn", async () => {
+        await kv.set(KV.crystals, "cry_bad", {
+          id: "cry_bad",
+          narrative: undefined as unknown as string,
+          keyOutcomes: [],
+          filesAffected: [],
+          lessons: [],
+          sourceActionIds: [],
+          createdAt: "",
+        });
+
+        const result = (await sdk.trigger("mem::diagnose", {
+          categories: ["crystals"],
+        })) as { checks: DiagnosticCheck[]; success?: boolean };
+
+        expect(result.success).toBe(true);
+        const warn = result.checks.find((c) => c.name.startsWith("crystal-empty-narrative:"));
+        expect(warn?.status).toBe("warn");
+      });
+
+      it("Infinity confidence on insight + semantic both flagged", async () => {
+        await kv.set(KV.insights, "ins_inf", {
+          id: "ins_inf",
+          title: "t",
+          content: "c",
+          confidence: Infinity,
+          reinforcements: 0,
+          sourceConceptCluster: [],
+          sourceMemoryIds: [],
+          sourceLessonIds: [],
+          sourceCrystalIds: [],
+          tags: [],
+          createdAt: "",
+          updatedAt: "",
+          decayRate: 0.05,
+        });
+        await kv.set(KV.semantic, "sem_nan", {
+          id: "sem_nan",
+          fact: "f",
+          confidence: NaN,
+          sourceSessionIds: [],
+          sourceMemoryIds: [],
+          accessCount: 0,
+          lastAccessedAt: "",
+          strength: 0,
+          createdAt: "",
+          updatedAt: "",
+        });
+
+        const result = (await sdk.trigger("mem::diagnose", {
+          categories: ["insights", "semantic"],
+        })) as { checks: DiagnosticCheck[] };
+
+        expect(result.checks.find((c) => c.name === "insight-bad-confidence:ins_inf")?.status).toBe("warn");
+        expect(result.checks.find((c) => c.name === "semantic-bad-confidence:sem_nan")?.status).toBe("warn");
+      });
+    });
   });
 });