From 8d0b6868ae3efdce2b7e8aa47d15475cbb056a2f Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 18 Sep 2026 01:38:14 -0400 Subject: [PATCH] fix(#4725): write normalization preserves tight paragraph-list shape (#4842) * test(#4725): write normalization must not reflow untouched prose (failing first) * fix(#4725): stop write normalization injecting a blank before a list after prose * test(#4725): repair ordered-list fixture and list-spacing snapshot * test(#4725): assert whole-file prose stability, fix heading-list comment * chore(#4725): backfill changeset PR number (4842) --------- Co-authored-by: sim --- .changeset/happy-ravens-rest.md | 5 + src/shell-command-projection.cts | 14 +- tests/audit-command-cutover.test.cjs | 12 +- tests/concurrency-safety.test.cjs | 9 +- tests/roadmap.test.cjs | 48 ++++++ ...l-command-projection-md-normalize.test.cjs | 152 +++++++++++++++++- 6 files changed, 222 insertions(+), 18 deletions(-) create mode 100644 .changeset/happy-ravens-rest.md diff --git a/.changeset/happy-ravens-rest.md b/.changeset/happy-ravens-rest.md new file mode 100644 index 000000000..91b73b22c --- /dev/null +++ b/.changeset/happy-ravens-rest.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4842 +--- +**Markdown writes no longer insert a blank line between a paragraph and a following list** — every .md write re-normalized the whole document and split tight paragraph→list transitions, reflowing prose the command never touched (e.g. `roadmap.update-plan-progress` editing unrelated phase-section prose). Tight lists now stay byte-identical on disk. (#4725) diff --git a/src/shell-command-projection.cts b/src/shell-command-projection.cts index ccf833b01..699acff33 100644 --- a/src/shell-command-projection.cts +++ b/src/shell-command-projection.cts @@ -1156,11 +1156,15 @@ function _normalizeMd(content: string): string { if (isFenceLine && i > 0 && prevTrimmed !== '' && !insideFence[i] && (i === 0 || !insideFence[i - 1] || isFenceLine)) { if (i === 0 || !insideFence[i - 1]) result.push(''); } - // #3854: the `!/^\s/.test(prev)` guard mirrors the after-a-bullet rule below — - // an indented non-bullet line is a CONTINUATION of the previous list item, not a - // preceding paragraph, so no separating blank may be injected before this bullet - // (that injection converted every tight multi-line list to a loose one on write). - if (/^(\s*[-*+]\s|\s*\d+\.\s)/.test(line) && i > 0 && prevTrimmed !== '' && !/^(\s*[-*+]\s|\s*\d+\.\s)/.test(prev) && !/^\s/.test(prev) && prevTrimmed !== '---') result.push(''); + // No "separate a list from a preceding paragraph" rule (#3854, #4725). + // This pass re-normalizes the ENTIRE document on every .md write, so an + // inserted blank before a bullet whose previous line is ordinary prose + // reflowed text the command never touched (converting tight lists to + // loose ones); #3854 first guarded the indented-continuation predecessor, + // #4725 removed the rule outright. A paragraph→list transition stays + // exactly as the author wrote it; a heading→list separation still comes + // from the after-heading rule below. The after-a-bullet rule at the end + // of this loop is a different transition (list→prose) and is unaffected. result.push(line); if (/^#{1,6}\s/.test(trimmed) && i < lines.length - 1 && (lines[i + 1] ?? '').trimEnd() !== '') result.push(''); if (/^```\s*$/.test(trimmed) && i > 0 && insideFence[i - 1] && i < lines.length - 1 && (lines[i + 1] ?? '').trimEnd() !== '') result.push(''); diff --git a/tests/audit-command-cutover.test.cjs b/tests/audit-command-cutover.test.cjs index f4622eb9c..b2ac019af 100644 --- a/tests/audit-command-cutover.test.cjs +++ b/tests/audit-command-cutover.test.cjs @@ -2189,11 +2189,13 @@ describe('bug #950: quick-task SUMMARY must carry status: complete', () => { const criticalBlock = lines.slice(criticalIdx, criticalEndIdx).join('\n'); assert.doesNotMatch(criticalBlock, /status: acknowledged/, 'the CRITICAL entry (and its continuation line) must NEVER be touched'); assert.match(criticalBlock, /see also: - minor typo/, 'the CRITICAL entry continuation line is preserved verbatim'); - // Measured: the write seam's `_normalizeMd` (src/shell-command-projection.cts:837) - // inserts a blank line before a list item whose predecessor is a non-blank, non-list - // line — so a blank line appears between the CRITICAL continuation line and the - // "- minor typo" bullet after this write. That is repo-wide `.md`-write normalization - // (50 callers through the single write seam), not something specific to this feature. + // The write seam's `_normalizeMd` carries no before-a-bullet insertion + // rule: #3854 guarded the indented-continuation predecessor, #4725 + // removed the prose-predecessor case outright. No blank line is + // injected between the CRITICAL continuation line and the "- minor + // typo" bullet by this write — repo-wide `.md`-write normalization + // (50 callers through the single write seam), not something specific + // to this feature. assert.match(content, /- minor typo\n {2}status: acknowledged/, 'the standalone "minor typo" entry (its OWN span) now carries the marker'); const after = audit(tmpDir); diff --git a/tests/concurrency-safety.test.cjs b/tests/concurrency-safety.test.cjs index 281914126..85daa7acf 100644 --- a/tests/concurrency-safety.test.cjs +++ b/tests/concurrency-safety.test.cjs @@ -420,7 +420,9 @@ describe('normalizeMd behavioral equivalence', () => { assert.ok(result.includes('\n\n## Section One\n\n'), 'Section One heading needs blank lines'); assert.ok(result.includes('\n\n## Section Two\n\n'), 'Section Two heading needs blank lines'); assert.ok(result.includes('\n\n## Section Three\n\n'), 'Section Three heading needs blank lines'); - assert.ok(result.includes('Paragraph text.\n\n- item 1'), 'list should have blank line before'); + // #4725: paragraph→list is preserved byte-identical — the removed + // before-a-bullet rule used to inject a blank here (tight list turned loose). + assert.ok(result.includes('Paragraph text.\n- item 1'), 'tight paragraph→list stays byte-identical'); assert.ok(result.includes('\n\n```bash'), 'code block should have blank line before'); assert.ok(result.includes('```\n\nAfter code.'), 'code block should have blank line after'); assert.ok(result.includes('echo hello'), 'code content should be preserved'); @@ -458,7 +460,10 @@ describe('normalizeMd snapshot tests', () => { test('snapshot - list spacing', () => { const input = 'Paragraph\n- item 1\n- item 2\nAnother paragraph'; - const expected = 'Paragraph\n\n- item 1\n- item 2\n\nAnother paragraph\n'; + // #4725: the paragraph→list transition is preserved byte-identical (the + // removed before-a-bullet rule used to inject a blank there); the + // list→prose separation below is the after-a-bullet rule and stays. + const expected = 'Paragraph\n- item 1\n- item 2\n\nAnother paragraph\n'; const result = normalizeMd(input); assert.strictEqual(result, expected, `List spacing snapshot mismatch.\nGot: ${JSON.stringify(result)}\nExpected: ${JSON.stringify(expected)}` diff --git a/tests/roadmap.test.cjs b/tests/roadmap.test.cjs index 03d544685..1a83d898e 100644 --- a/tests/roadmap.test.cjs +++ b/tests/roadmap.test.cjs @@ -969,6 +969,54 @@ describe('roadmap update-plan-progress command', () => { assert.ok(roadmapContent.includes('1/2'), 'roadmap should contain updated plan count'); }); + test('#4725: leaves surrounding prose byte-identical (bold paragraph above a tight list)', () => { + const fixture = [ + '# Roadmap', + '', + '### Phase 654: Stream Consumer', + '', + '**Goal:** bind the evidence HMAC key in production', + '', + '**Scope narrowed 2026-09-14, round `663-DISPOSITION` Q7 (3/3)** (`.planning/decisions/663-disposition.md`):', + '- The "for retry" javadoc correction moved to Phase 663', + '- Per Q6 (3/3), crash and failed-XACK residue stay this phase\'s population', + '', + '**Plans:** 2 plans', + '', + 'Plans:', + '', + '- [ ] 654-01-PLAN.md — (wave 1) the evidence HMAC key binds in production', + '- [ ] 654-02-PLAN.md — (wave 2) consumer hardening', + '', + ].join('\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), fixture); + + const phaseDir = path.join(tmpDir, '.planning', 'phases', '654-stream'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '654-01-PLAN.md'), '# Plan 1'); + fs.writeFileSync(path.join(phaseDir, '654-02-PLAN.md'), '# Plan 2'); + fs.writeFileSync(path.join(phaseDir, '654-01-SUMMARY.md'), '# Summary 1'); + + const result = runGsdTools('roadmap update-plan-progress 654', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.updated, true, 'should update'); + + // Whole-file assertion per the issue's Expected: ONLY the **Plans:** + // count line and the matching checkbox row change; every other byte — + // the bold paragraph and its tight list included — is untouched. + const expected = fixture + .replace('**Plans:** 2 plans', '**Plans:** 1/2 plans executed') + .replace('- [ ] 654-01-PLAN.md', '- [x] 654-01-PLAN.md'); + const written = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8'); + assert.strictEqual( + written, + expected, + `the file must differ from the input by exactly the two intended edits; written:\n${written}` + ); + }); + test('counts plans and summaries from plans/ subdirectory layout (#3053)', () => { fs.writeFileSync( path.join(tmpDir, '.planning', 'ROADMAP.md'), diff --git a/tests/shell-command-projection-md-normalize.test.cjs b/tests/shell-command-projection-md-normalize.test.cjs index add35feba..660889d07 100644 --- a/tests/shell-command-projection-md-normalize.test.cjs +++ b/tests/shell-command-projection-md-normalize.test.cjs @@ -18,12 +18,17 @@ * indented next lines; the before-a-bullet rule must too. * * These tests pin both directions: tight lists stay tight through the write - * seam, and the legitimate paragraph↔list separations the rule exists for - * still happen. + * seam. #4725 (maintainer brief on the issue) removed the paragraph→list + * half of the old "separate a list from a preceding paragraph" rule: the + * pass re-normalizes whole documents, so the inserted blank reflowed + * untouched prose — a paragraph→list transition is now preserved + * byte-identical. Heading→list and list→prose separations are OTHER rules' + * transitions and still happen (pinned below). */ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); +const fc = require('fast-check'); const fs = require('fs'); const os = require('node:os'); const path = require('path'); @@ -73,12 +78,15 @@ describe('#3854: write normalization preserves tight multi-line lists', () => { ); }); - test('the paragraph→list separation the rule exists for STILL happens', () => { + test('#4725: the paragraph→list transition is preserved byte-identical (no separating blank)', () => { + // Supersedes the pre-#4725 pin that the separation "STILL happens": the + // inserted blank reflowed untouched prose on every full-file .md write. const doc = 'A lead-in paragraph.\n- first item\n'; const { content } = normalizeContent(MD, doc); - assert.ok( - content.includes('A lead-in paragraph.\n\n- first item'), - 'a list following a paragraph still gets its separating blank' + assert.strictEqual( + content, + doc, + 'a list following a paragraph stays byte-identical — no separating blank' ); }); @@ -109,3 +117,135 @@ describe('#3854: write normalization preserves tight multi-line lists', () => { } }); }); + +describe('#4725: write normalization must not reflow untouched prose', () => { + // The issue's document shape: an ordinary (bold) paragraph immediately + // followed by a tight bullet list, inside a phase section that + // `roadmap update-plan-progress` never targets. Every full-file .md write + // re-normalized the transition and injected a blank, converting the tight + // list to a loose one and editing prose the command never touched. + const issueFixture = () => [ + '### Phase 654: Stream Consumer', + '', + '**Scope narrowed 2026-09-14, round `663-DISPOSITION` Q7 (3/3)** (`.planning/decisions/663-disposition.md`):', + '- The "for retry" javadoc correction moved to Phase 663', + '- Per Q6 (3/3), crash and failed-XACK residue stay this phase\'s population', + '', + '**Plans:** 2 plans', + '', + 'Plans:', + '', + '- [ ] 654-01-PLAN.md — (wave 1) the evidence HMAC key binds in production', + '- [ ] 654-02-PLAN.md — (wave 2) consumer hardening', + ].join('\n') + '\n'; + + const blankCount = (s) => s.split('\n').filter((l) => l.trim() === '').length; + + test('#4725: a paragraph directly above a bullet list is preserved byte-identical (no injected blank)', () => { + const doc = issueFixture(); + const { content } = normalizeContent(MD, doc); + assert.ok( + content.includes('663-disposition.md`):\n- The "for retry"'), + 'no blank may be injected between the paragraph and its tight list' + ); + assert.strictEqual( + blankCount(content), + blankCount(doc), + 'blank-line count must round-trip unchanged' + ); + }); + + test('#4725: a paragraph directly above an ordered list is preserved byte-identical', () => { + const doc = [ + 'Lead-in prose line.', + '1. first numbered item', + '2. second numbered item', + ].join('\n') + '\n'; + const { content } = normalizeContent(MD, doc); + assert.ok( + content.includes('Lead-in prose line.\n1. first numbered item'), + 'no blank may be injected between the paragraph and its tight ordered list' + ); + assert.strictEqual(blankCount(content), blankCount(doc), 'blank-line count must round-trip unchanged'); + }); + + test('#4725 negative space: --- above a list stays byte-identical', () => { + const doc = '---\n- item\n'; + const { content } = normalizeContent(MD, doc); + assert.strictEqual(content, doc, 'a --- above a list never triggered the rule and must still not gain a blank'); + }); + + test('#4725 negative space: list→prose separation is a different transition and still happens', () => { + // The after-a-bullet rule is NOT part of #4725; the list→prose separation + // it provides must keep working. + const doc = '- a\n- b\nAfter prose.\n'; + const { content } = normalizeContent(MD, doc); + assert.ok(content.includes('- b\n\nAfter prose.'), 'list→prose keeps its separating blank'); + }); + + test('#4725: normalization is idempotent on the paragraph-above-list fixture', () => { + const doc = issueFixture(); + const once = normalizeContent(MD, doc).content; + const twice = normalizeContent(MD, once).content; + assert.strictEqual(once, twice, 'second pass must be a no-op'); + assert.strictEqual(once, doc, 'and the first pass must not have grown the document'); + }); + + test('#4725: CRLF paragraph-above-list does not grow a blank', () => { + const doc = 'Lead-in paragraph.\r\n- first item\r\n- second item\r\n'; + const { content } = normalizeContent(MD, doc); + assert.ok(!content.includes('\r'), 'CRLF is normalized to LF'); + assert.ok( + content.includes('Lead-in paragraph.\n- first item'), + 'the CRLF variant of the transition is byte-stable too (LF-form)' + ); + }); + + test('#4725 property: prose/list documents are byte-stable through the write seam', () => { + // Alphabet: prose lines, tight bullet/ordered items, blank lines. Within + // this domain NO other normalization rule may act, so byte-stability pins + // exactly the #4725 seam. Deliberate exclusions, each the domain of a + // different rule or a separately-recorded defect: + // - headings/fences: heading & fence blank-line rules own those + // transitions (rows 6-8 of the test matrix pin them singly); + // - list→prose adjacency: the after-a-bullet rule's domain, out of + // #4725's scope — the generator inserts the blank it will re-insert; + // - fence lines: the blank-line rules do not consult fence state + // (pre-existing fence-blind reflow, separate filing). + const proseLine = fc.stringMatching(/^[A-Z][a-z]+(?: [a-z]+){0,7}[.:]?$/); + const bulletItem = fc.tuple( + fc.constantFrom('- ', '* ', '+ '), + fc.stringMatching(/^[A-Za-z][A-Za-z0-9 ]{0,40}$/) + ).map(([marker, text]) => marker + text); + const orderedItem = fc.tuple( + fc.integer({ min: 1, max: 99 }), + fc.stringMatching(/^[A-Za-z][A-Za-z0-9 ]{0,40}$/) + ).map(([n, text]) => `${n}. ${text}`); + const rawLine = fc.oneof(proseLine, bulletItem, orderedItem, fc.constant('')); + // Repair the generated line list so no rule OTHER than #4725's can fire: + // no list→prose adjacency (blank inserted — rule 6's domain), no doubled + // blanks (blank-run collapse), no trailing blanks (trailing-newline trim). + const constrain = (lines) => { + const out = []; + let prevClass = 'blank'; + for (const line of lines) { + const cls = line === '' ? 'blank' : /^(?:[-*+] |\d+\. )/.test(line) ? 'item' : 'prose'; + if (cls === 'prose' && prevClass === 'item') out.push(''); + if (cls === 'blank' && prevClass === 'blank') continue; + out.push(line); + prevClass = cls; + } + while (out.length > 0 && out[out.length - 1] === '') out.pop(); + return out; + }; + const docGen = fc.array(rawLine, { minLength: 1, maxLength: 40 }) + .map((lines) => constrain(lines).join('\n') + '\n'); + fc.assert( + fc.property(docGen, (doc) => { + const out = normalizeContent(MD, doc).content; + assert.strictEqual(out, doc, `byte-stability violated for:\n${JSON.stringify(doc)}`); + }), + { seed: 4725, numRuns: 300, endOnFailure: true } + ); + }); +});