From 75c2e259d07565c5060a091ce57bedfe85848523 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 17 Jun 2026 13:45:05 -0400 Subject: [PATCH] refactor(#1387): migrate adr-parser onto markdown-sectionizer seam (epic #1372 T2) (#1388) * refactor(#1387): migrate adr-parser onto markdown-sectionizer seam (T2) Replace hand-rolled parseSections (split/heading-regex walk/body accumulation) with collectSections(content, () => true) from the canonical seam. Replace hand-rolled splitEntries bullet-strip regex with iterateBullets from the seam, preserving the plain-text-line fallback for byte-identical output. Removes the last inline heading/bullet scanning from adr-parser.cts; normalizeAdrHeader and all ADR-specific classification logic are unchanged. 218/218 tests pass before and after. Co-Authored-By: Claude Sonnet 4.6 * refactor(#1387): keep splitEntries flat (iterateBullets changed its contract); seam adoption stays in parseSections Co-Authored-By: Claude Sonnet 4.6 * refactor(#1387): drop dead preamble reconstruction; add targeted adr-parser mutation tests parseSections' preamble reconstruction block (heading: null entry) was dead code: both consumers (parseAdrMarkdown and parseStatusFromSections) skip heading: null sections immediately on entry. Confirmed via analytical trace and zero-diff corpus head-to-head across all 44 docs/adr/*.md files. Adds 11 targeted behavioral tests to kill cheap surviving mutants: - pushUnique intra-values dedup (kills seen.add removal mutant) - body split/join round-trip with multi-line prose and entries - parseStatusFromSections [0] indexing (only first line determines status) - classifyHeader equality vs prefix-match boundary (exact match, prefix match, synonym+letter non-match) - goal section prose vs entries distinction (bullet markers preserved in context) - normalizeAdrHeader non-word char removal (parens and slash behavior) Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- src/adr-parser.cts | 38 ++++++---- tests/adr-parser.unit.test.cjs | 128 +++++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 14 deletions(-) diff --git a/src/adr-parser.cts b/src/adr-parser.cts index c6b91019c..135c80c07 100644 --- a/src/adr-parser.cts +++ b/src/adr-parser.cts @@ -10,6 +10,7 @@ import fs from 'node:fs'; import path from 'node:path'; import { requireSafePath } from './security.cjs'; +import { collectSections } from './markdown-sectionizer.cjs'; const STATUS_REJECT_SET = new Set(['superseded', 'rejected', 'deprecated']); @@ -231,23 +232,32 @@ interface MarkdownSection { body: string[]; } +/** + * Thin adapter: wraps the seam's `collectSections` to produce the same + * `{ heading: string | null, body: string[] }` shape the rest of adr-parser + * consumes. ADR-1372 T2 migration. + */ function parseSections(markdown: unknown): MarkdownSection[] { - const lines = (typeof markdown === 'string' ? markdown : '').split(/\r?\n/); - const sections: MarkdownSection[] = []; - let current: MarkdownSection = { heading: null, body: [] }; + const content = typeof markdown === 'string' ? markdown : ''; - for (const line of lines) { - const m = line.match(/^#{1,6}\s+(.*)$/); - if (m) { - if (current.heading || current.body.length) sections.push(current); - current = { heading: m[1].trim(), body: [] }; - } else { - current.body.push(line); - } - } + // collectSections(content, () => true) collects every heading as a stop + // boundary — mirrors the old line-by-line heading walk exactly. + const sections = collectSections(content, () => true); - if (current.heading || current.body.length) sections.push(current); - return sections; + // Map seam Section → MarkdownSection. The seam's HeadingToken.text is the + // heading text after trimming (same as the old m[1].trim() capture). + // The body is a trimEnd()-ed joined string; split it back to lines to match + // the old string[] shape consumed by parseStatusFromSections / parseAdrMarkdown. + // + // Note: the old parseSections emitted a leading { heading: null, body: [...] } + // entry for preamble text before the first heading. Both consumers skip it + // immediately (parseAdrMarkdown: `if (!heading) continue`; parseStatusFromSections: + // `classifyHeader(normalizeAdrHeader(null))` → null ≠ 'status' → continue), so + // the preamble entry was dead code and is not reconstructed here. + return sections.map((sec) => ({ + heading: sec.heading.text, + body: sec.body === '' ? [] : sec.body.split('\n'), + })); } function parseStatusFromSections(sections: MarkdownSection[]): string { diff --git a/tests/adr-parser.unit.test.cjs b/tests/adr-parser.unit.test.cjs index 7612a0762..03e6cd67f 100644 --- a/tests/adr-parser.unit.test.cjs +++ b/tests/adr-parser.unit.test.cjs @@ -1322,6 +1322,22 @@ describe('splitEntries (via parseAdrMarkdown decisions)', () => { const out = parseAdrMarkdown('## Decision\n \n- Real entry.\n '); assert.deepEqual(out.decisions, ['Real entry.']); }); + + // Regression guard for ADR-1372 T2: iterateBullets folded indented non-bullet + // lines into the preceding bullet — the flat splitEntries must keep them. + test('indented non-bullet line (4-space) kept verbatim as its own entry', () => { + const md = '## Decision\n- Bullet entry\n indented non-bullet line\n- Another bullet'; + const out = parseAdrMarkdown(md); + assert.deepEqual(out.decisions, ['Bullet entry', 'indented non-bullet line', 'Another bullet']); + }); + + // Regression guard for ADR-1372 T2: iterateBullets stripped numbered markers + // ("1. Foo" → "Foo") — the flat splitEntries only strips [-*+], not numbers. + test('numbered list item kept verbatim (not stripped to bare text)', () => { + const md = '## Decision\n1. First\n2. Second'; + const out = parseAdrMarkdown(md); + assert.deepEqual(out.decisions, ['1. First', '2. Second']); + }); }); // ───────────────────────────────────────────────────────────────────────────── @@ -1401,3 +1417,115 @@ describe('parseAdrMarkdown: full document integration', () => { assert.deepEqual(out.unmapped_headers, ['ADR-0001: Switch to PostgreSQL']); }); }); + +// ───────────────────────────────────────────────────────────────────────────── +// Targeted mutation-killing tests (T2 adapter seam) +// Each test is annotated with the mutant it kills. +// ───────────────────────────────────────────────────────────────────────────── +describe('targeted: pushUnique intra-values deduplication', () => { + // Kills: `seen.add(value)` removal mutant — without it, values-internal dups pass through. + test('duplicate entries within the same section body are deduplicated', () => { + const md = '## Decision\n- Same entry.\n- Same entry.\n- Different entry.'; + const out = parseAdrMarkdown(md); + assert.deepEqual(out.decisions, ['Same entry.', 'Different entry.']); + }); +}); + +describe('targeted: parseSections body-split round-trip', () => { + // Kills: body split/join mutations — each body line must be its own array element. + // The adapter does sec.body.split('\n'); parseAdrMarkdown does section.body.join('\n'). + // A mutant replacing '\n' with ' ' in either call would break this. + test('multi-line goal body has each line preserved with internal newlines in prose', () => { + const md = '## Context\nLine one.\nLine two.\nLine three.'; + const out = parseAdrMarkdown(md); + // prose = section.body.join('\n').trim() — must include all three lines separated by \n + assert.ok(out.context.includes('Line one.'), `context missing line one: ${out.context}`); + assert.ok(out.context.includes('Line two.'), `context missing line two: ${out.context}`); + assert.ok(out.context.includes('Line three.'), `context missing line three: ${out.context}`); + assert.ok(out.context.includes('\n'), 'context must retain internal newlines'); + }); + + test('multi-line decision body produces one entry per non-blank line', () => { + // entries = splitEntries(section.body.join('\n')) — join must be '\n' not ' ' + const md = '## Decision\n- Alpha.\n- Beta.\n- Gamma.'; + const out = parseAdrMarkdown(md); + assert.deepEqual(out.decisions, ['Alpha.', 'Beta.', 'Gamma.']); + }); +}); + +describe('targeted: parseStatusFromSections uses first entry only', () => { + // Kills: mutants that remove [0] indexing or change `splitEntries(...)[0]` to return all. + test('only the first non-blank line of the status body determines status', () => { + // Second line "rejected" must NOT influence the result. + const md = '# ADR\n\n## Status\naccepted\nrejected\n'; + const out = parseAdrMarkdown(md); + assert.equal(out.status, 'accepted'); + }); +}); + +describe('targeted: classifyHeader exact-match vs prefix-match boundary', () => { + // Kills: mutants that remove the trailing space from startsWith check, or remove + // the equality check. + + // Case 1: exact match — heading IS the synonym (no trailing content) + test('heading exactly equal to synonym matches (equality branch)', () => { + const out = parseAdrMarkdown('## Status\naccepted\n'); + assert.equal(out.status, 'accepted'); + }); + + // Case 2: prefix match — heading starts with synonym + space + more text + test('heading starting with synonym + space matches (prefix branch)', () => { + // "status of the adr" → starts with "status " → classified as status + const out = parseAdrMarkdown('## Status of the ADR\naccepted\n'); + assert.equal(out.status, 'accepted'); + }); + + // Case 3: heading IS synonym but no trailing space should NOT match via startsWith + // (it matches via equality instead) — this verifies the equality check fires + test('heading that exactly equals a synonym is classified without trailing space', () => { + // "context" equals the synonym exactly — must be classified as goal + const out = parseAdrMarkdown('## Context\nExact match context.'); + assert.equal(out.context.trim(), 'Exact match context.'); + }); + + // Case 4: heading with wrong suffix (synonym+letter, no space) must NOT match prefix + test('heading that is synonym + letter (no space) does NOT match prefix', () => { + // "statuses" → normalizes to "statuses", not "status " prefix — unclassified + const out = parseAdrMarkdown('## Statuses\naccepted\n'); + assert.ok(out.unmapped_headers.includes('Statuses')); + // status should fall back to 'accepted' default (no status section found) + assert.equal(out.status, 'accepted'); + }); +}); + +describe('targeted: goal section prose vs entries distinction', () => { + // The goal/context case uses `prose` (joined + trimmed multi-line text), not `entries` + // (bullet-stripped list). Killing the `prose` variable or swapping it for `entries` + // would strip bullet markers from context text. + test('goal section body with bullet markers is preserved verbatim in context (prose, not entries)', () => { + // If parser used entries instead of prose, "- with a dash" would become "with a dash". + const md = '## Context\nThis is context.\n- with a dash item.\nMore prose.'; + const out = parseAdrMarkdown(md); + assert.ok(out.context.includes('- with a dash item.'), + `context should preserve bullet markers in prose: ${out.context}`); + }); +}); + +describe('targeted: normalizeAdrHeader non-word char removal', () => { + // Kills: regex mutation in the [^\w\s] replacement — e.g. inverting the class + // or changing the replacement target. + test('parentheses in heading are stripped by non-word removal', () => { + // "Context (v2)" normalizes to "context v2" — still matches "context" via prefix "context " + const out = parseAdrMarkdown('## Context (v2)\nSome context here.'); + assert.equal(out.context.trim(), 'Some context here.'); + }); + + test('non-word chars adjacent to word chars are stripped without inserting a space', () => { + // "Context/Background" → [^\w\s] removes '/' → "contextbackground" (no space) + // So it does NOT classify as goal (exact "contextbackground" ≠ any synonym). + const out = parseAdrMarkdown('## Context/Background\nSlash context.'); + // Does not classify as goal — goes to unmapped_headers + assert.ok(out.unmapped_headers.includes('Context/Background')); + assert.equal(out.context, ''); + }); +});