fix(#3457): split deferred items on leaf headings, not bullets (#3488)

* fix(#3457): split deferred items on leaf headings, not bullets

* fix(#3457): backfill changeset pr with 3488

---------

Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
2026-08-14 12:31:03 -04:00
committed by GitHub
parent 8bead8b0ff
commit 08940c9071
3 changed files with 347 additions and 3 deletions

View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 3488
---
parseDeferredItems now counts heading-delimited deferred items as ONE entry (a heading plus its descriptive sub-bullets) instead of one per bullet, across flat, container-heading, and mixed-depth files; headless one-bullet-per-item files are unchanged. A bolded `- **Status:** resolved` marker now resolves its item instead of surfacing as a bogus unresolved entry.

View File

@@ -890,6 +890,13 @@ function parseGapsTableItems(sectionBody: string): UatItem[] {
* `.planning/todos/pending/*.md` entry required). Every other entry —
* including one with no `status:` field at all — is UNRESOLVED and is
* surfaced.
*
* #3457: when the section body contains headings, entries are delimited by
* LEAF headings (see `splitDeferredHeadingEntries`) rather than by bullets —
* the executor convention writes one deferred item as a heading followed by
* sibling `- **Field:** …` bullets, which the bullet-only split mis-counted as
* one item PER BULLET. A body with no headings keeps the original
* one-bullet-per-item split unchanged.
*/
function parseDeferredItems(content: string): UatItem[] {
const deferredSection = collectSection(
@@ -901,8 +908,23 @@ function parseDeferredItems(content: string): UatItem[] {
const items: UatItem[] = [];
for (const entryLines of splitGapsEntries(sectionBody)) {
const fields = extractGapEntryFields(entryLines);
// #3457: heading-delimited shape — an entry's fields live in sibling bullets
// (`- **Status:** resolved`), so the bullet marker is stripped on EVERY line
// before field extraction, not just line 0 (which `extractGapEntryFields`
// does for the headless/Gaps shape, where a later `- ` line is a nested
// sub-list, not a field).
const headingEntries = splitDeferredHeadingEntries(sectionBody);
const entries = headingEntries !== null
? headingEntries.map((entryLines) => ({
lines: entryLines,
fields: extractGapEntryFields(entryLines.map(stripLeadingBulletMarker)),
}))
: splitGapsEntries(sectionBody).map((entryLines) => ({
lines: entryLines,
fields: extractGapEntryFields(entryLines),
}));
for (const { lines: entryLines, fields } of entries) {
const rawStatus = fields.status;
if (rawStatus && rawStatus.toLowerCase() === 'resolved') continue;
@@ -924,6 +946,111 @@ function parseDeferredItems(content: string): UatItem[] {
return items;
}
/**
* Strip one leading `- ` bullet marker (#3457). Heading-delimited deferred
* entries carry their fields as sibling bullets; `extractGapEntryFields` only
* de-bullets line 0 (Gaps-protective — there, a later `- ` line is a nested
* sub-list), so the deferred heading path de-bullets every line itself before
* field extraction. Non-bullet lines pass through untouched.
*/
function stripLeadingBulletMarker(line: string): string {
return line.replace(/^(\s*)-\s+/, '');
}
/**
* Split a deferred-items section body into entries delimited by LEAF headings
* (#3457). Returns `null` when the body contains no heading at all — the
* caller then falls back to `splitGapsEntries`, keeping headless
* one-bullet-per-item files byte-for-byte on the pre-#3457 path.
*
* A heading is a CONTAINER (group/provenance/title label, contributes no
* entry) iff the NEXT heading is deeper — a deeper heading lives inside its
* span. Otherwise it is a LEAF: an entry boundary. This handles all three
* corpus shapes without hardcoding a depth: flat `#` title + `##` entries
* (title's next heading is deeper → container; each `##` followed by a
* same-or-shallower heading → leaf), a `##` container with `###` entries
* (container's next heading is deeper), and mixed-depth files where a
* childless `##` entry sits alongside a `##` group with `###` children — every
* childless heading is a leaf at whatever depth it is written. The shallower
* rules the issue reports as already tried (split on every heading; shallowest
* level; deepest level) each mis-count one of these shapes.
*
* A leaf entry is [heading text, ...body lines up to the next heading] and is
* kept only when its body (minus table lines) contains at least one `- `
* bullet:
* - a prose-only or bare heading contributes nothing — "prose is not an item"
* is this parser's pre-existing contract (see the `# Notes` case);
* - a table-only body is left entirely to `parseDeferredTableItems`, which
* unions over the same section body, so the heading cannot double-count the
* table's rows.
*
* Lines before the first heading, and lines directly under a container heading
* (before its first child), are split one-bullet-per-item by the unchanged
* `splitGapsEntries` — headless parity, so loose bullets before a later
* heading group (the mixed shape) stay one item each.
*/
function splitDeferredHeadingEntries(sectionBody: string): string[][] | null {
const headings = tokenizeHeadings(sectionBody);
if (headings.length === 0) return null;
const lines = sectionBody.split('\n');
const headingByLine = new Map<number, { text: string; isContainer: boolean }>();
for (let i = 0; i < headings.length; i++) {
// Container iff the next heading is deeper (see doc comment). An empty
// heading text (`##` alone) does not itself mean container — the flag is
// carried explicitly so a bare LEAF heading still opens an entry.
const isContainer = i + 1 < headings.length && headings[i + 1].level > headings[i].level;
headingByLine.set(headings[i].line, { text: headings[i].text, isContainer });
}
const entries: string[][] = [];
let current: string[] | null = null; // accumulating a leaf heading's entry
let pending: string[] = []; // preamble / container-heading body lines
let currentHasBullet = false;
const flushCurrent = (): void => {
// Keep the leaf entry only when its body carries a bullet; the heading
// text line itself (element 0) never counts as one.
if (current !== null && currentHasBullet) entries.push(current);
current = null;
currentHasBullet = false;
};
const flushPending = (): void => {
entries.push(...splitGapsEntries(pending.join('\n')));
pending = [];
};
for (let i = 0; i < lines.length; i++) {
const lineNo = i + 1;
const heading = headingByLine.get(lineNo);
if (heading !== undefined) {
flushCurrent();
// Headless-shaped region (preamble / container-direct bullets) ends at
// ANY heading; flushing here keeps entries in document order even when
// a container's direct bullets precede its first child entry.
flushPending();
if (!heading.isContainer) {
// Leaf heading: open an entry with the heading text as line 0.
current = [heading.text];
currentHasBullet = false;
}
continue;
}
// Table lines belong to parseDeferredTableItems, never to a heading entry.
if (/^\s*\|/.test(lines[i].replace(/\r$/, ''))) continue;
if (current !== null) {
current.push(lines[i]);
if (/^\s*-\s/.test(lines[i].replace(/\r$/, ''))) currentHasBullet = true;
} else {
pending.push(lines[i]);
}
}
flushCurrent();
flushPending();
return entries;
}
/**
* Extract deferred entries from GFM pipe tables in a deferred-items.md body
* (#2766) — a UNION with the bullet scan in `parseDeferredItems`.
@@ -1021,10 +1148,22 @@ function splitGapsEntries(sectionBody: string): string[][] {
* any nested sub-list content in the template's field ordering); later
* `key:`-shaped nested-list content is captured, if it parses as one, but
* never overrides an already-seen top-level field.
*
* #3457: markdown emphasis around the KEY (`**Status:** resolved` — the
* deferred-items convention bolds every field, and a bolded resolution marker
* previously failed this regex outright and surfaced as its own bogus
* unresolved entry) is unwrapped before the match, still anchored at the
* start of the line. The unwrapped key is lower-cased, because the bolded
* convention form is Title-cased (`**Status:**`) while the field vocabulary
* this module reads is lowercase (`status`) — the same normalization
* `mapGapsHeader` already applies to table header cells. Bare (unbolded) keys
* keep their literal case, and mid-line emphasis is untouched, preserving the
* start-anchored decoy invariant above.
*/
function extractGapEntryFields(entryLines: string[]): Record<string, string> {
const fields: Record<string, string> = {};
const fieldLineRe = /^([A-Za-z_][A-Za-z0-9_-]*):\s*(.*)$/;
const boldedKeyRe = /^\*+([A-Za-z_][A-Za-z0-9_-]*):\*+/;
entryLines.forEach((rawLine, idx) => {
const line = rawLine.replace(/\r$/, '');
@@ -1033,7 +1172,8 @@ function extractGapEntryFields(entryLines: string[]): Record<string, string> {
// `splitGapsEntries` already folding it in — it is not itself a field
// line unless it independently matches `key: value` after stripping.
const bulletStripped = line.match(/^(\s*)-\s+(.*)$/);
const content = idx === 0 && bulletStripped ? bulletStripped[2] : line.trim();
const content = (idx === 0 && bulletStripped ? bulletStripped[2] : line.trim())
.replace(boldedKeyRe, (_m, key: string) => `${key.toLowerCase()}:`);
const m = fieldLineRe.exec(content);
if (!m) return;

View File

@@ -1974,6 +1974,205 @@ describe('#2766 parseDeferredItems: GFM table shape', () => {
});
});
// ─── #3457: heading-delimited deferred entries ────────────────────────────────
describe('#3457 parseDeferredItems: heading-delimited entries', () => {
const items = (md) => parseDeferredItems(md);
const names = (md) => items(md).map(i => i.name);
test('issue minimal repro: heading + sibling field bullets = ONE item', () => {
const got = items([
'# Deferred Items',
'',
'## Deferred Items',
'',
'### Widget layout suite — 3 failing assertions',
'',
'- **What:** three assertions fail on widget alignment.',
'- **Cause:** a pre-existing uncommitted edit in the working tree.',
'- **Scope:** out of this plan\'s scope.',
'- **Disposition:** NOT fixed here; left for a follow-up plan.',
].join('\n'));
assert.strictEqual(got.length, 1, JSON.stringify(got.map(i => i.name)));
assert.match(got[0].name, /Widget layout suite — 3 failing assertions/);
assert.match(got[0].name, /three assertions fail/);
assert.strictEqual(got[0].result, 'unresolved');
assert.strictEqual(got[0].category, 'deferred');
});
test('flat shape: `#` title + `##` entries — title is not an item', () => {
const got = names([
'# Deferred Items',
'',
'## DEF-01 renderer fix',
'',
'- **What:** a.',
'',
'## DEF-02 seed drift',
'',
'- **What:** b.',
].join('\n'));
assert.strictEqual(got.length, 2, JSON.stringify(got));
assert.match(got[0], /^DEF-01 renderer fix/);
assert.match(got[1], /^DEF-02 seed drift/);
});
test('container shape: `##` group label + `###` entries — group is not an item, entries not collapsed', () => {
// The shape both shallow-boundary rules get wrong: "count all headings"
// counts the group; "shallowest level" collapses both entries into one.
const got = names([
'# Deferred Items',
'',
'## Plan 28-02 provenance',
'',
'### Entry A — flaky seed',
'',
'- **What:** a.',
'',
'### Entry B — slow build',
'',
'- **What:** b.',
].join('\n'));
assert.strictEqual(got.length, 2, JSON.stringify(got));
assert.match(got[0], /^Entry A — flaky seed/);
assert.match(got[1], /^Entry B — slow build/);
// A following entry's heading must not be swallowed into the previous
// entry's name (the pre-fix bullet-split folded it in).
assert.ok(!got[0].includes('Entry B'), got[0]);
});
test('mixed shape: loose preamble bullets before a later heading group stay one-per-bullet', () => {
const got = names([
'# Deferred Items',
'',
'- loose preamble item one',
'- loose preamble item two',
'',
'## Group under here',
'',
'### Entry C',
'- **What:** c.',
].join('\n'));
assert.deepStrictEqual(
got.map(n => n.replace(/\s+- \*\*What:\*\*.*$/, '')),
['loose preamble item one', 'loose preamble item two', 'Entry C'],
JSON.stringify(got),
);
});
test('mixed depths: childless `##` entry alongside a `##` group with `###` children — all counted', () => {
// The case "deepest heading level present" rules miss: the childless ##
// is shallower than the deepest level in the file but is still an entry.
const got = names([
'# Deferred Items',
'',
'## Group with children',
'',
'### Entry A',
'- **What:** a.',
'',
'### Entry B',
'- **What:** b.',
'',
'## Standalone entry',
'',
'- **What:** standalone.',
].join('\n'));
assert.strictEqual(got.length, 3, JSON.stringify(got));
assert.ok(got.some(n => /^Standalone entry/.test(n)), JSON.stringify(got));
});
test('no headings at all → one-bullet-per-item, unchanged names (no regression)', () => {
assert.deepStrictEqual(
names('## Deferred Items\n\n- entry one\n- entry two\n'),
['entry one', 'entry two'],
);
});
test('bolded `- **Status:** resolved` under a leaf heading resolves the entry', () => {
const got = names([
'## Deferred Items',
'',
'### Item resolved inline',
'',
'- **What:** x.',
'- **Status:** resolved',
].join('\n'));
assert.deepStrictEqual(got, [], JSON.stringify(got));
});
test('bolded `- **Status:** resolved` with no headings: resolves itself, never surfaces as its own item', () => {
// The issue's negative control: previously count = 2 with a literal
// `**Status:** resolved` pseudo-entry; must match the bare form's count = 1.
const got = names('## Deferred Items\n\n- **What:** one deferred item.\n- **Status:** resolved\n');
assert.strictEqual(got.length, 1, JSON.stringify(got));
assert.match(got[0], /\*\*What:\*\* one deferred item\./);
assert.ok(!got.some(n => /Status/.test(n)), JSON.stringify(got));
});
test('bare `status: resolved` controls keep working (no regression on #2287)', () => {
// Headless continuation form.
assert.strictEqual(names('## Deferred Items\n\n- a\n status: resolved\n- b\n').length, 1);
// Bare status as a sibling bullet under a leaf heading.
assert.strictEqual(names([
'## Deferred Items',
'',
'### Item resolved bare',
'',
'- **What:** x.',
' status: resolved',
].join('\n')).length, 0);
});
test('leaf heading over a table-only body → table rows only, no double-count', () => {
// parseDeferredTableItems owns the rows; the heading must not add an item.
const got = names([
'## Discovered during 01-03',
'',
'| Test | Failing seeds |',
'|------|---------------|',
'| test_a | 0, 1 |',
].join('\n'));
assert.deepStrictEqual(got, ['test_a — 0, 1'], JSON.stringify(got));
});
test('prose-only or bare headings contribute no items', () => {
// "Prose is not an item" is this parser's pre-existing contract (#2766
// `# Notes` case) — heading mode must not start counting prose sections.
assert.deepStrictEqual(names('## Deferred Items\n\n### Musings\n\njust prose here.\n'), []);
assert.deepStrictEqual(names('## Deferred Items\n\n### A bare heading with no body\n'), []);
});
test('CRLF files: heading entries still split and resolve', () => {
const got = names('## Deferred Items\r\n\r\n### Entry\r\n\r\n- **What:** x.\r\n- **Status:** resolved\r\n');
assert.deepStrictEqual(got, [], JSON.stringify(got));
});
test('mid-line `status: resolved` decoy under a heading must not resolve the entry', () => {
// The #2287 decoy invariant, ported to the heading shape: a status-shaped
// phrase inside entry prose is never a field.
const got = items([
'## Deferred Items',
'',
'### Entry with decoy prose',
'',
'- note: saw a status: resolved message in the log',
].join('\n'));
assert.strictEqual(got.length, 1, JSON.stringify(got.map(i => i.name)));
assert.strictEqual(got[0].result, 'unresolved');
});
});
// ─── Bug 3: table-shaped ## Gaps section ──────────────────────────────────────
describe('#2766 parseGapsItems: GFM table shape', () => {