* fix: make punctuated CANONICAL_HEADERS synonyms reachable (audit M7)
classifyHeader received an already-normalized header (via normalizeAdrHeader,
which collapses [\s:._-]+ to a space and strips [^\w\s]) but compared it against
the RAW synonym strings. So any synonym carrying a hyphen/apostrophe ('trade-offs',
'non-goals', 'anti-goals', 'follow-up', 'cross-cuts', 'post-grilling', "how we'll
know", "won't do/have") could never match — its ADR section silently went unmapped.
Nine synonyms across six buckets were dead; the repo had characterization tests
pinning that quirk ('unreachable synonym').
Root-cause fix (per ADR-1372's 'compound, don't accrete' guidance): normalize BOTH
sides via a module-load-precomputed index, instead of pre-baking 9 normalized
literals into the data table. Closes the abstraction asymmetry once for all current
and future synonyms; the table stays human-readable. Also de-dupes 'trade-offs' from
considered_options so it no longer shadows risks once both normalize to 'trade offs'.
Insertion order preserved → first-match-wins + exact-then-prefix precedence byte-
identical for already-normalized synonyms; only the 9 dead synonyms gain matching.
Updates the 7 characterization tests to the corrected behavior and adds a
reachability+no-collision invariant test guarding the whole class against regression.
Note: the audit's M7 premise (trade-offs *misclassified into considered_options*)
was factually wrong — it was unmapped, and tested as such. adr-parser is CLI-only
(ADR-1372 T2), so the behavior change cannot reach in-process gates.
Claude-Session: https://claude.ai/code/session_01R88n7Q54bAaVHFkDbbH1yz
* chore(changeset): Fixed fragment for #1536 (adr-parser punctuated synonyms)
Claude-Session: https://claude.ai/code/session_01R88n7Q54bAaVHFkDbbH1yz
---------
Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
This commit is contained in:
5
.changeset/daring-lemurs-rally.md
Normal file
5
.changeset/daring-lemurs-rally.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 1536
|
||||
---
|
||||
adr-parser now classifies 9 previously-dropped punctuated ADR headers (Trade-offs, Non-Goals, Won't Do, Follow-up, How We'll Know, etc.) into their intended buckets instead of leaving them unmapped.
|
||||
@@ -72,7 +72,6 @@ const CANONICAL_HEADERS: Record<CanonicalHeader, string[]> = {
|
||||
'candidates',
|
||||
'approaches considered',
|
||||
'variants',
|
||||
'trade-offs',
|
||||
'pros and cons of the options',
|
||||
'discussion',
|
||||
],
|
||||
@@ -208,13 +207,30 @@ function normalizeAdrHeader(raw: unknown): string {
|
||||
.trim();
|
||||
}
|
||||
|
||||
function classifyHeader(normalizedHeader: string): CanonicalHeader | null {
|
||||
// Normalized synonym index (audit M7). classifyHeader receives an ALREADY-normalized
|
||||
// header (via normalizeAdrHeader), but historically compared it against the RAW synonym
|
||||
// strings. Because normalizeAdrHeader collapses [\s:._-]+ to a space and strips [^\w\s],
|
||||
// any synonym carrying a hyphen/apostrophe/etc. ('trade-offs', "won't do", 'post-grilling')
|
||||
// could never match a normalized header — it was silently dead, and its ADR section went
|
||||
// unmapped. Normalizing BOTH sides closes that abstraction asymmetry once, so every synonym
|
||||
// (current and future) is reachable regardless of punctuation. Precomputed at module load to
|
||||
// avoid re-normalizing the whole table per call; insertion order is preserved so first-match-
|
||||
// wins and the exact-then-prefix precedence stay identical to the prior raw-compare loop.
|
||||
const _NORMALIZED_SYNONYM_INDEX: Array<[string, CanonicalHeader]> = (() => {
|
||||
const index: Array<[string, CanonicalHeader]> = [];
|
||||
for (const [canonical, synonyms] of Object.entries(CANONICAL_HEADERS) as Array<[CanonicalHeader, string[]]>) {
|
||||
for (const synonym of synonyms) {
|
||||
if (normalizedHeader === synonym) return canonical;
|
||||
if (normalizedHeader.startsWith(`${synonym} `)) return canonical;
|
||||
index.push([normalizeAdrHeader(synonym), canonical]);
|
||||
}
|
||||
}
|
||||
return index;
|
||||
})();
|
||||
|
||||
function classifyHeader(normalizedHeader: string): CanonicalHeader | null {
|
||||
for (const [synonym, canonical] of _NORMALIZED_SYNONYM_INDEX) {
|
||||
if (normalizedHeader === synonym) return canonical;
|
||||
if (normalizedHeader.startsWith(`${synonym} `)) return canonical;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
|
||||
@@ -620,10 +620,10 @@ describe('parseAdrMarkdown: risks section', () => {
|
||||
assert.deepEqual(out.consequences_positive, []);
|
||||
});
|
||||
|
||||
test('"Trade-offs" heading normalized to "trade offs" does NOT match synonym "trade-offs" (unreachable synonym)', () => {
|
||||
test('"Trade-offs" maps to consequences_negative (M7: both sides normalized, synonym now reachable)', () => {
|
||||
const out = parseAdrMarkdown('## Trade-offs\n- Increased latency.');
|
||||
assert.deepEqual(out.consequences_negative, []);
|
||||
assert.ok(out.unmapped_headers.includes('Trade-offs'));
|
||||
assert.deepEqual(out.consequences_negative, ['Increased latency.']);
|
||||
assert.ok(!out.unmapped_headers.includes('Trade-offs'));
|
||||
});
|
||||
|
||||
test('"Drawbacks" maps to consequences_negative', () => {
|
||||
@@ -712,12 +712,12 @@ describe('parseAdrMarkdown: success_criteria section', () => {
|
||||
assert.deepEqual(out.consequences_positive, ['Better DX.']);
|
||||
});
|
||||
|
||||
test('"How We\'ll Know" normalized to "how well know" does NOT match synonym "how we\'ll know" (unreachable synonym)', () => {
|
||||
// The apostrophe in "we'll" is stripped by normalizeAdrHeader, yielding "how well know".
|
||||
// The synonym "how we'll know" is stored with apostrophe — can't match.
|
||||
test('"How We\'ll Know" maps to consequences_positive (M7: synonym normalized on both sides, now reachable)', () => {
|
||||
// The apostrophe in "we'll" is stripped by normalizeAdrHeader on BOTH the header and the
|
||||
// synonym, so both yield "how well know" and now match (success_criteria → consequences_positive).
|
||||
const out = parseAdrMarkdown("## How We'll Know\n- Sales increase.");
|
||||
assert.deepEqual(out.consequences_positive, []);
|
||||
assert.ok(out.unmapped_headers.includes("How We'll Know"));
|
||||
assert.deepEqual(out.consequences_positive, ['Sales increase.']);
|
||||
assert.ok(!out.unmapped_headers.includes("How We'll Know"));
|
||||
});
|
||||
|
||||
test('"Compliance" maps to consequences_positive', () => {
|
||||
@@ -967,12 +967,10 @@ describe('parseAdrMarkdown: key_files section', () => {
|
||||
// parseAdrMarkdown — out_of_scope section
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
describe('parseAdrMarkdown: out_of_scope section', () => {
|
||||
test('"Non-goals" heading normalized to "non goals" does NOT match synonym "non-goals" (unreachable synonym)', () => {
|
||||
// "Non-goals" normalizes to "non goals"; CANONICAL_HEADERS stores "non-goals" (with hyphen).
|
||||
// classifyHeader does exact equality — these can't match, so it goes to unmapped_headers.
|
||||
test('"Non-goals" maps to out_of_scope (M7: both sides normalized to "non goals", now reachable)', () => {
|
||||
const out = parseAdrMarkdown('## Non-goals\n- Not this.');
|
||||
assert.deepEqual(out.out_of_scope, []);
|
||||
assert.ok(out.unmapped_headers.includes('Non-goals'));
|
||||
assert.deepEqual(out.out_of_scope, ['Not this.']);
|
||||
assert.ok(!out.unmapped_headers.includes('Non-goals'));
|
||||
});
|
||||
|
||||
test('"Excluded" maps to out_of_scope', () => {
|
||||
@@ -995,10 +993,10 @@ describe('parseAdrMarkdown: out_of_scope section', () => {
|
||||
assert.deepEqual(out.out_of_scope, ['Billing system.']);
|
||||
});
|
||||
|
||||
test('"Anti-goals" heading normalized to "anti goals" does NOT match synonym "anti-goals" (unreachable synonym)', () => {
|
||||
test('"Anti-goals" maps to out_of_scope (M7: both sides normalized to "anti goals", now reachable)', () => {
|
||||
const out = parseAdrMarkdown('## Anti-goals\n- Gold plating.');
|
||||
assert.deepEqual(out.out_of_scope, []);
|
||||
assert.ok(out.unmapped_headers.includes('Anti-goals'));
|
||||
assert.deepEqual(out.out_of_scope, ['Gold plating.']);
|
||||
assert.ok(!out.unmapped_headers.includes('Anti-goals'));
|
||||
});
|
||||
|
||||
test('out_of_scope is empty when no section', () => {
|
||||
@@ -1026,12 +1024,10 @@ describe('parseAdrMarkdown: deferred section', () => {
|
||||
assert.deepEqual(out.deferred, ['Optimize later.']);
|
||||
});
|
||||
|
||||
test('"Follow-up" heading normalized to "follow up" does NOT match synonym "follow-up" (unreachable synonym)', () => {
|
||||
// Synonym "follow-up" has a hyphen which normalizeAdrHeader converts to a space.
|
||||
// Since classifyHeader does exact string comparison with raw synonyms, this can't match.
|
||||
test('"Follow-up" maps to deferred (M7: both sides normalized to "follow up", now reachable)', () => {
|
||||
const out = parseAdrMarkdown('## Follow-up\n- Monitor metrics.');
|
||||
assert.deepEqual(out.deferred, []);
|
||||
assert.ok(out.unmapped_headers.includes('Follow-up'));
|
||||
assert.deepEqual(out.deferred, ['Monitor metrics.']);
|
||||
assert.ok(!out.unmapped_headers.includes('Follow-up'));
|
||||
});
|
||||
|
||||
test('"Next Steps" maps to deferred', () => {
|
||||
@@ -1074,10 +1070,10 @@ describe('parseAdrMarkdown: dependencies section', () => {
|
||||
assert.deepEqual(out.dependencies, ['Team capacity.']);
|
||||
});
|
||||
|
||||
test('"Cross-cuts" heading normalized to "cross cuts" does NOT match synonym "cross-cuts" (unreachable synonym)', () => {
|
||||
test('"Cross-cuts" maps to dependencies (M7: both sides normalized to "cross cuts", now reachable)', () => {
|
||||
const out = parseAdrMarkdown('## Cross-cuts\n- Security layer.');
|
||||
assert.deepEqual(out.dependencies, []);
|
||||
assert.ok(out.unmapped_headers.includes('Cross-cuts'));
|
||||
assert.deepEqual(out.dependencies, ['Security layer.']);
|
||||
assert.ok(!out.unmapped_headers.includes('Cross-cuts'));
|
||||
});
|
||||
|
||||
test('"Related ADRs" maps to dependencies', () => {
|
||||
@@ -1144,10 +1140,11 @@ describe('parseAdrMarkdown: update section', () => {
|
||||
assert.deepEqual(out.updates[0].entries, ['Ship v2.']);
|
||||
});
|
||||
|
||||
test('"Post-grilling" heading normalized to "post grilling" does NOT match synonym "post-grilling" (unreachable synonym)', () => {
|
||||
test('"Post-grilling" maps to updates (M7: both sides normalized to "post grilling", now reachable)', () => {
|
||||
const out = parseAdrMarkdown('## Post-grilling\n- Revised after review.');
|
||||
assert.equal(out.updates.length, 0);
|
||||
assert.ok(out.unmapped_headers.includes('Post-grilling'));
|
||||
assert.equal(out.updates.length, 1);
|
||||
assert.deepEqual(out.updates[0].entries, ['Revised after review.']);
|
||||
assert.ok(!out.unmapped_headers.includes('Post-grilling'));
|
||||
});
|
||||
|
||||
test('"Addendum" maps to updates', () => {
|
||||
@@ -1208,6 +1205,59 @@ describe('parseAdrMarkdown: consequences canonical section', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// classifyHeader — cross-bucket synonym collision (audit M7)
|
||||
// 'trade-offs' must resolve to risks (consequences_negative), not considered_options.
|
||||
// CANONICAL_HEADERS once listed 'trade-offs' under BOTH buckets; classifyHeader is
|
||||
// first-match-wins over Object.entries and considered_options is declared first, so
|
||||
// '## Trade-offs' always misclassified as options and the risks entry was dead code.
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
describe('parseAdrMarkdown: punctuated synonyms are reachable (M7)', () => {
|
||||
// Root cause: classifyHeader receives a normalized header but historically compared it
|
||||
// against RAW synonyms; normalizeAdrHeader collapses [\s:._-]+ → space and strips [^\w\s],
|
||||
// so any synonym with a hyphen/apostrophe was dead and its section went unmapped. The fix
|
||||
// normalizes both sides, making the whole class reachable while the table stays readable.
|
||||
test('"## Trade-offs" lands in consequences_negative (risks), not options_considered', () => {
|
||||
const out = parseAdrMarkdown('## Trade-offs\n- adds a per-acquire syscall\n- larger lock body');
|
||||
assert.deepEqual(out.consequences_negative, ['adds a per-acquire syscall', 'larger lock body']);
|
||||
assert.deepEqual(out.options_considered, []);
|
||||
});
|
||||
|
||||
test('all formerly-dead punctuated headers now classify to their bucket', () => {
|
||||
assert.deepEqual(parseAdrMarkdown('## Non-Goals\n- x').out_of_scope, ['x']);
|
||||
assert.deepEqual(parseAdrMarkdown('## Anti-Goals\n- x').out_of_scope, ['x']);
|
||||
assert.deepEqual(parseAdrMarkdown("## Won't Do\n- x").out_of_scope, ['x']);
|
||||
assert.deepEqual(parseAdrMarkdown('## Follow-up\n- x').deferred, ['x']);
|
||||
assert.deepEqual(parseAdrMarkdown('## Cross-cuts\n- x').dependencies, ['x']);
|
||||
assert.deepEqual(parseAdrMarkdown("## How We'll Know\n- x").consequences_positive, ['x']);
|
||||
assert.equal(parseAdrMarkdown('## Post-grilling\n- 2026-01-01: note').updates[0].heading, 'Post-grilling');
|
||||
});
|
||||
|
||||
test("'trade-offs' lives only in risks (de-duped from considered_options to avoid a cross-bucket collision)", () => {
|
||||
assert.ok(!CANONICAL_HEADERS.considered_options.includes('trade-offs'));
|
||||
assert.ok(CANONICAL_HEADERS.risks.includes('trade-offs'));
|
||||
});
|
||||
|
||||
// Reachability invariant — guards the whole class against regression: every synonym in
|
||||
// CANONICAL_HEADERS must classify (a header written as that synonym is never unmapped),
|
||||
// and no two synonyms may normalize into different buckets (cross-bucket collision).
|
||||
test('invariant: every CANONICAL_HEADERS synonym is reachable and collision-free', () => {
|
||||
const byNormalized = new Map();
|
||||
for (const [bucket, synonyms] of Object.entries(CANONICAL_HEADERS)) {
|
||||
for (const syn of synonyms) {
|
||||
const out = parseAdrMarkdown(`## ${syn}\n- z`);
|
||||
assert.ok(!out.unmapped_headers.includes(syn), `synonym "${syn}" (bucket ${bucket}) is unreachable`);
|
||||
const n = syn.toLowerCase().replace(/[\s:._-]+/g, ' ').replace(/[^\w\s]/g, '').trim();
|
||||
if (byNormalized.has(n)) {
|
||||
assert.equal(byNormalized.get(n), bucket, `normalized synonym "${n}" collides across buckets (${byNormalized.get(n)} vs ${bucket})`);
|
||||
} else {
|
||||
byNormalized.set(n, bucket);
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// classifyHeader — prefix-match branch
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user