fix(diagnostics): diagnostic style in one leaf module + byte-parity Rust truncate (5mi)#99
Conversation
…ust truncate (5mi) Sheriff finding #4 [LAW:one-source-of-truth]: the red-error FG/BG/RESET literals were hand-duplicated across error-glyph.ts, server.ts's composeWithDiagnostics, and the Rust mirror, with a false "avoid daemon->client coupling" comment justifying the copies. - New leaf src/render/diagnostic-style.ts owns the diagnostic visual identity (error trio + warning pair + reset); error-glyph.ts and server.ts import it — the same daemon->render direction already blessed for diagnostic-text.ts. The false-dichotomy comments are gone. - check-protocol's three style rows repoint to the leaf in this same commit (missing anchor fails loudly); PREFIX/MAX_MESSAGE_LEN stay anchored in error-glyph.ts (glyph shape, not duplicated in TS). - Folded divergence: Rust truncate now collapses whitespace runs and trims like TS sanitizeAndTruncate, making the byte-identical mirror claim true for multi-control inputs ("\r\n" -> one space in both). The Rust test that pinned the divergence is corrected [LAW:behavior-not-structure], and both suites now pin the same collapse/trim fixtures (incl. the U+FEFF edge JS \s implies). Verified: typecheck, lint, check:protocol (12/12), jest glyph/diagnostic /render-cache suites (31), cargo test (19) all green; residue grep shows no raw style literals outside the leaf and the enforced Rust mirror.
There was a problem hiding this comment.
Z.ai Coding Agent Review
Clean refactoring with no required changes. The PR extracts diagnostic style constants (ANSI escape codes for error/warning foreground and background) from their duplicated homes in error-glyph.ts and server.ts into a single leaf module diagnostic-style.ts, and brings the Rust truncate() to byte-identical parity with the TS sanitizeAndTruncate() by adding collapse-whitespace-runs and trim (the TS side already did this via replace(/\s+/g, " ").trim()). The check-protocol.mjs script is updated to verify the constants against the new canonical source. All three LAW claims in the diff — [LAW:one-source-of-truth] (style in one leaf), [LAW:one-way-deps] (leaf imports nothing, both daemon and client depend on it), [LAW:one-type-per-behavior] (Rust mirrors TS behavior, pinned by paired fixtures) — are accurate and well-documented. No violations introduced.
✅ Approved
Summary
Remediation for sheriff finding #4 (
brandon-diagnostics-5mi, epic 58c): the diagnostic ANSI style constants were hand-duplicated acrosssrc/render/error-glyph.ts,src/daemon/server.ts(composeWithDiagnostics), andrust-client/src/error_glyph.rs, justified by a false "avoid daemon→client coupling" comment.[LAW:one-source-of-truth]src/render/diagnostic-style.tsowns the diagnostic visual identity (error red trio, warning amber pair,ANSI_RESET). Botherror-glyph.tsandserver.tsimport it — the same daemon→render direction already blessed fordiagnostic-text.ts. The false-dichotomy comments are deleted in both runtimes.glyph FG/BG/RESET) now anchor on the leaf's exported names;PREFIX/MAX_MESSAGE_LENstay anchored inerror-glyph.ts(glyph shape, never duplicated in TS). 12/12 mirrored constants verified.truncatenow collapses whitespace runs + trims exactly like TSsanitizeAndTruncate, so the byte-identical mirror claim is true for multi-control inputs ("\r\n"→ one space in both runtimes). The Rust test that pinned the divergence is corrected ([LAW:behavior-not-structure]), and a new Rust test pins the same collapse/trim fixtures astest/diagnostic-text.test.ts— including the U+FEFF edge implied by JS\s.Verification
pnpm typecheck✓,pnpm lint✓,pnpm check:protocol✓ (12 mirrored constants)pnpm test -- test/error-glyph.test.ts test/diagnostic-text.test.ts test/daemon-render-cache.test.ts→ 31 passedcargo test→ 19 passed (incl. newtruncate_collapses_whitespace_runs_and_trims)200;40;40/220;160;40literals outside the leaf and the checker-enforced Rust mirror; the "avoid daemon" coupling comment no longer exists anywhere.