Skip to content

feat(adf): compile preserves retained sections (#318 5/6) - #358

Merged
stackbilt-admin merged 3 commits into
mainfrom
auto/feature/issue-303-retained
Oct 16, 2026
Merged

stackbilt-admin merged 3 commits into
mainfrom
auto/feature/issue-303-retained

Conversation

@stackbilt-admin

Copy link
Copy Markdown
Member

Closes #303
Refs #318

Summary

adf compile --write keeps the user's retained sections (## Environment and the other headings adf tidy retains) in a fenced block at the end of each compiled file, instead of deleting them (compiled re-compile, the #303 hole) or refusing on them (pointer conversion, #295).

<!-- charter:retained:start -->
<!-- Kept verbatim by charter adf compile. Edit freely between the retained markers. -->

## Environment
- Use WSL for builds
<!-- charter:retained:end -->
  • New packages/cli/src/commands/adf-retained.ts: composeCompiled(fresh, existing, file) is the one function --write, --check and vendor restore share. It reads from block, legacy, pointer, unmanaged or none. Also linesConvertWouldDrop and compiledMatchesFresh. It never imports adf.ts (adf migrate writes the generic pointer for GEMINI.md/AGENTS.md: POINTER_TEMPLATES built during a circular import #355 class).
  • adf-compile.ts: --write writes the composed output. refused-retained is retired. New refusals for converting a pointer: refused-bloated (remedy adf tidy --source F first, then --force, which discards N item(s)) and refused-would-drop. Rows add retainedSections and retainedSource. --check compares against the composed output and adds staleReason (content | retained-unfenced).
  • adf-vendor.ts: restore's compiled-differs ignores the block and legacy retained sections, and restore carries them into the pointer (keptSections). The retained-divergent refusal is retired.
  • Untouched: packages/adf (compileAdf), score.ts, adf.ts, adf-migrate.ts, index.ts, tidy, doctor, hooks.

Before → after

Case main @ 67f3c28 this PR
Compiled, no retained content re-renders byte-identical (fixture from main)
Compiled + user ## Environment --write deletes it, exit 0 moved into the block, kept on every later write
Same, --check stale forever after a hand edit stale (retained-unfenced) until one --write, then fresh
Edit inside the block stale fresh, and kept
Pointer with notes → --mode compiled refused-retained written, notes carried
Pointer with bloat → --mode compiled written, bloat lost refused-bloated, tidy remedy first
Pointer with other stray lines lost refused-would-drop; --force discards
--mode compiled --force over a hand-authored file everything replaced retained sections carried, the rest discarded
restore of a compiled file with notes retained-divergent notes carried into the pointer
score / tidy / doctor / stdout compile — unchanged

Sign-offs (Gate 2)

  • CP-1: compiled files carrying user retained-heading sections change bytes on the next --write. Repos without them stay byte-identical, pinned by the compile-no-retained.main-67f3c28.json fixture snapshot, generated by main's build.
  • CP-2: such legacy files read stale (retained-unfenced) under --check until one --write.
  • CP-7: refused-retained is no longer emitted (CHANGELOG: JSON consumers no longer see it), and neither is restore's retained-divergent.
  • CP-3: HTML markers for every target, .cursorrules included. CP-4: carried under --mode compiled --force. CP-5: on a heading collision both are kept, with a warning. CP-6: score keeps the pure compile. replacedPointers is not added (previousMode covers it).

Invariants and where they are tested (adf-retained.test.ts unless noted)

  • INV-P5-1 (no retained content → byte-identical to main): regression and structure › … byte-identically to main @ 67f3c28. Fixture scenarios: absent, hand-edited re-compile with a stale ## PROTOCOL, CRLF re-compile, clean pointers. The e2e repeated this against a main build.
  • INV-P5-2 (never deletes a retained section): #303 hole CLI test; conversion tests for pointer, --force and unmanaged --force; adf-compile.test.ts --force still carries….
  • INV-P5-3 (idempotent): is idempotent for every source, and two --write runs are byte-identical for every target … LF and CRLF.
  • INV-P5-4 (generated headings never resurrect): never resurrects a generated heading from a legacy file.
  • INV-P5-5 (block edits never stale): copies an existing block verbatim…, plus the edit step of the CLI adf compile: preserve retained sections instead of only refusing on them #303 test.
  • INV-P5-6 (one compose function): source pin, plus behaviour (write output is check-fresh; restore's compiled-differs test).
  • INV-P5-7 (stdout and compileAdf unchanged): stdout mode prints the pure render even when the file has a block.
  • INV-P5-8 (additive JSON, no refused-retained): never emits refused-retained…, plus the check staleReason assertions and the updated rows in adf-vendor-consumers.test.ts.
  • INV-P5-9 (adf.ts, adf-migrate.ts, index.ts untouched): diff. LOC evidence still PASS at 1027/1041 and 780/792 (unchanged files).
  • INV-P5-10 (no adf.ts import; works when the CLI entry loads first): adf-retained.ts never imports adf.ts… (resets modules and imports ../index first).

Mutation-checked: each core guard (verbatim block copy, generated-heading filter, empty-means-no-block, check and write compose, conversion guards, restore differs and carry, embedded-render guard, fence boundary, guard-by-markers) was broken in turn, and at least one test failed each time.

The sentinel claim was verified: stripCharterSentinels matches <!-- charter:retained:start|end --> (tidy's extractBeyondPointer, doctor's bloat scan, and the markdown parser used by migrate). Compiled files are skipped by the banner in tidy, doctor and migrate. Score reads the file text or the pure compile, so it needs no stripping.

NOT in PR5

  • New retained headings, or any change to isRetainedHeading. A hash-style sentinel for .cursorrules.
  • CRLF-tolerant --check (still strict bytes). Banner text change. Legacy agents.md (PR6).

Implemented via constellation-loop against a locked blueprint.

🤖 Generated with Claude Code

 #303)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@stackbilt-admin stackbilt-admin left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CHANGES_REQUESTED

Adversarial review of #358 (PR 5/6 of #318, closes #303) at 2756951. I built it from a detached worktree and compared it with an origin/main (67f3c28) build. I ran 24 CP-1 variants, 27 data-loss and block variants, 23 pointer and unmanaged flows, restore round trips, 7 circular-import load orders, and --check JSON parity. Harness: rv358/{h,cp1,dl,ptr,rs,circ}.cjs in the session scratchpad.

The core design is sound and CP-1 holds for every ordinary repo. It holds for all 4 targets, with and without weights, with CRLF, with or without a trailing newline, with extra trailing blank lines, with pre-#207 index paths, with a stale ## VENDORS, with stale ## PROTOCOL and ## SESSION_PROTOCOL _(load-bearing)_ sections, with lowercase ## environment, ### Environment, stray edits, and with no ROLE or ON_DEMAND. In all of these PR --write is byte-identical to main, --write is idempotent, and --check exits 0 after one --write. The compiler banner and title text have not changed since #203, so files compiled by older Charter versions produce no hidden diff. The three items below must be fixed (or, for item 1, explicitly signed off) before merge.

Required changes

  1. Text from a .ai/ value is carried into the block when .ai/ changes or deletes it. This breaks CP-1 and INV-P5-4. Location: packages/cli/src/commands/adf-retained.ts:130-136 (legacySections dedupe).

    • The case: a .ai/ text value that itself contains a retained heading. For example NOTES: with the lines ## Session notes / - keep tabs, or ## Environment / Use WSL.. The dedupe (fresh.includes(text)) handles only the case where .ai/ is unchanged.
    • What happens: when the user later edits that value (Use WSL. → Use Docker.) or removes the module, the existing compiled file still holds the old text. That old text no longer matches the fresh render, so it is taken as user content and moved into a new charter:retained block.
    • Result: the file carries both the new and the old instruction, or only the old one after a removal, permanently. Each later --write copies the block. Reproduced on all 4 targets, three variants (cp1.cjs: "then .ai edited", "then .ai removed it", "## Environment, .ai edited").
    • Why it matters: this repo has no user-written retained section, yet its bytes differ from main. It is the #296 class ("deleted rules come back") through a new route.
    • Fix, one of:
      • (a) In legacy mode, do not adopt a retained H2 that sits directly inside a rendered section's body. The renderer's text bodies have no blank line before an embedded ## line, while a hand-added section follows a blank line. Pin it with tests.
      • (b) Do not adopt a legacy heading line that appears, dedented, in any current .ai/ module. This covers edits only, so it must be combined with (c) for removals.
      • (c) An explicit operator CP sign-off that accepts this carve-out, plus a CHANGELOG and cli-reference sentence and a test that pins the behaviour.
    • My recommendation: (a), or (b) plus (c). Do not merge it silently.
  2. The file grows without bound when a .ai/ text value contains the start marker line. Location: adf-retained.ts:167 (lines.findIndex(l => l.trim() === RETAINED_START)).

    • The case: the rendered .ai/ text contains <!-- charter:retained:start -->. That line is found first and treated as the block start, so the "block" is everything from it to EOF. That includes generated sections and the module index. It is then appended to the next render.
    • Result: CLAUDE.md went 445 → 735 → 995 → 1255 → 1515 bytes over 5 --write runs. --check stays stale forever, and each run duplicates generated content.
    • Fix: skip marker lines that the fresh render itself contains, for example by matching only after the last line that fresh accounts for, or by counting occurrences in fresh. Add a test. An end-only marker in .ai/ text is already stable (429 bytes on every run).
  3. Section parsing ignores code fences, so part of a "kept" section is dropped. This breaks INV-P5-2 (and W4 under --force). Location: adf-retained.ts:79 (isH2) and :85-100 (sectionsOf).

    • The bug: every line whose trimmed form starts with ## starts a new section, even inside a fenced code block or an indented code block.
    • Example: a legacy ## Environment that holds a fenced code block with a ## install deps comment line in it:
      ## Environment
      
      ```sh
      ## install deps
      pnpm i
      
      • after fence
      The section is cut at the comment. On `--write`, `## install deps`, `pnpm i` and `- after fence` are deleted, and the run prints `Written CLAUDE.md (kept ## Environment)` with exit 0. A `<!-- agents-modules -->` line inside a fence also cuts the section, and so does a 4-space-indented `    ## x` (CommonMark treats that as code, not a heading).
      
    • Where it shows up:
      • Legacy compiled files.
      • --mode compiled --force over unmanaged files.
      • retainedSections and keptSections reporting: restore reported "keptSections": ["## Environment", "## install"] for such a block.
    • Pointer conversion is protected by refused-bloated.
    • Fix: track ``` and ~~~ fences in sectionsOf, and treat only lines matching /^ {0,3}## / as H2. Add tests for all three shapes.
    • Main deletes the whole section, so this is not a regression. But it is a new promise ("kept") that the code does not keep, and the fix is small.

Rulings on the deviations

  • (a) Accept. Copying the existing block verbatim, note line and blank lines included, is what makes C1 / INV-P5-5 hold (an edit inside the block, or removing the note, stays fresh, as tested). The note is re-emitted only when the block is rebuilt.
  • (b) Accept. Merging sections from outside the block into it, rather than "first source wins", loses nothing and is deterministic. Verified: a retained section after the block is merged, and the next run is idempotent.
  • (c) Accept, subject to item 1. Stopping at the module-index fence is correct. The "already rendered" dedupe cannot drop a user's only copy, because the text is by construction present in the output. The risk is the reverse (item 1). Inside a block nothing is deduped, so a user who wants a duplicate kept can keep it there.
  • (d) Accept. A stray end marker falls back to legacy; the section is kept and the marker is dropped (tested).
  • (e) Accept. Collision matching ignores the weight suffix: ## CONTEXT next to ## CONTEXT _(load-bearing)_ is the same visible collision.
  • (f) Accept. retained-unfenced wins over content-stale. The remedy (--write) fixes both causes, and the more specific reason is the more useful one.
  • (g) Accept: no working flow breaks.
    • Unchanged: every pristine pointer template (CLAUDE.md ×2, AGENTS.md, .cursorrules, GEMINI.md), LF and CRLF, under --mode compiled, gives the same bytes and exit code as main. So does bootstrap --yes followed by compile --target all --write --mode compiled, and so does a pristine recorded-compiled pointer under plain --write.
    • Newly refused (exit 1; main wrote with exit 0): only a pointer-looking file holding bloat or stray lines, either under --mode compiled or recorded compiled under plain --write. On main those were the silent-loss cases.
    • No other command calls the compile write path (only adf.ts dispatches it).
    • Doc nit: cli-reference.md:567 says "Converting a pointer refuses". It should also cover a recorded-compiled file that is still a pointer under plain --write.
  • (h) Accept. retained-divergent disappearing from RestoreRefusal is a value that is no longer emitted, and the CHANGELOG says so under Changed. It is not re-exported from packages/cli/src/index.ts.

Verified OK

  • Idempotency: --write twice is byte-identical in every non-failing scenario across all targets. CRLF input produces LF output.
  • Block edits:
    • An edit inside the block survives, and so does a non-retained heading inside it.
    • Deleting a section inside the block, or deleting the whole block, sticks.
    • A start marker with no end runs to EOF and the end is re-added.
    • A block moved into the middle of the file is moved to the end.
    • A marker line with trailing whitespace is handled.
  • --check:
    • JSON rows equal main's for pristine, notes, bloated, unmanaged and absent CLAUDE.md, apart from the added staleReason key. The PR2 pointer rules are unchanged.
    • A legacy section reports retained-unfenced, and so does an unfenced section together with a .ai/ change. --write then makes the file fresh.
  • Restore:
    • A block or legacy section is carried into the pointer, keptSections is correct, and compiled-differs is false when only the block differs.
    • Doctor's adf vendor bloat passes on the restored pointer.
    • The pointer → compiled round trip loses nothing.
  • One shared function (K1): --write (adf-compile.ts:420/462), --check (:622) and restore (compiledMatchesFresh, composeCompiled) all go through composeCompiled, with no duplicated logic. score.ts is untouched (K4).
  • INV-P5-10 (circular imports): adf-retained.js touches its imports only inside functions (dist lines 48, 158, 166, 192). Loading the module first, or adf-compile, adf-vendor, adf-tidy, adf, adf-migrate or index, all compose correctly.
  • Docs: the CHANGELOG puts CP-1/2/7 under Changed and #303 under Fixed (Unreleased). Both READMEs and cli-reference (retained block, --check, statuses, restore) are accurate apart from the (g) nit.
  • This repo: adf compile --target all --check exits 0 on the branch build (4 × pointer-ok).
  • Tests: the CLI suite gives 688 passed and 3 failed. The 3 failures are Windows path-separator tests (bootstrap #307, context-refresh, serve-context) and fail identically on main.

Non-blocking notes

  1. Restore carries a non-retained heading the user put inside the block (for example ## My Notes) into the pointer as a plain H2. The pointer is then tidy-bloat: tidy --dry-run extracts it to core.adf, and the reverse --mode compiled reports refused-bloated. Nothing is lost, but consider listing such headings in the restore output or the docs.

  2. Two cases are dropped silently, as on main:

    • a non-retained section after the block;
    • a second charter:retained block's non-retained content.

    Consider merging every block.

  3. stripCharterSentinels accepts <!--\s*charter:retained:start\s*-->, but composeCompiled needs the exact trimmed text. A user who retypes it as <!--charter:retained:start--> falls back to legacy, so non-retained headings inside are dropped. Consider sharing the regex.

  4. An end marker inside a fenced code block in the block ends the block early, so the lines after it are dropped. This matches stripCharterSentinels semantics, but it is worth one doc sentence.

  5. User edits to ## Session Start, ## Module Index or the preamble of a compiled file are still dropped, as on main and by design (contract §1). The messaging is fine.

kovermier and others added 2 commits October 16, 2026 15:42
…refs #303)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…refs #303)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@stackbilt-admin
stackbilt-admin merged commit a50de35 into main Oct 16, 2026
5 checks passed
@stackbilt-admin
stackbilt-admin deleted the auto/feature/issue-303-retained branch October 16, 2026 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

adf compile: preserve retained sections instead of only refusing on them

2 participants