v0.22.12 feat: structured error code summary for sync --skip-failed (closes #500) by garrytan · Pull Request #518 · garrytan/gbrain

garrytan · 2026-04-29T15:40:12Z

Summary

Closes #500. Operators running gbrain sync --skip-failed no longer see a single bare count when thousands of files fail. They see the breakdown by error code at the moment of the skip, so a systemic-class failure (SLUG_MISMATCH=2685) is visible immediately instead of three weeks later.

Foundation: PR #501 by @WinterMute (cherry-picked as c356ea4) — classifier, doctor breakdown, AcknowledgeResult shape, 12 unit tests.

This branch adds the eng-review hardening on top:

Classifier coverage for FILE_TOO_LARGE + SYMLINK_NOT_ALLOWED — the four real production sites in src/core/import-file.ts:199, 347, 352, 401 previously bucketed to UNKNOWN. Exactly the silent-systemic pattern that motivated sync --skip-failed should emit structured summary by error code and persist skip manifest #500. Two new regex lines.
Three regex breadths — MISSING_OPEN, MISSING_CLOSE, EMPTY_FRONTMATTER only matched the literal code-name prefix. Broadened to match the actual validator messages emitted by src/core/markdown.ts:159-244.
Six new unit tests pinning the regex set against literal production strings. Anyone reworking a validator message has to update both sides.
One end-to-end test in test/e2e/sync.test.ts exercising the full failure loop: broken file → sync blocks with grouped breakdown → --skip-failed advances bookmark with grouped acknowledgement → second broken file → second cycle. Hermetic on a developer machine (saves+restores the user's real ~/.gbrain/sync-failures.jsonl).

Test Coverage

Unit: test/sync-failures.test.ts 22 → 28 (+6 new), all green
E2E: test/e2e/sync.test.ts 13 → 14 (+1 new failure-loop test), all green against real Postgres + pgvector
Full E2E suite: 235 pass, 1 pre-existing failure (multi-source cascade delete) verified unchanged from master baseline

COVERAGE: 12/12 classifier codes have unit tests (was 7/12) · 100%
QUALITY: ★★★ tests pin literal production message strings, not aspirational regex behavior
GAPS: 0 (the 4 markdown validator codes don't naturally surface in production today
       since sync uses parseMarkdown without {validate:true} — that plumbing is the
       v0.22.13+ follow-up. Tests pin the contract for when it does land.)

Pre-Landing Review

3 ship-blockers caught and resolved before commit:

Classifier UNKNOWN gaps — file-size and symlink rejections bucketed to UNKNOWN. Fixed with 2 regex lines in src/core/sync.ts.
4 of 12 codes untested (MISSING_OPEN, MISSING_CLOSE, NESTED_QUOTES, EMPTY_FRONTMATTER). Fixed with 6 unit tests pinning literal production strings.
Zero E2E coverage for failure loop — entire new code path (record → classify → block → skip → doctor render) had only mocked-JSONL coverage. Fixed with 1 comprehensive E2E test.

Plan file: ~/.claude/plans/hold-scope-just-want-bubbly-penguin.md documents the full eng review and decisions.

Plan Completion

All scope items from the plan landed:

Add FILE_TOO_LARGE regex (covers import-file.ts:199, 352, 401)
Add SYMLINK_NOT_ALLOWED regex (covers import-file.ts:347)
Broaden 3 existing regexes to match real validator messages
6 new unit tests
1 E2E failure-loop test
Version bump (0.22.6.1 → 0.22.12)
CHANGELOG entry with release-summary, "to take advantage" block, credit to @WinterMute
CLAUDE.md annotations on src/core/sync.ts, src/commands/doctor.ts, test/sync-failures.test.ts, test/e2e/sync.test.ts

TODOS

Filed for v0.22.13:

Doctor sync_skipped warn elevation for acked-only entries (issue sync --skip-failed should emit structured summary by error code and persist skip manifest #500 ask 3)
--retry-skipped flag with safe bookmark rewind (issue sync --skip-failed should emit structured summary by error code and persist skip manifest #500 ask 4)
Plumb structured code through ImportResult.error from import-file.ts / markdown.ts (replaces heuristic regex with first-class field)

Test plan

bun test test/sync-failures.test.ts — 28 pass, 0 fail
DATABASE_URL=... bun test test/e2e/sync.test.ts — 14 pass, 0 fail
DATABASE_URL=... bun run test:e2e — 235 pass, 1 pre-existing fail (unchanged from master baseline)
Manual smoke: classifier returns expected codes for all 12 categories

🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.7 noreply@anthropic.com

^{Need help on this PR? Tag @codesmith with what you need.}

Let Codesmith autofix CI failures and bot reviews

When sync encounters per-file failures, the blocked/skip-failed messages now include a breakdown by error code (SLUG_MISMATCH, YAML_PARSE, etc.) instead of just a raw count. This makes it immediately obvious *why* files failed without requiring manual investigation. Changes: - Add classifyErrorCode() — maps error messages to ParseValidationCode - Add summarizeFailuresByCode() — groups failures into sorted code summary - SyncFailure now carries a 'code' field (backfilled on acknowledge) - acknowledgeSyncFailures() returns AcknowledgeResult {count, summary} - sync blocked + skip-failed messages show code breakdown - doctor sync_failures check shows code breakdown for both unacked and historical - 12 new tests for classifyErrorCode, summarizeFailuresByCode, and structured returns Before: Sync blocked: 2688 file(s) failed to parse. After: Sync blocked: 2688 file(s) failed to parse: SLUG_MISMATCH: 2685 YAML_DUPLICATE_KEY: 3 Closes #500

…it tests Eng review of PR #501 found two ship-blocking gaps in the classifier: 1. Four real production error sites in src/core/import-file.ts emit strings that bucketed to UNKNOWN — exactly the silent-systemic-failure pattern that motivated #500 in the first place. Add two regex lines: FILE_TOO_LARGE — covers import-file.ts:199, 352, 401 SYMLINK_NOT_ALLOWED — covers import-file.ts:347 2. Three existing classifier regexes (MISSING_OPEN, MISSING_CLOSE, EMPTY_FRONTMATTER) only matched the literal code-name prefix. The actual message strings emitted by markdown.ts:159-244 (e.g. "Frontmatter must start with --- on the first non-empty line") wouldn't match. Broaden each to match production message text. NESTED_QUOTES already worked. Add 6 unit tests pinning the contract between markdown.ts/import-file.ts strings and the classifier regex set. If anyone reworks a validator message, both sides have to move together — the test fails loudly otherwise. Test count: 22 → 28 in test/sync-failures.test.ts, all green.

…ip-blocker) The full code path (record → classify → block → skip → doctor render → second cycle) had only mocked-JSONL unit coverage. For a hotfix that changes user-visible CLI output and the doctor surface, that's thin. One comprehensive E2E test covers the loop: 1. First sync of clean repo — succeeds, bookmark advances 2. Add file with bad slug — sync returns 'blocked_by_failures', bookmark stays put, JSONL has 1 unacked entry coded SLUG_MISMATCH 3. --skip-failed — bookmark advances past the bad commit, entry transitions to acknowledged, AcknowledgeResult.summary aggregates 4. Second broken file (different path, same code) — sync blocks again, 1 acked + 1 unacked, dedup honors path identity 5. --skip-failed again — both acked, summary correctly counts 2 Hermetic on a developer machine: saves ~/.gbrain/sync-failures.jsonl before the test, restores it after. Doctor rendering verified by calling the same primitives doctor.ts uses (loadSyncFailures + summarizeFailuresByCode) rather than runDoctor() — runDoctor is a CLI entrypoint with stdout/exit side effects that truncate the test mid-flow. E2E count: 13 → 14 in test/e2e/sync.test.ts. All 14 pass under real Postgres + pgvector (gbrain-test-pg/pgvector:pg16).

@WinterMute

Closes issue #500. PR #501 by @WinterMute is the foundation (cherry-picked as c356ea4 — classifier, doctor breakdown, AcknowledgeResult shape, 12 unit tests). This release adds: - Classifier coverage for FILE_TOO_LARGE + SYMLINK_NOT_ALLOWED (the four size/symlink rejection sites in import-file.ts that bucketed to UNKNOWN). - Three regex breadths (MISSING_OPEN, MISSING_CLOSE, EMPTY_FRONTMATTER) matching actual markdown.ts validator messages, not just the literal code-name prefix. - 6 new unit tests pinning literal production strings. - 1 comprehensive E2E test exercising the full failure loop. Total v0.22.12 diff: ~340 lines on top of PR #501. Backward-compatible — pre-v0.22.12 JSONL entries get classified at acknowledge time.

…v0-22-12 # Conflicts: # CHANGELOG.md # CLAUDE.md # VERSION # package.json

CI regen-drift guard caught that llms-full.txt was stale after the v0.22.12 CLAUDE.md annotation updates (sync.ts, doctor.ts, sync-failures.test.ts, e2e/sync.test.ts entries). Per CLAUDE.md "Auto-derived" rule: run `bun run build:llms` after any release ship that touches Key Files annotations. The bundle reflects current docs state. llms.txt unchanged (curated index doesn't index those entries). llms-full.txt: 308192 bytes. test/build-llms.test.ts now passes 7/7 (was 6/7 in CI).

WinterMute · 2026-04-29T16:50:04Z

Any chance you could not tag me with this stuff?

…v0-22-12 # Conflicts: # CHANGELOG.md # VERSION # package.json # src/commands/sync.ts # src/core/sync.ts # test/sync-failures.test.ts

…v0-22-12 # Conflicts: # CHANGELOG.md # VERSION # package.json

Wintermute and others added 5 commits April 28, 2026 08:47

Merge remote-tracking branch 'origin/master' into garrytan/issue-500-…

34077a6

…v0-22-12 # Conflicts: # CHANGELOG.md # CLAUDE.md # VERSION # package.json

garrytan mentioned this pull request Apr 29, 2026

v0.22.9 feat: structured error code summary for sync --skip-failed #501

Merged

4 tasks

garrytan added 3 commits April 29, 2026 11:20

Merge remote-tracking branch 'origin/master' into garrytan/issue-500-…

b7fcf0d

…v0-22-12 # Conflicts: # CHANGELOG.md # VERSION # package.json # src/commands/sync.ts # src/core/sync.ts # test/sync-failures.test.ts

Merge remote-tracking branch 'origin/master' into garrytan/issue-500-…

0d48224

…v0-22-12 # Conflicts: # CHANGELOG.md # VERSION # package.json

Merge remote-tracking branch 'origin/master' into garrytan/issue-500-…

6c9abf2

…v0-22-12 # Conflicts: # CHANGELOG.md # VERSION # package.json

garrytan merged commit 1e73e93 into master Apr 30, 2026
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

v0.22.12 feat: structured error code summary for sync --skip-failed (closes #500)#518

v0.22.12 feat: structured error code summary for sync --skip-failed (closes #500)#518
garrytan merged 9 commits into
masterfrom
garrytan/issue-500-v0-22-12

garrytan commented Apr 29, 2026 •

edited by blacksmith-sh Bot

Loading

Uh oh!

WinterMute commented Apr 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

garrytan commented Apr 29, 2026 • edited by blacksmith-sh Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Coverage

Pre-Landing Review

Plan Completion

TODOS

Test plan

Uh oh!

WinterMute commented Apr 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

garrytan commented Apr 29, 2026 •

edited by blacksmith-sh Bot

Loading