Skip to content

fix(archive): preserve blank lines inside code fences - #1798

Open
dwin-gharibi wants to merge 2 commits into
Fission-AI:mainfrom
dwin-gharibi:fix-archive-fence-preservation
Open

fix(archive): preserve blank lines inside code fences#1798
dwin-gharibi wants to merge 2 commits into
Fission-AI:mainfrom
dwin-gharibi:fix-archive-fence-preservation

Conversation

@dwin-gharibi

@dwin-gharibi dwin-gharibi commented Sep 6, 2026

Copy link
Copy Markdown

Closes #1797.

Why

The final assembly in buildUpdatedSpec normalises blank lines across the whole
rebuilt document:

.join('\n\n')
.replace(/\n{3,}/g, '\n\n')   // whole-document, fence-unaware

That normalisation exists to tidy the seams between the four slices being
rejoined, but it also rewrites the inside of fenced code blocks. A requirement
documenting a sample with two or more consecutive blank lines has that sample
silently edited on archive, and re-edited on every subsequent archive.

It matters for whitespace-significant content — YAML block scalars, Python,
expected-output fixtures, Markdown-in-Markdown examples. Every other structural
pass in this module is already fence-aware via buildCodeFenceMask; this one
was not.

What Changes

  • Replaced the whole-document regex with collapseBlankRunsOutsideFences, which
    walks the assembled document with buildCodeFenceMask and collapses blank
    runs only outside fenced blocks.
  • Semantics outside fences are byte-identical to before: only a truly empty
    line counts as blank, exactly as /\n{3,}/ did, so a line of spaces is still
    never a collapse boundary.

Testing

test/core/specs-apply.fence-preservation.test.ts — 8 tests, written first and
watched fail (5 failed / 3 passed before, 8 passed after).

Edge cases covered:

  • two blank lines, and a longer run, inside a backtick fence
  • a tilde (~~~) fence
  • indentation-sensitive Python content inside a fence
  • blank runs outside fences still collapse (no \n{3,} survives)
  • a spec with no fences is byte-identical to the previous output (asserted
    against the full expected string, not a substring)
  • a whitespace-only line is still not collapsed, matching the old regex exactly
  • fenced blank lines carried in from the existing main spec (not just the
    delta) are preserved

Full suite green on this branch.

Changeset

Not added. Per .changeset/README.md the default path is the normal release
cadence; happy to run pnpm changeset if a maintainer wants dedicated release
notes.

Summary by CodeRabbit

  • Bug Fixes

    • Preserved multiple blank lines inside fenced code samples during specification updates.
    • Continued collapsing excessive blank lines outside fenced sections.
    • Maintained existing behavior for specifications without fenced code.
    • Preserved indentation-sensitive and whitespace-significant content within fenced samples.
  • Tests

    • Added coverage for backtick- and tilde-fenced samples, indentation-sensitive content, whitespace-only lines, and existing specification content.

Copilot AI lite review requested due to automatic review settings September 6, 2026 08:15
@dwin-gharibi
dwin-gharibi requested a review from a team as a code owner September 6, 2026 08:15
@dwin-gharibi
dwin-gharibi requested review from TabishB and removed request for a team September 6, 2026 08:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 2f452435-a839-448f-a8ea-86674e404516

📥 Commits

Reviewing files that changed from the base of the PR and between 3de67b3 and 36f135c.

📒 Files selected for processing (1)
  • test/core/specs-apply.fence-preservation.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/core/specs-apply.fence-preservation.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The spec assembly now collapses blank-line runs only outside fenced code blocks. The change adds fence-aware normalization and tests for backtick fences, tilde fences, indentation-sensitive content, whitespace-only lines, and existing spec content.

Changes

Spec normalization

Layer / File(s) Summary
Fence-aware normalization
src/core/specs-apply.ts
buildUpdatedSpec now uses collapseBlankRunsOutsideFences. The helper collapses empty-line runs outside fences and preserves content inside backtick and tilde fences.
Normalization coverage
test/core/specs-apply.fence-preservation.test.ts
Tests verify fenced blank-line preservation, outside-fence collapsing, no-fence compatibility, whitespace-only lines, indentation-sensitive content, and fenced content from the existing main spec.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 36f13

The change preserves blank lines in fenced code blocks while retaining existing normalization outside fences, with no current merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly states the primary change: preserving blank lines inside code fences during archive processing.
Linked Issues check ✅ Passed The changes address issue #1797. buildUpdatedSpec now preserves consecutive blank lines inside backtick and tilde fences while retaining blank-line normalization outside fences. Tests cover the requ…
Out of Scope Changes check ✅ Passed The changes are limited to fence-aware normalization in src/core/specs-apply.ts and focused regression tests. No unrelated changes are present.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

archive rewrites the inside of fenced code blocks

2 participants