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 <noreply@anthropic.com>

* refactor(#1387): keep splitEntries flat (iterateBullets changed its contract); seam adoption stays in parseSections

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-06-17 13:45:05 -04:00
committed by GitHub
parent 917d903330
commit 75c2e259d0
2 changed files with 152 additions and 14 deletions

View File

@@ -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 {

View File

@@ -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, '');
});
});