Collapse fence marker strategies (#360)

[lodyai]

Summary

This branch replaces the duplicate fence rewrite enums with one private
Strategy enum, so every fence-marker rewrite uses a single, consistent
binary choice.

Closes #360.

It adds generated strategy invariants, representative normalization snapshots,
and developer guidance. The Kani harness is intentionally omitted because the
project has neither a Kani dev-dependency nor harness infrastructure; the
finite strategy states are covered across bounded marker lengths by proptest.

Review walkthrough

• Start with src/fences.rs for the canonical dispatch and the retained matched-block preserve path.
• Then review src/fences_properties.rs for generated compression, preservation, and idempotency invariants.
• Finish with tests/fences.rs, tests/snapshots/fences_compress_overlong_backticks.snap, and docs/developers-guide.md for locked output and the module contract.

Validation

• make check-fmt: passed
• make typecheck: passed
• make lint: passed
• make test: passed
• make markdownlint: passed
• make nixie: passed
• mbake validate Makefile: passed
• make all: passed
• coderabbit review --agent: zero findings

References

• Issue: refactor(fences): collapse twin FenceRewrite/MarkerStrategy enums into one Strategy #360
• Lody session: https://lody.ai/leynos/sessions/72f261be-58f7-4da1-b720-b4b4f08da421

[sourcery-ai]

Sorry @LodyAI[bot], you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8e31e1cd-c019-4a95-abcf-de7833b6bb54

📥 Commits

Reviewing files that changed from the base of the PR and between 527b482 and 750a1a3.

⛔ Files ignored due to path filters (3)

• tests/snapshots/fences_compress_overlong_backticks.snap is excluded by !**/*.snap
• tests/snapshots/fences_compress_tilde_fences.snap is excluded by !**/*.snap
• tests/snapshots/fences_preserve_interior_conflict.snap is excluded by !**/*.snap

📒 Files selected for processing (4)

• docs/developers-guide.md
• src/fences.rs
• src/fences_properties.rs
• tests/fences.rs

---

Summary

• Consolidated fence-marker rewriting behind a private Strategy enum with consistent Compress and Preserve behaviour.
• Added developer documentation defining strategy dispatch and matched-block handling.
• Added property-based invariant tests, normalization snapshots, and coverage for preserving null language during interior conflicts.
• Updated caching and fence-line emission while retaining matched-block preservation.
• Formatting, type checking, linting, tests, documentation validation, full build, and review checks passed.

Walkthrough

Fence normalisation now routes delimiter rewriting through a shared Strategy enum, updates fence-processing paths to use it, documents its semantics, and adds property-based and snapshot coverage.

Changes

Fence normalisation

Layer / File(s)	Summary
Unify marker rewrite strategies src/fences.rs	Replace specialised rewrite helpers with Strategy::Compress and Strategy::Preserve through rewrite_marker, including cached compressed output.
Route fence normalisation through Strategy src/fences.rs, docs/developers-guide.md	Apply strategy-based dispatch when selecting and emitting rewrites for matched fence blocks, and document the dispatch rules.
Document and validate rewriting behaviour src/fences_properties.rs, tests/fences.rs	Test compression, preservation, idempotency, null-language handling, and snapshot normalisation cases.

Sequence Diagram(s)

sequenceDiagram
  participant ParsedLine
  participant rewrite_fence_line
  participant rewrite_marker
  ParsedLine->>rewrite_marker: Cache Compress rewrite
  rewrite_fence_line->>rewrite_marker: Compute Preserve rewrite
  rewrite_marker-->>rewrite_fence_line: Return rewritten fence line

Loading

Possibly related PRs

• leynos/mdtablefix#396: Refactors overlapping fence-marker compression and preservation paths in src/fences.rs.

Suggested labels: Issue

Suggested reviewers: leynos

Poem

Backticks march in ordered rows,
Tildes yield where structure grows.
Compress, preserve, lines align,
Tests guard every fence design.

🚥 Pre-merge checks | ✅ 20

✅ Passed checks (20 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title matches the main change and includes the linked issue reference (#360).
Description check	✅ Passed	The description clearly describes the fence-marker strategy consolidation and related tests and docs.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Testing (Overall)	✅ Passed	PASS: keep the new property tests and snapshots; they exercise Compress/Preserve dispatch, null-language handling, matched and unmatched fences, and idempotency.
User-Facing Documentation	✅ Passed	PASS: docs/users-guide.md already documents --fences compression, preservation and orphan-specifier attachment, and this PR only refactors internals.
Developer Documentation	✅ Passed	PASS: docs/developers-guide.md documents the new private Strategy enum and rewrite dispatch, and no locale or roadmap gaps were found.
Module-Level Documentation	✅ Passed	All touched modules carry top-level docs describing purpose and relationships, including the new test-only companion module.
Testing (Unit And Behavioural)	✅ Passed	Unit proptests cover Strategy invariants and edge cases; integration tests hit the public compress_fences/attach_orphan_specifiers boundary with snapshots.
Testing (Property / Proof)	✅ Passed	PASS: src/fences_properties.rs adds proptest invariants for compress/preserve/idempotency, and tests/fences.rs adds snapshots for fence normalisation cases.
Testing (Compile-Time / Ui)	✅ Passed	Pass: no new compile-time behaviour appears; the PR adds focused insta snapshots and proptest invariants, and trybuild coverage already exists in tests/compile.rs.
Unit Architecture	✅ Passed	The fence rewrite path keeps parsing fallible via Option, isolates pure rewrite logic from block-flushing commands, and adds explicit test-only property checks without hidden side effects.
Domain Architecture	✅ Passed	Touched files stay in pure Markdown-processing code, tests, and docs; no HTTP, SQL, filesystem, CLI, or other infrastructure concerns leaked into domain logic.
Observability	✅ Passed	Pure fence-normalisation refactor plus tests/docs; no new operational boundaries, logging, metrics, tracing, or alerts were required.
Security And Privacy	✅ Passed	The PR only rewrites Markdown fence lines; no secrets, env vars, auth/permission changes, or unsafe sinks appear in the touched code, tests, snapshots, or docs.
Performance And Resource Use	✅ Passed	Only a comment in src/fences.rs changed; no loops, allocations, caches, or I/O paths were altered, so no performance regression is introduced.
Concurrency And State	✅ Passed	State stays local: only immutable LazyLock regexes and per-call buffers/trackers; no async, locks, or shared mutable state were added.
Architectural Complexity And Maintainability	✅ Passed	Private Strategy replaces duplicate fence-rewrite plumbing and stays local, with explicit docs and bounded tests rather than a speculative framework.
Rust Compiler Lint Integrity	✅ Passed	No new dead_code/unused suppressions appear in the touched files; the new property module is cfg(test)-gated, and the remaining clones are ordinary owned output, not lint appeasement.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue-360-refactor-fences-collapse-twin-fencerewrite-markerstrategy-enums-into-one-strategy

---

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

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/developers-guide.md`:
- Around line 610-612: Update the rewrite_fence_line documentation to state that
flush_matched_block performs matched-block selection, while rewrite_fence_line
only dispatches on Strategy and falls back to the original line.

In `@tests/fences.rs`:
- Around line 18-33: Add committed Insta snapshot fixtures under
tests/snapshots/ for the three cases exercised by fence_normalization_snapshots:
fences_compress_overlong_backticks, fences_compress_tilde_fences, and
fences_preserve_interior_conflict. Use the accepted generated snapshot contents
so the tests pass without leaving .snap.new files.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: af5dad99-f083-4b18-b092-0e673964fbc3

📥 Commits

Reviewing files that changed from the base of the PR and between 9bf67eb and 527b482.

⛔ Files ignored due to path filters (3)

• tests/snapshots/fences_compress_overlong_backticks.snap is excluded by !**/*.snap
• tests/snapshots/fences_compress_tilde_fences.snap is excluded by !**/*.snap
• tests/snapshots/fences_preserve_interior_conflict.snap is excluded by !**/*.snap

📒 Files selected for processing (4)

• docs/developers-guide.md
• src/fences.rs
• src/fences_properties.rs
• tests/fences.rs

[buzzybee-df12]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/fences_properties.rs`:
- Around line 18-19: Update the property-test input generator around the
language value strategy to include null and case-variant null values in addition
to existing strings. Extend the assertions for both rewrite strategies to verify
that each omits the language suffix when the language is null, ensuring
rewrite_marker’s absent-language branch is exercised.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 528db9d8-5a78-4e69-b293-4d17aaa8397a

📥 Commits

Reviewing files that changed from the base of the PR and between 9bf67eb and 527b482.

⛔ Files ignored due to path filters (3)

• tests/snapshots/fences_compress_overlong_backticks.snap is excluded by !**/*.snap
• tests/snapshots/fences_compress_tilde_fences.snap is excluded by !**/*.snap
• tests/snapshots/fences_preserve_interior_conflict.snap is excluded by !**/*.snap

📒 Files selected for processing (4)

• docs/developers-guide.md
• src/fences.rs
• src/fences_properties.rs
• tests/fences.rs

[pandalump]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[leynos]

@coderabbitai Have the following now been resolved?

src/fences.rs (1)

92-94: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Limit the cache claim to compression.
Line 92 states that every flush path avoids another normalization regex run, but

Strategy::Preserve calls rewrite_marker at Line 158. State that caching

avoids repeated work only for compressed output and unmatched-block handling.

🤖 Detailed instructions

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @src/fences.rs around lines 92 - 94, Update the documentation comment above
the cached compressed value in the relevant fences flow to limit its claim to
avoiding repeated compression work and supporting unmatched-block handling;
remove the broader assertion that every flush path avoids rerunning
normalization, since Strategy::Preserve still invokes rewrite_marker.

[leynos]

@coderabbitai resume

[coderabbitai]

✅ Action performed

Reviews resumed.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

ast-grep outline src/fences.rs --items all --view expanded
printf '\n--- src/fences.rs Lines 75–180 ---\n'
sed -n '75,180p' src/fences.rs
printf '\n--- PR diff for src/fences.rs ---\n'
git diff "$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main)"...HEAD -- src/fences.rs 2>/dev/null || true

Length of output: 10781

---

@leynos Treat this finding as resolved.

CachedLine now states that compressed “avoids repeated compression work” and supports flush_unmatched_block; it no longer claims that every flush path avoids rerunning normalization. The rewrite_fence_line documentation also explicitly says that Strategy::Preserve computes its rewrite on demand, matching the rewrite_marker(&line, Strategy::Preserve) call.

[buzzybee-df12]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.