Skip to content

Share canonical frontmatter boundary (#361) - #407

Merged
leynos merged 6 commits into
mainfrom
issue-361-refactor-process-share-frontmatter-split-rejoin-boundary-via-a-body-closure
Jul 24, 2026
Merged

Share canonical frontmatter boundary (#361)#407
leynos merged 6 commits into
mainfrom
issue-361-refactor-process-share-frontmatter-split-rejoin-boundary-via-a-body-closure

Conversation

@lodyai

@lodyai lodyai Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary

The branch makes process_with_frontmatter the canonical leading-YAML
split/rejoin boundary, preventing the library and CLI pipelines from diverging.
It closes issue #361.

Closes #361.

Review walkthrough

Validation

  • make check-fmt: passed.
  • make typecheck: passed.
  • make lint: passed with warnings denied.
  • make test: passed.
  • make markdownlint: passed.
  • make nixie: passed.
  • mbake validate Makefile: passed.
  • make: passed.
  • coderabbit review --agent: requested after both implementation milestones; no findings were emitted.

Notes

Kani is intentionally not used: the contract operates over unbounded Markdown
line input, so property testing is the appropriate adversary rather than a
bounded model-checking harness. A deliberate mutation that omitted the restored
frontmatter prefix was rejected by the preservation property.

References

leynos added 3 commits July 22, 2026 17:01
Route both the library and CLI pipelines through
`process_with_frontmatter` so only one function owns the leading YAML
frontmatter split and verbatim rejoin.
Prove that the shared helper preserves valid YAML frontmatter, confines
closures to the Markdown body, and handles documents without frontmatter.
Pin the library and CLI formatting paths with end-to-end snapshots.
Define `process_with_frontmatter` as the sole owner of leading YAML
frontmatter preservation and require every body transform to execute
inside its closure.

@sourcery-ai sourcery-ai Bot 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.

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

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

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: 13191ca0-1b0e-413f-9c3a-e1e0033bd5b6

📥 Commits

Reviewing files that changed from the base of the PR and between 694bc89 and faa7ab3.

📒 Files selected for processing (3)
  • docs/architecture.md
  • docs/users-guide.md
  • src/process.rs

Summary

  • Made process_with_frontmatter the canonical split/rejoin boundary for leading YAML frontmatter.
  • Routed both library and CLI processing through the closure-based helper, preserving frontmatter verbatim while applying transforms only to the Markdown body.
  • Added property tests and library/CLI snapshot coverage for documents with and without frontmatter.
  • Updated user, developer, and architecture documentation to describe the API contract and behaviour.

Validation

  • Full make gate passed.
  • CodeRabbit reported no findings.

Walkthrough

Changes

Frontmatter processing

Layer / File(s) Summary
Define the shared frontmatter boundary
src/process.rs, docs/developers-guide.md, docs/architecture.md, docs/users-guide.md
Route body transformations through process_with_frontmatter, preserve leading YAML verbatim, and document the contract and public behaviour.
Route library and CLI processing
src/process.rs, src/main.rs
Delegate library and CLI processing through the shared boundary, including conditional list renumbering and break formatting.
Validate library and CLI behaviour
tests/process_frontmatter_props.rs, tests/process_frontmatter_snapshots.rs, tests/cli_frontmatter_snapshots.rs
Verify frontmatter preservation, closure input scope, and snapshot outputs with and without frontmatter.

Suggested labels: Issue

Suggested reviewers: leynos

Poem

YAML stays snug at the top of the page,
While body transforms dance on the stage.
One boundary guards every line,
Tests keep the contract in time.
Tables and lists now flow bright—
Frontmatter returns just right.

🚥 Pre-merge checks | ✅ 19 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Developer Documentation ⚠️ Warning Developer and architecture docs are updated, but docs/execplans/yaml-frontmatter.md still describes the old split-helper design, not the new closure boundary. Update the living execplan to describe process_with_frontmatter’s closure-based boundary and current progress, or retire it if it is no longer the active plan.
✅ Passed checks (19 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the frontmatter-boundary refactor and includes the linked issue reference.
Description check ✅ Passed The description clearly describes the frontmatter boundary refactor and its verification work.
Linked Issues check ✅ Passed The changes satisfy #361 by centralising the boundary, updating both callers, adding property and snapshot tests, and documenting the contract.
Out of Scope Changes check ✅ Passed The changes stay focused on frontmatter handling, tests, and documentation with no evident unrelated additions.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Testing (Overall) ✅ Passed Property tests cover prefix/body invariants, and end-to-end library/CLI snapshots exercise frontmatter-preserving transforms; these catch plausible regressions.
User-Facing Documentation ✅ Passed Document the new frontmatter-preservation behaviour in docs/users-guide.md; it covers CLI, library callers, and process_with_frontmatter, and no extra locale is present.
Module-Level Documentation ✅ Passed PASS: a repo-wide scan found 0 Rust modules missing top-level //! docs, and the new frontmatter modules clearly describe their role and boundaries.
Testing (Unit And Behavioural) ✅ Passed Property tests verify frontmatter invariants, and snapshots exercise the public library boundary plus the real CLI stdin/stdout path.
Testing (Property / Proof) ✅ Passed The PR adds substantive proptest coverage for the frontmatter invariant and body-only closure contract; no separate proof obligation is introduced.
Testing (Compile-Time / Ui) ✅ Passed Focused proptest and Insta snapshots cover the runtime frontmatter contract; no new compile-time UI boundary is introduced here, and trybuild coverage already exists.
Unit Architecture ✅ Passed PASS: process_with_frontmatter is the explicit seam, library paths stay pure and side-effect-free, and CLI I/O remains confined to main.rs/handle_file.
Domain Architecture ✅ Passed PASS: Core processing stays pure, CLI and file I/O remain in main/io, and the new frontmatter boundary keeps transport concerns out of domain code.
Observability ✅ Passed PASS: The change is a pure in-process frontmatter-routing refactor; it adds no new service/queue boundary or operational failure mode, and existing file-I/O errors still surface on stderr.
Security And Privacy ✅ Passed Only plain-text frontmatter routing, docs, and synthetic snapshots changed; I found no secrets, auth changes, injection sinks, or sensitive-data exposure.
Performance And Resource Use ✅ Passed PASS: The refactor stays linear, with one frontmatter scan and one owned prefix copy, and adds no new unbounded buffers or hot-path I/O; tests are bounded.
Concurrency And State ✅ Passed No shared mutable state, locks, async tasks, or ordering-sensitive concurrency were introduced; the new boundary is purely synchronous and local.
Architectural Complexity And Maintainability ✅ Passed Use one explicit closure boundary; remove duplicated frontmatter orchestration and keep the helper narrow, documented, and immediately reused by library and CLI paths.
Rust Compiler Lint Integrity ✅ Passed No dead_code/unused suppressions were added; only narrow clippy::struct_excessive_bools expects remain, and clone use is limited to required test setup and verbatim prefix copying.
📋 Issue Planner

Built with CodeRabbit's Coding Plans for faster development and fewer bugs.

View plan used: #361

✨ 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-361-refactor-process-share-frontmatter-split-rejoin-boundary-via-a-body-closure

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

@leynos
leynos marked this pull request as ready for review July 23, 2026 12:54

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 694bc89698

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/process.rs
@coderabbitai coderabbitai Bot added the Issue label Jul 23, 2026
@pandalump

Copy link
Copy Markdown
Collaborator

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor
✅ 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 added 2 commits July 23, 2026 22:05
Add a doctested public API example and align user, architecture, and
frontmatter planning documentation with the canonical closure boundary.
State the delivered public boundary and retained internal detector without
leaving the historical revision note ambiguous.
@leynos

leynos commented Jul 24, 2026

Copy link
Copy Markdown
Owner

@coderabbitai no need to retcon historical execplans. The execplans should reflect the state of the architecture at the time they were implemented.

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Tip

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

@leynos Acknowledge this guidance. Preserve historical execplans as contemporaneous architectural records; do not retcon them to match later architecture.


✏️ Learnings added
Learnt from: leynos
URL: https://github.com/leynos/mdtablefix/pull/407

Timestamp: 2026-07-24T00:42:50.404Z
Learning: In the mdtablefix repository, treat historical execplans as records of the architecture at the time of implementation. Do not update them retroactively to reflect later architectural changes.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Remove the follow-up implementation notes so the completed ExecPlan retains
its original historical record.
@leynos

leynos commented Jul 24, 2026

Copy link
Copy Markdown
Owner

@coderabbitai Have the following failed checks now been resolved?

If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures.

Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. (Treat documentation and validation coverage as in scope).

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
User-Facing Documentation ⚠️ Warning docs/users-guide.md never mentions YAML frontmatter; the only frontmatter docs are in README and the developers guide. Add a user-guide section for YAML frontmatter preservation and the CLI/library behaviour, or explicitly mark the change as internal-only.
Developer Documentation ⚠️ Warning The guide was updated, but docs/architecture.md still describes the old split helper, so the design docs are stale. Update the architecture guide to name process_with_frontmatter as the canonical boundary and align the progress/decision notes with the closure-based API.

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Changed files ==='
git diff --name-status "$(git merge-base HEAD origin/main)" HEAD 2>/dev/null || git diff --name-status HEAD~1 HEAD

echo
echo '=== Documentation frontmatter references ==='
rg -n -i -C 4 'yaml frontmatter|frontmatter|process_with_frontmatter|split_frontmatter|split.*frontmatter' \
  README.md docs 2>/dev/null || true

echo
echo '=== Documentation outlines ==='
for file in docs/users-guide.md docs/architecture.md docs/developers-guide.md; do
  if [ -f "$file" ]; then
    echo "--- $file ---"
    rg -n '^#{1,6} ' "$file" || true
  else
    echo "Missing: $file"
  fi
done

echo
echo '=== Relevant documentation diff ==='
git diff "$(git merge-base HEAD origin/main)" HEAD -- \
  docs/users-guide.md docs/architecture.md docs/developers-guide.md 2>/dev/null \
  || git diff HEAD~1 HEAD -- docs/users-guide.md docs/architecture.md docs/developers-guide.md

Length of output: 41908


@leynos Mark both warnings resolved.

  • User-Facing Documentation: Resolved. docs/users-guide.md now documents YAML frontmatter preservation, valid leading delimiters, body-only formatting, CLI option behaviour—including --breaks—and equivalent library stream behaviour.
  • Developer Documentation: Resolved. docs/architecture.md now names process_with_frontmatter as the canonical split/rejoin boundary, specifies the body-transform closure contract, and records that library and CLI-only transforms route through it. It correctly retains frontmatter::split_leading_yaml_frontmatter as the internal detector rather than describing it as the pipeline boundary.

Do not create a follow-up issue or perform further documentation work for these two checks.

@buzzybee-df12

Copy link
Copy Markdown

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor
✅ 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
leynos merged commit 34e3b6e into main Jul 24, 2026
4 checks passed
@leynos
leynos deleted the issue-361-refactor-process-share-frontmatter-split-rejoin-boundary-via-a-body-closure branch July 24, 2026 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(process): share frontmatter split/rejoin boundary via a body closure

3 participants