fix(memory): stop treating thematic breaks as instruction units

[Pantoria]

Intent

Fix issue #192: a thematic-break-only line (---, **, ___) was parsed as an instruction unit by parseMemoryUnits (src/memory.js), inflating the status instruction count, getting an AG id in the analysis index, and showing as a similarity-1.00 cross-surface overlap against skills that also contain ---. The user (issue author, Pantoria) asked to implement their own ready-for-pr backpass issues, each in its own worktree and PR. Follow AGENTS.md invariants and VISION.md. No live model calls were made while implementing.
Change: CommonMark thematic breaks outside fences (3+ of the same -//_ char, optional internal spaces/tabs, up to 3 leading spaces) are not units and flush the current unit. Maintainer cautions honored: '- item' list items stay units, and --- inside fenced blocks stays content. Deliberate decision: a dash-only line directly under a plain paragraph is a setext H2 underline as in CommonMark, so the paragraph becomes the section name like a ## heading (neither is a unit); under a list item, quote, or fence it is an ordinary break; *** and ___ never underline. ANALYSIS_INDEX_VERSION bumped 3->4 deliberately, per the AGENTS.md rule, because analysis evidence stores positional AG ids validated by sanitizeEvidence and the cache key (memory content hash) would not change when only the parser does - so cached evidence on files containing --- would otherwise cite shifted ids. Rejection instructionIds are content hashes and stay valid. Out of scope: === setext and 1-2 dash underlines.

What Changed

• parseMemoryUnits (src/memory.js) uses a new exported THEMATIC_BREAK pattern (CommonMark: 3 or more of the same -, * or _, optional internal spaces or tabs, up to 3 leading spaces). A break line outside a fence is no longer an instruction unit and ends the current unit. A dash-only line directly under a plain paragraph is now read as a setext H2 underline, so that paragraph becomes the section heading. Under a list item, a quote, or a fenced line it stays an ordinary break, and *** / ___ never act as underlines. - item list items still count as units, and --- inside fences stays content.
• splitRemovalHunk (src/workspace.js) keeps a setext title and its dash underline with the content they introduce, the same way it handles ATX headings. It skips plain thematic breaks when looking for a heading's content.
• ANALYSIS_INDEX_VERSION goes from 3 to 4 (src/state.js). Cached analysis evidence uses positional AG ids, and those ids shift for files that contain ---, so old cache entries are invalidated. Covered by the new test/memory-units.test.js plus new cases in test/status.test.js and test/workspace.test.js.

🤖 Generated with Claude Code

Risk Assessment

⚠️ Medium: The parser fix is small, matches the stated intent and bumps ANALYSIS_INDEX_VERSION correctly, but the setext-heading behavior removes a unit boundary that splitRemovalHunk relied on, so an extraction can now split a heading from its content.

Testing

I ran the real backpass status CLI, in both text and JSON form, on a throwaway git repo with an isolated home directory, once at the base commit and once at the target commit. The target no longer counts ---, *** or _ _ _ lines as instructions (10 → 6) and drops the false similarity-1.00 skill overlaps. List items, content inside fences and setext sections behave as the intent describes. Adversarial edge-case files were parsed at both commits. The removal-split guard and the analysis-index version bump can only be reached through stages that call a model, so they are marked untested live. Offline checks of both passed: direct function calls, plus the three focused test files (43/43). The temporary files were removed.

• Live validation: ✅ go - 4 of 6 scenarios driven live against the product

Scenario	Result	Live	Evidence
User runs backpass status on an AGENTS.md containing ---, *** and _ _ _ breaks; the instruction count no longer counts the breaks	✅ pass	live	~/.no-mistakes/evidence/01M503RN2X4TE17RKBGZ6T3XDP/status-before-after.txt (base: 10 instructions, target: 6 instructions; --json budgets[0].instructions=6)
User runs backpass status with a skill whose frontmatter/body contains ---; no similarity-1.00 CROSS-SURFACE line is reported for a break	✅ pass	live	status-before-after.txt: base printed 'AG-002 restates demo body · similarity 1.00' (AG-002 = '---'); target prints no CROSS-SURFACE section; --json crossSurfaceDuplicates=[]
Adversarial: '- item' and '* item' list items stay units, --- / *** inside ``` or ~~~ fences stays content, and a 4-space-indented ---- is not treated as a break	✅ pass	live	~/.no-mistakes/evidence/01M503RN2X4TE17RKBGZ6T3XDP/edge-cases-units.txt and status-before-after.txt (fence unit '\n---\n' kept; list items AG-002/003 kept)
Setext decision: 'Release steps\n---' turns the paragraph into a section name. --- under a list item or quote, and *** or ___ under a paragraph, act as ordinary breaks	✅ pass	live	status-before-after.txt (target unit 'Tag the commit before release.' in section [Guide > Release steps]); edge-cases-units.txt (list/quote/paragraph units are separated rather than merged as at base)
Removal-hunk split guard: a setext title stays with its section, a plain break does not tie an extraction to a later deletion, and an ATX heading followed by a break is not orphaned	⏸️ untested	no	The prior payload did not establish a live result. This path only runs inside propose/apply after a synthesis model call, and live model calls were out of scope here (no acpx/model access allowed by u…
Cached analysis evidence from before the parser change is invalidated (ANALYSIS_INDEX_VERSION 3->4)	⏸️ untested	no	The prior payload did not establish a live result. Creating real cached evidence needs a live backpass analyze model call through acpx, which was out of scope (no live model calls). It was only chec…

Evidence: status before/after (base vs target CLI transcript)

$ cat AGENTS.md
     1	# Guide
     2	
     3	Run the linter before you commit.
     4	
     5	---
     6	
     7	## Tests
     8	
     9	- Run the unit tests with `make test`.
    10	- Keep fixtures small.
    11	
    12	***
    13	
    14	Keep commits small.
    15	
    16	 _ _ _
    17	
    18	Release steps
    19	---
    20	
    21	Tag the commit before release.
    22	
    23	## Fences
    24	
    25	`` `
    26	---
    27	`` `

$ cat .agents/skills/demo/SKILL.md
---
name: demo
description: Use when releasing.
---

# Release

Write the changelog entry.

---

Push the tag.

===== base commit: backpass status
repo /tmp/bp-test.QGqs/repo

BUDGET (always-loaded)
  AGENTS.md + skill descriptions [................................] 63 / 5,000 tok · 10 instructions
  overflow: 1 skill(s) in .agents/skills · 28 tok on trigger, 5 tok always loaded

CROSS-SURFACE (report-only)
  AG-002 restates demo body · 1 tok · similarity 1.00 · body loads on trigger; weigh placement
  AG-010 restates demo body · 3 tok · similarity 1.00 · body loads on trigger; weigh placement

--- parsed units (base)
AG-001	L3-3	[Guide]	"Run the linter before you commit."
AG-002	L5-5	[Guide]	"---"
AG-003	L9-9	[Guide > Tests]	"- Run the unit tests with `make test`."
AG-004	L10-10	[Guide > Tests]	"- Keep fixtures small."
AG-005	L12-12	[Guide > Tests]	"***"
AG-006	L14-14	[Guide > Tests]	"Keep commits small."
AG-007	L16-16	[Guide > Tests]	" _ _ _"
AG-008	L18-19	[Guide > Tests]	"Release steps\n---"
AG-009	L21-21	[Guide > Tests]	"Tag the commit before release."
AG-010	L25-27	[Guide > Fences]	"`` `\n---\n`` `"

===== target commit: backpass status
repo /tmp/bp-test.QGqs/repo

BUDGET (always-loaded)
  AGENTS.md + skill descriptions [................................] 63 / 5,000 tok · 6 instructions
  overflow: 1 skill(s) in .agents/skills · 28 tok on trigger, 5 tok always loaded

CACHE
  scan cache      0 file(s) fingerprinted
  evidence        0 ok · 0 skipped · 0 failed
  rejections      0 remembered
--- parsed units (target)
AG-001	L3-3	[Guide]	"Run the linter before you commit."
AG-002	L9-9	[Guide > Tests]	"- Run the unit tests with `make test`."
AG-003	L10-10	[Guide > Tests]	"- Keep fixtures small."
AG-004	L14-14	[Guide > Tests]	"Keep commits small."
AG-005	L21-21	[Guide > Release steps]	"Tag the commit before release."
AG-006	L25-27	[Guide > Fences]	"`` `\n---\n`` `"

Evidence: edge-case unit parsing (lists, quotes, fences, *** / ___)

     1	# Edge
     2	
     3	- first item
     4	---
     5	> quoted rule
     6	---
     7	Plain para
     8	***
     9	Another para
    10	___
    11	* * *
    12	- - -
    13	    ----
    14	~~~
    15	***
    16	~~~
    17	- item stays
    18	* star item stays

     1	# Edge
     2	
     3	    ----
     4	
     5	~~~
     6	***
     7	~~~
     8	
     9	- item stays
    10	* star item stays
    11	- ---
===== base
AG-001	L3-10	[Edge]	"- first item\n---\n> quoted rule\n---\nPlain para\n***\nAnother para\n___"
AG-002	L11-11	[Edge]	"* * *"
AG-003	L12-16	[Edge]	"- - -\n    ----\n~~~\n***\n~~~"
AG-004	L17-17	[Edge]	"- item stays"
AG-005	L18-18	[Edge]	"* star item stays"
---
AG-001	L3-3	[Edge]	"    ----"
AG-002	L5-7	[Edge]	"~~~\n***\n~~~"
AG-003	L9-9	[Edge]	"- item stays"
AG-004	L10-10	[Edge]	"* star item stays"
AG-005	L11-11	[Edge]	"- ---"
===== target
AG-001	L3-3	[Edge]	"- first item"
AG-002	L5-5	[Edge]	"> quoted rule"
AG-003	L7-7	[Edge]	"Plain para"
AG-004	L9-9	[Edge]	"Another para"
AG-005	L13-18	[Edge]	"    ----\n~~~\n***\n~~~\n- item stays\n* star item stays"
---
AG-001	L3-3	[Edge]	"    ----"
AG-002	L5-7	[Edge]	"~~~\n***\n~~~"
AG-003	L9-9	[Edge]	"- item stays"
AG-004	L10-10	[Edge]	"* star item stays"

Evidence: splitRemovalHunk review cases base vs target

== /tmp/bp-test.QGqs/base
setext title + its section (expect null: keep together):
   null
plain break before stale deletion (expect split):
   [{"text":"Tag the commit before release.\n\n---\n"},{"text":"\nDrop this stale note.\n\n"}]
ATX heading, break, extracted content (expect null):
   null
== ~/.no-mistakes/worktrees/8ce6865cb38e/01M503RN2X4TE17RKBGZ6T3XDP
setext title + its section (expect null: keep together):
   null
plain break before stale deletion (expect split):
   [{"text":"Tag the commit before release.\n\n---\n"},{"text":"\nDrop this stale note.\n\n"}]
ATX heading, break, extracted content (expect null):
   null

Evidence: analysis index version invalidation

base version 3 -> target 4
v3 evidence fresh under target (same memory hash): false

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

⚠️ **Review** - 1 info

• ⚠️ src/memory.js:112 - Setext headings now leave splitRemovalHunk (src/workspace.js:404-421) unguarded, so an extraction can separate a heading from its content. Before this change, Release steps\n--- was one unit, and the unit-kind check returned null whenever its lines had mixed kinds. With the setext branch (src/memory.js:112-125), the title line and its underline belong to no unit. The heading/content guard at src/workspace.js:416-421 only recognizes ATX # headings, so nothing else catches this. Verified: with old text # Guide\n\nKeep it small.\n\nRelease steps\n---\n\nTag the commit.\n\n## Other..., a created skill ---\nname: release\n...\n---\n\n# Release\n\nTag the commit. and a removal hunk over lines 5-9, the merged hunk is now split into two decisions: Release steps\n (deleted) and ---\n\nTag the commit.\n\n (recovered). The old parser kept them as one hunk. Every created SKILL.md's frontmatter contributes --- to the recovered counts, which makes the mismatch easy to hit. If the user accepts only the extraction, Release steps is left as an orphan paragraph. Fix: extend the heading/content guard in splitRemovalHunk to treat a setext title plus its --- underline as a heading, e.g. require the title line, the underline and the next content line to share one kind. Alternatively, expose heading spans from parseMemoryUnits so this check doesn't re-derive them.
• ℹ️ src/memory.js:113 - The setext check only looks at buffer[0]. A paragraph whose later line starts a list item (the parser keeps it in the same buffer because only a list-item buffer[0] flushes) is turned into a heading as a whole, e.g. Intro\n- item\n--- becomes the section Intro - item. In CommonMark, the list item interrupts the paragraph, so the --- would be a thematic break. This is a rare edge case, and the old parser already merged these lines into one unit, so it is noted only.

🔧 Fix applied.
2 issues (1 warning, 1 info) still open:

• ℹ️ src/memory.js:113 - The setext check only looks at buffer[0]. A paragraph whose later line starts a list item (the parser keeps it in the same buffer because only a list-item buffer[0] flushes) is turned into a heading as a whole, e.g. Intro\n- item\n--- becomes the section Intro - item. In CommonMark, the list item interrupts the paragraph, so the --- would be a thematic break. This is a rare edge case, and the old parser already merged these lines into one unit, so it is noted only.
• ⚠️ src/workspace.js:418 - Round 1's fix fixed the setext case but made the guard too broad. It now treats every non-blank line outside a unit as a heading that must share a kind with the next unit. That includes plain thematic breaks, which introduce nothing, so splitting now fails in the common case this change creates. Every created SKILL.md's frontmatter adds two --- lines to recovered, so a --- separator in the memory file usually counts as "recovered". When that break directly precedes a deleted paragraph or list item, the guard returns null and the extraction and the unrelated deletion merge back into one decision. Verified on the current code: old text # Guide\n\nKeep it small.\n\nTag the commit before release.\n\n---\n\nDrop this stale note.\n\n## Other\n\nStay.\n, a skill containing Tag the commit before release., and hunk 5-10 gives null. At the base commit, --- was a single-line unit, and at feaf96a it was ignored by the ATX-only guard, so both split this into [recovered, deleted]. The same thing happens for - a\n---\n- b, where a break follows a list item. A related case: an ATX heading directly followed by --- (## X\n---) is now one block that must share a kind, so it also refuses to split. Minimal fix: apply the guard only to heading blocks, meaning ATX # lines or a block of two or more lines ending in a dash-only setext underline. Skip a block that is a lone thematic break, which preserves the base splitting behavior. This defect is in code added by the round-1 fix for review-1.

🔧 Fix applied.
3 issues (2 warnings, 1 info) still open:

• ℹ️ src/memory.js:113 - The setext check only looks at buffer[0]. A paragraph whose later line starts a list item (the parser keeps it in the same buffer because only a list-item buffer[0] flushes) is turned into a heading as a whole, e.g. Intro\n- item\n--- becomes the section Intro - item. In CommonMark, the list item interrupts the paragraph, so the --- would be a thematic break. This is a rare edge case, and the old parser already merged these lines into one unit, so it is noted only.
• ⚠️ src/workspace.js:418 - Round 1's fix fixed the setext case but made the guard too broad. It now treats every non-blank line outside a unit as a heading that must share a kind with the next unit. That includes plain thematic breaks, which introduce nothing, so splitting now fails in the common case this change creates. Every created SKILL.md's frontmatter adds two --- lines to recovered, so a --- separator in the memory file usually counts as "recovered". When that break directly precedes a deleted paragraph or list item, the guard returns null and the extraction and the unrelated deletion merge back into one decision. Verified on the current code: old text # Guide\n\nKeep it small.\n\nTag the commit before release.\n\n---\n\nDrop this stale note.\n\n## Other\n\nStay.\n, a skill containing Tag the commit before release., and hunk 5-10 gives null. At the base commit, --- was a single-line unit, and at feaf96a it was ignored by the ATX-only guard, so both split this into [recovered, deleted]. The same thing happens for - a\n---\n- b, where a break follows a list item. A related case: an ATX heading directly followed by --- (## X\n---) is now one block that must share a kind, so it also refuses to split. Minimal fix: apply the guard only to heading blocks, meaning ATX # lines or a block of two or more lines ending in a dash-only setext underline. Skip a block that is a lone thematic break, which preserves the base splitting behavior. This defect is in code added by the round-1 fix for review-1.
• ⚠️ src/workspace.js:436 - The round-2 narrowing (which fixed review-r2-1) introduced a regression: an ATX heading can now be split from its content when a thematic break sits between them. For an ATX heading, contentLine is the next non-blank line in the hunk. When that line is a ---/*** break, the break is no longer a unit (this change's purpose), so !unitLines.has(contentLine) hits continue and the heading is never compared with the content that follows the break. The break line itself also fails isSetextTitle(lineNo - 1) because the line before it is ATX, so nothing guards the pair.

Verified on HEAD: old text # Guide\n\nKeep it small.\n\n## Notes\n---\n\nTag the commit before release.\n\n## Other\n\nStay.\n, a created skill ---\nname: release\n...\n---\n\n# Release\n\nTag the commit before release.\n, and hunk 5-9 return ["## Notes\n", "---\n\nTag the commit before release.\n\n"]. That splits the deleted heading from its extracted section. If the user accepts only the extraction, ## Notes is left as an orphan heading over ## Other. At the base commit, --- was a unit whose kind (recovered) differed from the heading (deleted), so the result was null; at feaf96a the ATX guard compared against the next non-blank line, also giving null. The ## X\n\n---\n\ncontent layout follows the same path.

The round-2 test afterHeading (test/workspace.test.js) only covers a heading the skill also contains, so it passes either way. Fix: when looking for an ATX heading's contentLine (and a setext underline's, same loop), skip lines that match THEMATIC_BREAK, so the heading is compared with the first real unit line after it. Add the case above to the test, expecting null.

🔧 Fix applied.
1 info still open:

• ℹ️ src/memory.js:113 - The setext check only looks at buffer[0]. A paragraph whose later line starts a list item (the parser keeps it in the same buffer because only a list-item buffer[0] flushes) is turned into a heading as a whole, e.g. Intro\n- item\n--- becomes the section Intro - item. In CommonMark, the list item interrupts the paragraph, so the --- would be a thematic break. This is a rare edge case, and the old parser already merged these lines into one unit, so it is noted only.

✅ **Test** - passed

✅ No issues found.

• Live validation: ✅ go - 4 of 6 scenarios driven live against the product

Scenario	Result	Live	Evidence
User runs backpass status on an AGENTS.md containing ---, *** and _ _ _ breaks; the instruction count no longer counts the breaks	✅ pass	live	~/.no-mistakes/evidence/01M503RN2X4TE17RKBGZ6T3XDP/status-before-after.txt (base: 10 instructions, target: 6 instructions; --json budgets[0].instructions=6)
User runs backpass status with a skill whose frontmatter/body contains ---; no similarity-1.00 CROSS-SURFACE line is reported for a break	✅ pass	live	status-before-after.txt: base printed 'AG-002 restates demo body · similarity 1.00' (AG-002 = '---'); target prints no CROSS-SURFACE section; --json crossSurfaceDuplicates=[]
Adversarial: '- item' and '* item' list items stay units, --- / *** inside ``` or ~~~ fences stays content, and a 4-space-indented ---- is not treated as a break	✅ pass	live	~/.no-mistakes/evidence/01M503RN2X4TE17RKBGZ6T3XDP/edge-cases-units.txt and status-before-after.txt (fence unit '\n---\n' kept; list items AG-002/003 kept)
Setext decision: 'Release steps\n---' turns the paragraph into a section name. --- under a list item or quote, and *** or ___ under a paragraph, act as ordinary breaks	✅ pass	live	status-before-after.txt (target unit 'Tag the commit before release.' in section [Guide > Release steps]); edge-cases-units.txt (list/quote/paragraph units are separated rather than merged as at base)
Removal-hunk split guard: a setext title stays with its section, a plain break does not tie an extraction to a later deletion, and an ATX heading followed by a break is not orphaned	⏸️ untested	no	The prior payload did not establish a live result. This path only runs inside propose/apply after a synthesis model call, and live model calls were out of scope here (no acpx/model access allowed by u…
Cached analysis evidence from before the parser change is invalidated (ANALYSIS_INDEX_VERSION 3->4)	⏸️ untested	no	The prior payload did not establish a live result. Creating real cached evidence needs a live backpass analyze model call through acpx, which was out of scope (no live model calls). It was only chec…

• backpass status and backpass status --json (real CLI, isolated HOME/XDG_CONFIG_HOME, disposable git repo) at base e6a030a and target b96b4fb
• parseMemoryUnits dump of the disposable AGENTS.md and two adversarial edge-case files at base and target
• splitRemovalHunk run on the setext, plain-break and ATX+break review cases at base and target (offline)
• evidenceKey/isEvidenceFresh check that v3-keyed evidence is stale under v4 (offline)
• node --import ./test/helpers/hermetic-config.js --test test/workspace.test.js test/memory-units.test.js test/status.test.js

✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; the previous command-loss issue is fixed.

_{Reviews (2) · Last reviewed commit: "no-mistakes(ci): Fixed Greptile finding ..." · Reviewed by Greptile}

[kunchenguid]

Speaking as Kun's firstmate: this is merged. Thank you @Pantoria — really appreciate you taking the time on this.