diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 45eb88d36..d12277c2d 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -249,25 +249,29 @@ export async function runDoctor(engine: BrainEngine | null, args: string[], dbSo // Without this doctor check, users see "sync blocked" and have no // surface showing which files to fix. try { - const { unacknowledgedSyncFailures, loadSyncFailures } = await import('../core/sync.ts'); + const { unacknowledgedSyncFailures, loadSyncFailures, summarizeFailuresByCode } = await import('../core/sync.ts'); const unacked = unacknowledgedSyncFailures(); const all = loadSyncFailures(); if (unacked.length > 0) { + const codeSummary = summarizeFailuresByCode(unacked); + const codeBreakdown = codeSummary.map(s => `${s.code}=${s.count}`).join(', '); const preview = unacked.slice(0, 3).map(f => `${f.path} (${f.error.slice(0, 60)})`).join('; '); checks.push({ name: 'sync_failures', status: 'warn', message: - `${unacked.length} unacknowledged sync failure(s). ${preview}` + + `${unacked.length} unacknowledged sync failure(s) [${codeBreakdown}]. ${preview}` + `${unacked.length > 3 ? `, and ${unacked.length - 3} more` : ''}. ` + `Fix the file(s) and re-run 'gbrain sync', or use 'gbrain sync --skip-failed' to acknowledge.`, }); } else if (all.length > 0) { - // Acknowledged-only: informational, not a warning. + // Acknowledged-only: show code breakdown for visibility. + const ackedSummary = summarizeFailuresByCode(all); + const ackedBreakdown = ackedSummary.map(s => `${s.code}=${s.count}`).join(', '); checks.push({ name: 'sync_failures', status: 'ok', - message: `${all.length} historical sync failure(s), all acknowledged.`, + message: `${all.length} historical sync failure(s), all acknowledged [${ackedBreakdown}].`, }); } } catch { diff --git a/src/commands/sync.ts b/src/commands/sync.ts index 0e5e40fbc..cc08b4a68 100644 --- a/src/commands/sync.ts +++ b/src/commands/sync.ts @@ -12,6 +12,7 @@ import { recordSyncFailures, unacknowledgedSyncFailures, acknowledgeSyncFailures, + summarizeFailuresByCode, } from '../core/sync.ts'; import { estimateTokens, CHUNKER_VERSION } from '../core/chunkers/code.ts'; import { EMBEDDING_MODEL, estimateEmbeddingCostUsd } from '../core/embedding.ts'; @@ -522,9 +523,14 @@ export async function performSync(engine: BrainEngine, opts: SyncOpts): Promise< // current set, --retry-failed re-parses before running the normal sync. if (failedFiles.length > 0) { recordSyncFailures(failedFiles, headCommit); + // Emit structured summary grouped by error code so the operator + // can see *why* files failed, not just how many. + const codeSummary = summarizeFailuresByCode(failedFiles); + const codeBreakdown = codeSummary.map(s => ` ${s.code}: ${s.count}`).join('\n'); if (!opts.skipFailed) { console.error( - `\nSync blocked: ${failedFiles.length} file(s) failed to parse. ` + + `\nSync blocked: ${failedFiles.length} file(s) failed to parse:\n` + + `${codeBreakdown}\n\n` + `Fix the YAML frontmatter in the files above and re-run, or use ` + `'gbrain sync --skip-failed' to acknowledge and move on.`, ); @@ -547,8 +553,12 @@ export async function performSync(engine: BrainEngine, opts: SyncOpts): Promise< } // --skip-failed: acknowledge the now-recorded set and proceed. const acked = acknowledgeSyncFailures(); - if (acked > 0) { - console.error(` Acknowledged ${acked} failure(s) and advancing past them.`); + if (acked.count > 0) { + const ackedBreakdown = acked.summary.map(s => ` ${s.code}: ${s.count}`).join('\n'); + console.error( + ` Acknowledged ${acked.count} failure(s) and advancing past them:\n` + + `${ackedBreakdown}`, + ); } } @@ -656,9 +666,12 @@ async function performFullSync( // the sync module owns the last_commit write. Respect the same gate. if (result.failures.length > 0) { recordSyncFailures(result.failures, headCommit); + const codeSummary = summarizeFailuresByCode(result.failures); + const codeBreakdown = codeSummary.map(s => ` ${s.code}: ${s.count}`).join('\n'); if (!opts.skipFailed) { console.error( - `\nFull sync blocked: ${result.failures.length} file(s) failed. ` + + `\nFull sync blocked: ${result.failures.length} file(s) failed:\n` + + `${codeBreakdown}\n\n` + `Fix the YAML in those files and re-run, or use '--skip-failed'.`, ); await engine.setConfig('sync.last_run', new Date().toISOString()); @@ -675,7 +688,13 @@ async function performFullSync( }; } const acked = acknowledgeSyncFailures(); - if (acked > 0) console.error(` Acknowledged ${acked} failure(s) and advancing past them.`); + if (acked.count > 0) { + const ackedBreakdown = acked.summary.map(s => ` ${s.code}: ${s.count}`).join('\n'); + console.error( + ` Acknowledged ${acked.count} failure(s) and advancing past them:\n` + + `${ackedBreakdown}`, + ); + } } // Persist sync state so next sync is incremental (C1 fix: was missing). diff --git a/src/core/sync.ts b/src/core/sync.ts index 3876b9d6b..45796ac8e 100644 --- a/src/core/sync.ts +++ b/src/core/sync.ts @@ -307,6 +307,8 @@ import { createHash as _createHash } from 'crypto'; export interface SyncFailure { path: string; error: string; + /** Structured error code extracted from the error message. */ + code?: string; commit: string; line?: number; ts: string; @@ -314,6 +316,40 @@ export interface SyncFailure { acknowledged_at?: string; } +/** + * Best-effort extraction of a structured error code from a sync failure + * message. Matches known ParseValidationCode patterns (SLUG_MISMATCH, + * YAML_PARSE, etc.) and common DB / timeout errors. Returns 'UNKNOWN' + * when no pattern matches. + */ +export function classifyErrorCode(errorMsg: string): string { + if (/slug.*does not match|SLUG_MISMATCH/i.test(errorMsg)) return 'SLUG_MISMATCH'; + if (/YAML parse|yaml.*parse|YAML_PARSE/i.test(errorMsg)) return 'YAML_PARSE'; + if (/MISSING_OPEN|missing.*open/i.test(errorMsg)) return 'MISSING_OPEN'; + if (/MISSING_CLOSE|missing.*close/i.test(errorMsg)) return 'MISSING_CLOSE'; + if (/NULL_BYTES|null.*byte/i.test(errorMsg)) return 'NULL_BYTES'; + if (/NESTED_QUOTES|nested.*quote/i.test(errorMsg)) return 'NESTED_QUOTES'; + if (/EMPTY_FRONTMATTER|empty.*frontmatter/i.test(errorMsg)) return 'EMPTY_FRONTMATTER'; + if (/duplicate.*key/i.test(errorMsg)) return 'YAML_DUPLICATE_KEY'; + if (/statement.*timeout/i.test(errorMsg)) return 'STATEMENT_TIMEOUT'; + if (/invalid.*utf/i.test(errorMsg)) return 'INVALID_UTF8'; + return 'UNKNOWN'; +} + +/** Group failures by error code and return a sorted summary. */ +export function summarizeFailuresByCode( + failures: Array<{ error: string; code?: string }>, +): Array<{ code: string; count: number }> { + const counts: Record = {}; + for (const f of failures) { + const code = f.code ?? classifyErrorCode(f.error); + counts[code] = (counts[code] ?? 0) + 1; + } + return Object.entries(counts) + .sort(([, a], [, b]) => b - a) + .map(([code, count]) => ({ code, count })); +} + function _failuresDir(): string { return _joinPath(_homedir(), '.gbrain'); } @@ -370,6 +406,7 @@ export function recordSyncFailures( const entry: SyncFailure = { path: f.path, error: f.error, + code: classifyErrorCode(f.error), commit, line: f.line, ts: now, @@ -380,28 +417,42 @@ export function recordSyncFailures( } } +export interface AcknowledgeResult { + count: number; + summary: Array<{ code: string; count: number }>; +} + /** * Mark all unacknowledged failures as acknowledged. Used by - * `gbrain sync --skip-failed`. Returns the number newly acknowledged. + * `gbrain sync --skip-failed`. Returns count and a structured summary + * grouped by error code so the operator can see *why* files were skipped. * * We do not delete — acknowledged entries stay as historical record so * doctor can still show them under a "previously skipped" bucket. */ -export function acknowledgeSyncFailures(): number { +export function acknowledgeSyncFailures(): AcknowledgeResult { const entries = loadSyncFailures(); - if (entries.length === 0) return 0; + if (entries.length === 0) return { count: 0, summary: [] }; const now = new Date().toISOString(); let changed = 0; + const newlyAcked: SyncFailure[] = []; const updated = entries.map(e => { if (e.acknowledged) return e; changed++; - return { ...e, acknowledged: true, acknowledged_at: now }; + // Backfill code for entries that predate the code field. + const code = e.code ?? classifyErrorCode(e.error); + const acked = { ...e, code, acknowledged: true, acknowledged_at: now }; + newlyAcked.push(acked); + return acked; }); - if (changed === 0) return 0; + if (changed === 0) return { count: 0, summary: [] }; _mkdirSync(_failuresDir(), { recursive: true }); const fd = require('fs').writeFileSync; fd(syncFailuresPath(), updated.map(e => JSON.stringify(e)).join('\n') + '\n'); - return changed; + return { + count: changed, + summary: summarizeFailuresByCode(newlyAcked), + }; } /** Return only unacknowledged failures. */ diff --git a/test/sync-failures.test.ts b/test/sync-failures.test.ts index 7fa9a85ef..03607977a 100644 --- a/test/sync-failures.test.ts +++ b/test/sync-failures.test.ts @@ -76,18 +76,19 @@ describe('Bug 9 — sync-failures JSONL helpers', () => { { path: 'b.md', error: 'err2' }, ], 'commit1'); - const n = acknowledgeSyncFailures(); - expect(n).toBe(2); + const result = acknowledgeSyncFailures(); + expect(result.count).toBe(2); + expect(result.summary.length).toBeGreaterThan(0); const after = loadSyncFailures(); expect(after.every(e => e.acknowledged === true)).toBe(true); expect(after.every(e => typeof e.acknowledged_at === 'string')).toBe(true); // Second ack: nothing new to mark. - expect(acknowledgeSyncFailures()).toBe(0); + expect(acknowledgeSyncFailures().count).toBe(0); // Adding a fresh failure then ack: only the new one flips. recordSyncFailures([{ path: 'c.md', error: 'err3' }], 'commit2'); - expect(acknowledgeSyncFailures()).toBe(1); + expect(acknowledgeSyncFailures().count).toBe(1); expect(loadSyncFailures().length).toBe(3); expect(loadSyncFailures().every(e => e.acknowledged === true)).toBe(true); }); @@ -158,3 +159,107 @@ describe('Bug 9 — sync.ts CLI flag wiring', () => { expect(source).toContain('recordSyncFailures'); }); }); + +describe('classifyErrorCode — error message to code mapping', () => { + test('classifies SLUG_MISMATCH from error message', async () => { + const { classifyErrorCode } = await import('../src/core/sync.ts'); + expect(classifyErrorCode( + 'Frontmatter slug "my-friend-mike" does not match path-derived slug "2008-03-20-my-friend-mike"' + )).toBe('SLUG_MISMATCH'); + }); + + test('classifies YAML_PARSE from error message', async () => { + const { classifyErrorCode } = await import('../src/core/sync.ts'); + expect(classifyErrorCode('YAML parse failed: unexpected colon in title')).toBe('YAML_PARSE'); + }); + + test('classifies YAML_DUPLICATE_KEY', async () => { + const { classifyErrorCode } = await import('../src/core/sync.ts'); + expect(classifyErrorCode('YAMLException: duplicated mapping key')).toBe('YAML_DUPLICATE_KEY'); + }); + + test('classifies STATEMENT_TIMEOUT', async () => { + const { classifyErrorCode } = await import('../src/core/sync.ts'); + expect(classifyErrorCode('canceling statement due to statement timeout')).toBe('STATEMENT_TIMEOUT'); + }); + + test('classifies NULL_BYTES', async () => { + const { classifyErrorCode } = await import('../src/core/sync.ts'); + expect(classifyErrorCode('invalid UTF-8: null byte at position 3770')).toBe('NULL_BYTES'); + }); + + test('classifies INVALID_UTF8', async () => { + const { classifyErrorCode } = await import('../src/core/sync.ts'); + expect(classifyErrorCode('invalid UTF-8 sequence at position 500')).toBe('INVALID_UTF8'); + }); + + test('returns UNKNOWN for unrecognized errors', async () => { + const { classifyErrorCode } = await import('../src/core/sync.ts'); + expect(classifyErrorCode('something completely different')).toBe('UNKNOWN'); + }); +}); + +describe('summarizeFailuresByCode — grouped summary', () => { + test('groups failures by classified code', async () => { + const { summarizeFailuresByCode } = await import('../src/core/sync.ts'); + const summary = summarizeFailuresByCode([ + { error: 'Frontmatter slug "a" does not match path-derived slug "b"' }, + { error: 'Frontmatter slug "c" does not match path-derived slug "d"' }, + { error: 'YAML parse failed: bad colon' }, + { error: 'something unknown' }, + ]); + expect(summary).toEqual([ + { code: 'SLUG_MISMATCH', count: 2 }, + { code: 'YAML_PARSE', count: 1 }, + { code: 'UNKNOWN', count: 1 }, + ]); + }); + + test('respects pre-classified code field', async () => { + const { summarizeFailuresByCode } = await import('../src/core/sync.ts'); + const summary = summarizeFailuresByCode([ + { error: 'anything', code: 'SLUG_MISMATCH' }, + { error: 'anything', code: 'SLUG_MISMATCH' }, + { error: 'anything', code: 'YAML_PARSE' }, + ]); + expect(summary).toEqual([ + { code: 'SLUG_MISMATCH', count: 2 }, + { code: 'YAML_PARSE', count: 1 }, + ]); + }); + + test('returns empty array for no failures', async () => { + const { summarizeFailuresByCode } = await import('../src/core/sync.ts'); + expect(summarizeFailuresByCode([])).toEqual([]); + }); +}); + +describe('acknowledgeSyncFailures — structured return', () => { + test('returns count and code summary', async () => { + const { recordSyncFailures, acknowledgeSyncFailures } = await import('../src/core/sync.ts'); + recordSyncFailures([ + { path: 'a.md', error: 'Frontmatter slug "x" does not match path-derived slug "y"' }, + { path: 'b.md', error: 'Frontmatter slug "p" does not match path-derived slug "q"' }, + { path: 'c.md', error: 'YAML parse failed: bad' }, + ], 'commit1'); + + const result = acknowledgeSyncFailures(); + expect(result.count).toBe(3); + expect(result.summary).toEqual([ + { code: 'SLUG_MISMATCH', count: 2 }, + { code: 'YAML_PARSE', count: 1 }, + ]); + }); +}); + +describe('recordSyncFailures — code field', () => { + test('records classified code alongside error message', async () => { + const { recordSyncFailures, loadSyncFailures } = await import('../src/core/sync.ts'); + recordSyncFailures([ + { path: 'a.md', error: 'Frontmatter slug "x" does not match path-derived slug "y"' }, + ], 'commit1'); + + const entries = loadSyncFailures(); + expect(entries[0].code).toBe('SLUG_MISMATCH'); + }); +});