Conversation
Documents that begin with a YAML frontmatter block (starting with --- and ending with --- or ...) now have that block preserved exactly while the remainder of the document is formatted normally. Changes: - Add src/frontmatter.rs with split_leading_yaml_frontmatter() helper - Wire frontmatter detection through library pipeline in src/process.rs - Wire frontmatter protection through CLI transforms in src/main.rs - Add comprehensive unit and behavioural CLI tests - Update README.md with YAML frontmatter documentation - Update docs/architecture.md with pipeline description The frontmatter block is shielded from all transforms including --breaks, --wrap, --renumber, and table reflow. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
YAML Frontmatter Preservation SupportOverviewPreserves a leading YAML frontmatter block (opener line exactly Core changes
Tests
Documentation & design artefact
Tooling, refactors and misc
Review feedback and outstanding action
Behavioural contract
WalkthroughIntroduce YAML frontmatter preservation: add a frontmatter module that detects and splits a leading YAML block from input lines. Process only the remaining body through existing transforms and reconstruct output by prepending the preserved frontmatter prefix unchanged. Add tests and update documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as mdtablefix CLI
participant Split as split_leading_yaml_frontmatter
participant Pipeline as process_stream_inner
participant Reconstruct as Prepend Frontmatter
CLI->>Split: process_lines(all_lines)
Split-->>Split: check first line == "---"
Split-->>Split: scan for closer ("---" or "...")
Split-->>CLI: return (frontmatter_prefix, body_lines)
CLI->>Pipeline: process_stream_inner(body_lines)
Pipeline->>Pipeline: apply transforms (tables, headings, renumber, breaks, wrap)
Pipeline-->>CLI: processed_body
CLI->>Reconstruct: prepend frontmatter_prefix to processed_body
Reconstruct-->>CLI: final_output
Poem
🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Reviewer's GuideImplements YAML frontmatter preservation by introducing a shared splitter, integrating it into both the library and CLI pipelines so only the document body is transformed, and adds tests and documentation describing the behavior along with some minor refactors and formatting tweaks across the codebase and docs. Sequence diagram for YAML frontmatter preservation in CLI pipelinesequenceDiagram
actor User
participant CLI as CLI_main
participant FM as Frontmatter_splitter
participant Proc as Process_stream_opts
participant List as Renumber_lists
participant Breaks as Format_breaks
participant IO as File_IO
User->>CLI: invoke mdtablefix with flags
CLI->>FM: split_leading_yaml_frontmatter(lines)
FM-->>CLI: frontmatter_prefix, body
CLI->>Proc: process_stream_opts(body, options)
Proc-->>CLI: processed_body
alt renumber flag enabled
CLI->>List: renumber_lists(processed_body)
List-->>CLI: renumbered_body
else renumber flag disabled
CLI-->>CLI: renumbered_body = processed_body
end
alt breaks flag enabled
CLI->>Breaks: format_breaks(renumbered_body)
Breaks-->>CLI: broken_body
else breaks flag disabled
CLI-->>CLI: broken_body = renumbered_body
end
CLI-->>CLI: result = frontmatter_prefix + broken_body
CLI->>IO: write result to stdout or file
IO-->>User: formatted document with preserved frontmatter
Sequence diagram for YAML frontmatter handling in library process_stream_innersequenceDiagram
participant Lib as Library_client
participant Proc as Process_stream_inner
participant FM as Frontmatter_splitter
participant Fences as Fence_normalization
participant Html as Html_table_converter
participant Wrap as Wrapper_and_tables
participant Head as Heading_converter
participant Ell as Ellipsis_replacer
participant Foot as Footnote_converter
Lib->>Proc: process_stream_inner(lines, options)
Proc->>FM: split_leading_yaml_frontmatter(lines)
FM-->>Proc: frontmatter_prefix, body
alt fences option enabled
Proc->>Fences: compress_fences(body)
Fences-->>Proc: fenced_body
Proc->>Fences: attach_orphan_specifiers(fenced_body)
Fences-->>Proc: normalized_body
else fences option disabled
Proc-->>Proc: normalized_body = body
end
Proc->>Html: convert_html_tables(normalized_body)
Html-->>Proc: preprocessed_lines
Proc->>Wrap: classify_block and wrap_text(preprocessed_lines)
Wrap-->>Proc: wrapped_lines
Proc->>Head: convert_setext_headings(wrapped_lines)
Head-->>Proc: heading_lines
Proc->>Ell: replace_ellipsis(heading_lines)
Ell-->>Proc: ellipsis_lines
alt footnotes option enabled
Proc->>Foot: convert_footnotes(ellipsis_lines)
Foot-->>Proc: body_out
else footnotes option disabled
Proc-->>Proc: body_out = ellipsis_lines
end
Proc-->>Lib: frontmatter_prefix + body_out
Class diagram for frontmatter splitter and processing pipeline integrationclassDiagram
class frontmatter {
+split_leading_yaml_frontmatter(lines : &[String]) (&[String], &[String])
}
class process_stream_inner_fn {
+process_stream_inner(lines : &[String], opts : Options) Vec~String~
+process_stream(lines : &[String]) Vec~String~
}
class cli_process_lines_fn {
+process_lines(lines : &[String], opts : FormatOpts) Vec~String~
}
class io_module {
+rewrite(path : &Path) Result~()~
+rewrite_no_wrap(path : &Path) Result~()~
}
class wrap_module {
+wrap_text(lines : &[String], width : usize) Vec~String~
}
class html_module {
+convert_html_tables(lines : &[String]) Vec~String~
}
class footnotes_module {
+convert_footnotes(lines : &[String]) Vec~String~
}
class lists_module {
+renumber_lists(lines : &[String]) Vec~String~
}
class main_Options {
+From~FormatOpts~ for Options
}
frontmatter <.. process_stream_inner_fn : uses
frontmatter <.. cli_process_lines_fn : uses
process_stream_inner_fn ..> wrap_module : uses
process_stream_inner_fn ..> html_module : uses
process_stream_inner_fn ..> footnotes_module : uses
cli_process_lines_fn ..> lists_module : uses
io_module ..> process_stream_inner_fn : uses
main_Options <|.. Options
main_Options <|.. FormatOpts
Flow diagram for YAML frontmatter splitting and recombination in processing pipelineflowchart TD
A[Input_lines_from_file_or_stdin] --> B{First_line_is_exactly_---?}
B -- No --> C[No_frontmatter_prefix
returned]
B -- Yes --> D[Search_for_closer_line
---_or_...]
D --> E{Closer_found_before_body?}
E -- No --> C
E -- Yes --> F[Split_into_frontmatter_prefix
and_body_slice]
C --> G[Process_body_with_existing_pipeline]
F --> G
G --> H{CLI_only_transforms_enabled?}
H -- No --> I[Processed_body]
H -- Yes --> J[Apply_renumber_lists_and_format_breaks]
J --> I
I --> K[Prepend_frontmatter_prefix]
K --> L[Write_output_document]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
The rustfmt configuration uses unstable features that require the nightly toolchain. Updated the Makefile to use 'cargo +nightly fmt' for both the fmt and check-fmt targets. Also applied nightly formatting to the entire codebase. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
- Change toolchain channel from 1.89.0 to nightly-2026-03-26 - Add rust-analyzer to components list - Fix implicit_clone lint warnings by using clone() instead of to_string() Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
- Fix MD013 (line length) in yaml-frontmatter.md by wrapping long line - Fix MD029 (ordered list prefix) in rust-doctest-dry-guide.md and rust-testing-with-rstest-fixtures.md by using sequential numbering - Fix MD040 (fenced code language) by adding 'text' language to code blocks in footnotes Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96e5722f73
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/documentation-style-guide.md`:
- Line 166: The footnote definition is escaped as `\[^1\]:`, which prevents
Markdown from recognizing it and leaves the `[^1]` reference unresolved; fix it
by removing the backslash so the definition reads `[^1]:` (ensure the inline
reference `[^1]` elsewhere matches this identifier).
In `@docs/execplans/yaml-frontmatter.md`:
- Line 8: Update the ExecPlan frontmatter by changing the "Status: DRAFT" value
to the delivered status string used by the shipped implementation and update any
interface descriptions referenced in the document (the ExecPlan interface note)
to match the current public API/visibility; specifically, synchronize the
frontmatter status token "Status: DRAFT", the section describing the ExecPlan
interfaces, and the noted interface visibility so they reflect the completed
implementation and any visibility changes in the released codebase. Ensure
wording and field names match the actual shipped interfaces and roadmap entries
in docs/, and update the roadmap references noted in the ExecPlan to the proper
completed roadmap items.
In `@docs/rust-doctest-dry-guide.md`:
- Around line 622-685: The footnote definitions in the "Works cited" section are
escaped (e.g., "\[^1\]:") so they don't render as proper numbered footnotes;
unescape all footnote markers to use GitHub-flavored Markdown syntax (change
"\[^n\]: " to "[^n]: "), ensure each footnote marker matches the in-text
references and that numbering follows the order of appearance (update any
mismatched numbers so [^1], [^2], ... are sequential), and leave the citation
URLs and text unchanged.
- Line 21: The documentation contains escaped inline footnote markers like
"\[^3\]" that prevent GitHub-flavored Markdown footnotes from rendering; update
occurrences (e.g., the instance shown as "\[^3\]" and the other reported
instances) by removing the backslashes so they become "[^3]" (and likewise for
any other escaped footnote tokens) to reinstate live footnote references in the
body; scan the file for the pattern "\[^" and replace with "[^" ensuring the
corresponding footnote IDs match the citations section.
In `@docs/rust-testing-with-rstest-fixtures.md`:
- Line 1347: Update the attribute description for #[rstest] to use the correct
indefinite article by changing "an rstest test" to "a rstest test" in the
docs/rust-testing-with-rstest-fixtures.md content, ensuring the phrase now reads
"a rstest test; enables fixture injection and parameterization." and follows the
project's en-GB-oxendict grammar/style rules.
- Around line 1366-1444: The footnote markers in the "Works cited" block are
escaped (e.g., "\[^1\]", "\[^6\]") which prevents GitHub-flavored Markdown
footnotes from linking; unescape all footnote markers and definitions by
removing the backslashes so markers read "[^1]" etc., and collapse multi-line
definitions into single lines beginning with the matching "[^n]:" definition
(for example change the GitHub entry currently split across lines into a single
"[^6]: GitHub, accessed on June 12, 2025,
<https://github.com/web-infra-dev/rstest>"); also verify the numeric ordering of
the footnote labels follows their first appearance in the document.
In `@README.md`:
- Around line 79-83: Change the British-spelling word "recognised" in the README
frontmatter description to the project-standard en-GB-oxendict "-ize" form
"recognized"; locate the sentence that reads "Only a block at the very beginning
of the document is recognised as frontmatter." and replace "recognised" with
"recognized" so the section about YAML frontmatter uses the repository's
standardized spelling.
In `@src/frontmatter.rs`:
- Around line 64-206: The test module contains many repetitive unit tests
calling split_leading_yaml_frontmatter; replace them with rstest parameterized
cases and shared fixtures: create an rstest fixture that provides the lines
Vec<String> inputs and expected (prefix_len, prefix_contents, body_contents)
tuples and then convert each named test (e.g., empty_input_returns_empty_slices,
no_frontmatter_returns_empty_prefix,
detects_frontmatter_with_triple_dash_closer,
detects_frontmatter_with_triple_dot_closer, unmatched_opener_treated_as_body,
frontmatter_with_empty_body, frontmatter_only_no_body,
indented_opener_not_recognized, later_dash_block_not_frontmatter,
whitespace_around_closer_is_trimmed, multiline_yaml_values_preserved) into
parameterized #[rstest] cases that call split_leading_yaml_frontmatter and
assert results; ensure you add the rstest dependency and use a fixture for
common Vec<String> construction to avoid duplication while keeping assertions
specific to expected prefix/body for each case.
- Around line 48-50: The code currently uses let trimmed = line.trim() and
matches trimmed == "---" || trimmed == "..." which wrongly treats indented
closers as valid; change the check to only remove trailing whitespace (e.g., use
trim_end() or equivalent) so leading whitespace is preserved and only exact raw
closers (possibly with trailing spaces) match; update the variable/condition
that uses trimmed (the matching logic around line/trimmed) to use the new
trim_end-based value so indented `---` inside YAML scalars are not treated as
frontmatter closers.
In `@src/process.rs`:
- Around line 351-416: Replace the four near-duplicate #[test] functions in the
tests module with a single parameterised rstest: import rstest::rstest, add a
#[rstest] fn frontmatter_detection_behaviour(#[case] input: Vec<&str>, #[case]
has_frontmatter: bool) that converts input into Vec<String>, calls
process_stream(&input), asserts out is not empty and then branches on
has_frontmatter to check frontmatter behaviour (e.g., assert_eq!(out[0], "---")
when true, otherwise assert body content presence). Add the four cases matching
the original scenarios (standard frontmatter with --- closer, triple-dot closer,
no frontmatter, and unmatched opener) so the original expectations are
preserved.
In `@tests/cli.rs`:
- Around line 394-524: Several YAML-frontmatter CLI tests are duplicated;
refactor them into parameterized rstest cases using a shared helper. Replace the
individual test functions (test_cli_yaml_frontmatter_preserved,
test_cli_yaml_frontmatter_with_wrap, test_cli_yaml_frontmatter_with_breaks,
test_cli_yaml_frontmatter_dot_closer, test_cli_later_dash_block_not_frontmatter,
test_cli_yaml_frontmatter_in_place) with one or more #[rstest] parameterised
cases that pass inputs, args (e.g., ["--wrap"], ["--breaks","--wrap"], [], or an
in-place marker), and expected output patterns to a single helper (e.g., run_cli
or run_in_place helper) that invokes Command::cargo_bin("mdtablefix"), writes
stdin, asserts success, and checks stdout; add the rstest crate import/attribute
where needed and keep one distinct case per flag combination and expected
pattern.
🪄 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
Run ID: 07debdba-9b6b-49b4-97b5-3d5548be3edc
📒 Files selected for processing (32)
AGENTS.mdREADME.mddocs/architecture.mddocs/documentation-style-guide.mddocs/execplans/yaml-frontmatter.mddocs/rust-doctest-dry-guide.mddocs/rust-testing-with-rstest-fixtures.mdrust-toolchain.tomlsrc/code_emphasis.rssrc/fences.rssrc/footnotes/inline.rssrc/footnotes/mod.rssrc/footnotes/renumber.rssrc/frontmatter.rssrc/headings.rssrc/html.rssrc/io.rssrc/lib.rssrc/lists.rssrc/main.rssrc/process.rssrc/wrap.rssrc/wrap/block.rssrc/wrap/fence.rssrc/wrap/inline.rssrc/wrap/line_buffer.rssrc/wrap/tokenize/mod.rssrc/wrap/tokenize/scanning.rstests/cli.rstests/code_emphasis.rstests/common/mod.rstests/parallel.rs
- Make frontmatter module #[doc(hidden)] to avoid committing to public API - Add two new CLI tests: - test_cli_yaml_frontmatter_with_renumber: tests frontmatter with --renumber - test_cli_malformed_yaml_frontmatter_treated_as_body: tests malformed frontmatter - Fix escaped footnote markers in rust-testing-with-rstest-fixtures.md Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
- Remove duplicate frontmatter splitting in CLI pipeline by refactoring process_stream_inner to not handle frontmatter directly - Move frontmatter handling to library API functions (process_stream, process_stream_no_wrap, process_stream_opts) - Fix frontmatter closer check to only trim trailing whitespace, preventing indented '---' inside YAML block scalars from being treated as closers - Update tests to reflect new behavior: indented closers are not recognized, but trailing whitespace is still trimmed Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
- Fix MD013/line-length in docs/execplans/yaml-frontmatter.md - Fix MD029/ol-prefix ordered list numbering in rust-doctest-dry-guide.md and rust-testing-with-rstest-fixtures.md Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/rust-doctest-dry-guide.md (1)
435-444:⚠️ Potential issue | 🟠 MajorRemove
.unwrap()from doctest examples and use Result-based error handling.Rewrite both examples to use
fn main() -> Result<(), Box<dyn std::error::Error>> { … }pattern with proper function signatures (using hidden boilerplate for annotations), so the guide teaches graceful error handling rather than panics.Proposed doc fixes
/// # #[cfg(feature = "serde")] /// # { +/// # fn main() -> Result<(), Box<dyn std::error::Error>> { /// # let my_struct = MyStruct::new(); -/// # let json = serde_json::to_string(&my_struct).unwrap(); +/// # let json = serde_json::to_string(&my_struct)?; /// # assert_eq!(json, "{}"); +/// # Ok(()) +/// # } /// # }//! #![cfg_attr(not(feature = "serde"), doc = "```ignore")] //! #![cfg_attr(feature = "serde", doc = "```")] +//! fn main() -> Result<(), Box<dyn std::error::Error>> { //! // Example code that requires the "serde" feature. //! let my_struct = MyStruct::new(); -//! let json = serde_json::to_string(&my_struct).unwrap(); +//! let json = serde_json::to_string(&my_struct)?; //! assert_eq!(json, "{}"); +//! Ok(()) +//! }Applies to: lines 435–444 and 459–466
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/rust-doctest-dry-guide.md` around lines 435 - 444, The doctest examples use .unwrap() which causes panics; replace them with a Result-returning main so errors propagate: wrap the hidden test blocks around a fn main() -> Result<(), Box<dyn std::error::Error>> { … Ok(()) } and change serde_json::to_string(&my_struct).unwrap() to serde_json::to_string(&my_struct)?; update both occurrences (the hidden #[cfg(feature = "serde")] block and the cfg_attr-annotated example) and keep MyStruct::new() and assert_eq!(json, "{}") intact while using the ? operator to propagate errors.src/process.rs (1)
258-303:⚠️ Potential issue | 🟡 MinorDelete the duplicated Rustdoc block for
process_stream_opts.Keep one authoritative doc comment block only; duplicate docs will diverge and mislead maintainers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/process.rs` around lines 258 - 303, The file contains duplicated Rustdoc for process_stream_opts; remove the redundant doc block so only one authoritative Rustdoc comment remains above the process_stream_opts declaration/function, ensuring the remaining block documents Options, the footnotes flag, and the example as currently present for process_stream_opts.
♻️ Duplicate comments (5)
docs/rust-doctest-dry-guide.md (1)
645-659:⚠️ Potential issue | 🟠 MajorRepair malformed “Works cited” entries and keep each source inside a valid footnote definition.
Remove orphaned fragments (
accessed on...,have doctests?...) and attach each citation line to its numbered[^n]:definition so references remain coherent and linkable.Triage:
[type:syntax/md][type:grammar]As per coding guidelines, "Use GitHub-flavoured Markdown style for footnotes and endnotes." and "Numbered footnotes must be numbered by order of appearance in the document."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/rust-doctest-dry-guide.md` around lines 645 - 659, Fix the malformed "Works cited" footnotes by consolidating each orphaned fragment into its corresponding numbered footnote definition (e.g., [^11], [^12], [^13], [^14], [^15]) so that each citation line begins with its [^n]: marker and contains the full reference and "accessed on ..." text; remove any stray fragments like "have doctests?..." or duplicated "accessed on..." lines, ensure the footnotes are ordered by appearance and follow GitHub-flavored Markdown footnote syntax, and verify that each [^n] is a single coherent citation entry for rust-doctest-dry-guide.md.src/process.rs (1)
379-444:⚠️ Potential issue | 🟠 MajorRefactor these frontmatter tests into parameterized
rstestcases and move them out of this file.Extract shared setup, convert scenarios into
#[rstest]cases, and place them in a dedicated test module to bringsrc/process.rsback under the 400-line limit.As per coding guidelines, "Use
rstestfixtures for shared setup in Rust tests", "Replace duplicated tests with#[rstest(...)]parameterized cases", and "Files must not exceed 400 lines in length."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/process.rs` around lines 379 - 444, The four frontmatter tests (preserves_yaml_frontmatter_unchanged, frontmatter_with_triple_dot_closer_preserved, no_frontmatter_processes_normally, unmatched_frontmatter_opener_processed_as_body) should be refactored into parameterized rstest cases that call process_stream with different input/expected pairs: extract the common input/expected setup into an rstest fixture (e.g., a helper that returns Vec<String> or the expected assertions) and replace the four individual #[test] functions with a single #[rstest] annotated table of scenarios; move the new parameterized tests into a new tests module/file (so src/process.rs falls under 400 lines) and ensure each scenario references process_stream and asserts the expected preservation/formatting behavior.tests/cli.rs (1)
394-573: 🛠️ Refactor suggestion | 🟠 MajorCollapse these frontmatter CLI scenarios into parameterized
rstestcases.Replace the near-identical tests with a shared helper plus
#[rstest]cases for flags/input/expected output, then keep one focused assertion set per case.As per coding guidelines, "Use
rstestfixtures for shared setup in Rust tests" and "Replace duplicated Rust tests with#[rstest(...)]parameterized cases."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli.rs` around lines 394 - 573, Many of the tests in tests::cli.rs (e.g., test_cli_yaml_frontmatter_preserved, test_cli_yaml_frontmatter_with_wrap, test_cli_yaml_frontmatter_with_breaks, test_cli_yaml_frontmatter_dot_closer, test_cli_later_dash_block_not_frontmatter, test_cli_yaml_frontmatter_in_place, test_cli_yaml_frontmatter_with_renumber, test_cli_malformed_yaml_frontmatter_treated_as_body) are near-duplicates and should be collapsed into parameterized rstest cases: create a single helper (e.g., fn run_cli_case(flags: &[&str], input: &str) -> String or reuse run_in_place for in-place cases) and replace individual #[test] functions with #[rstest(flags, input, expected => case(...), ...)] entries that pass the flags, input, and expected outputs; ensure cases cover --wrap, --breaks, --renumber, --in-place, malformed frontmatter, and later --- scenarios and keep one focused assertion per case (compare stdout or in-place result), and add appropriate imports/attributes for rstest at the top of the file.src/frontmatter.rs (1)
66-223: 🛠️ Refactor suggestion | 🟠 MajorParameterize these unit tests with
rstestand shared fixtures.Replace repetitive arrange/assert blocks with
#[rstest]cases to reduce duplication and tighten maintenance.As per coding guidelines, "Use
rstestfixtures for shared setup in Rust tests" and "Replace duplicated tests with#[rstest(...)]parameterized cases."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontmatter.rs` around lines 66 - 223, Convert the duplicated unit tests for split_leading_yaml_frontmatter into parameterized rstest cases: add rstest to dev-dependencies, create a fixture (e.g., a function named lines_fixture or use #[fixture]) to provide common Vec<String> setups, then replace the many #[test] functions (empty_input_returns_empty_slices, no_frontmatter_returns_empty_prefix, detects_frontmatter_with_triple_dash_closer, detects_frontmatter_with_triple_dot_closer, unmatched_opener_treated_as_body, frontmatter_with_empty_body, frontmatter_only_no_body, indented_opener_not_recognized, later_dash_block_not_frontmatter, indented_closer_not_recognized, trailing_whitespace_on_closer_is_trimmed, multiline_yaml_values_preserved) with a single #[rstest] parameterized test that supplies (input_lines, expected_prefix_len, expected_body_len, optional_expected_prefix_contents or expected_body_sample) for each case and asserts against split_leading_yaml_frontmatter(&input_lines); keep any special content assertions (e.g., exact strings) as additional parameters per case.docs/execplans/yaml-frontmatter.md (1)
8-8:⚠️ Potential issue | 🟡 MinorUpdate this ExecPlan to delivered status and shipped interfaces.
Set the status and progress/outcome sections to reflect completed delivery, and sync the interface note with the implemented visibility/export shape.
Triage:
[type:docstyle]As per coding guidelines, "Ensure that any API or behavioural changes are reflected in the documentation in
docs/" and "Ensure that any completed roadmap steps are recorded in the appropriate roadmap indocs/."Also applies to: 88-97, 128-131, 313-327
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/execplans/yaml-frontmatter.md` at line 8, Replace the "Status: DRAFT" frontmatter with "Status: DELIVERED" and update the corresponding progress/outcome fields to reflect completed delivery and shipped interfaces; also edit the "interface note" section to describe the implemented visibility/export shape (matching the runtime API/visibility behavior). Repeat these updates for the other occurrences referenced (the sections around the previously noted blocks) so all docs reflect delivered status and the actual exported interface shape.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/rust-doctest-dry-guide.md`:
- Around line 107-112: Remove the HTML markdownlint disable/enable comments for
MD013 and reflow the paragraph text to wrap at 80 columns, keeping the same
content (mention of triple back-ticks, the `rust` language specifier, and the
macros `assert!`, `assert_eq!`, `assert_ne!`) so the prose complies with the
project's Markdown rules; replace the existing comment-wrapped block (the HTML
comments containing "markdownlint-disable MD013" and "markdownlint-enable
MD013") with a normal wrapped paragraph of plain Markdown that stays within 80
characters per line and preserves the inline code formatting and citations.
In `@src/frontmatter.rs`:
- Around line 5-7: Update the frontmatter documentation to state that YAML
closers accept trailing whitespace by noting that the implementation compares
line.trim_end() to '---' and '...'; change the top-level doc sentence about the
closer (the line mentioning "containing exactly `---` or `...`") and the other
doc occurrences (the lines around 13-14 and 50-52) to say the closer may have
trailing whitespace and that matching is done after trimming trailing whitespace
so readers know the parser allows e.g. '--- ' or '... '.
In `@tests/cli.rs`:
- Around line 394-573: The tests related to YAML frontmatter (functions
test_cli_yaml_frontmatter_preserved, test_cli_yaml_frontmatter_with_wrap,
test_cli_yaml_frontmatter_with_breaks, test_cli_yaml_frontmatter_dot_closer,
test_cli_later_dash_block_not_frontmatter, test_cli_yaml_frontmatter_in_place,
test_cli_yaml_frontmatter_with_renumber,
test_cli_malformed_yaml_frontmatter_treated_as_body) push tests/cli.rs past the
400-line limit; extract this contiguous frontmatter test block into a new
integration test file (e.g., tests/cli_frontmatter.rs), remove the moved tests
from tests/cli.rs, and ensure any helpers or imports used by those tests (such
as run_in_place, Command::cargo_bin usage, and String::from_utf8_lossy) are
available in the new file by copying required use statements or making helpers
public/shared before running tests.
---
Outside diff comments:
In `@docs/rust-doctest-dry-guide.md`:
- Around line 435-444: The doctest examples use .unwrap() which causes panics;
replace them with a Result-returning main so errors propagate: wrap the hidden
test blocks around a fn main() -> Result<(), Box<dyn std::error::Error>> { …
Ok(()) } and change serde_json::to_string(&my_struct).unwrap() to
serde_json::to_string(&my_struct)?; update both occurrences (the hidden
#[cfg(feature = "serde")] block and the cfg_attr-annotated example) and keep
MyStruct::new() and assert_eq!(json, "{}") intact while using the ? operator to
propagate errors.
In `@src/process.rs`:
- Around line 258-303: The file contains duplicated Rustdoc for
process_stream_opts; remove the redundant doc block so only one authoritative
Rustdoc comment remains above the process_stream_opts declaration/function,
ensuring the remaining block documents Options, the footnotes flag, and the
example as currently present for process_stream_opts.
---
Duplicate comments:
In `@docs/execplans/yaml-frontmatter.md`:
- Line 8: Replace the "Status: DRAFT" frontmatter with "Status: DELIVERED" and
update the corresponding progress/outcome fields to reflect completed delivery
and shipped interfaces; also edit the "interface note" section to describe the
implemented visibility/export shape (matching the runtime API/visibility
behavior). Repeat these updates for the other occurrences referenced (the
sections around the previously noted blocks) so all docs reflect delivered
status and the actual exported interface shape.
In `@docs/rust-doctest-dry-guide.md`:
- Around line 645-659: Fix the malformed "Works cited" footnotes by
consolidating each orphaned fragment into its corresponding numbered footnote
definition (e.g., [^11], [^12], [^13], [^14], [^15]) so that each citation line
begins with its [^n]: marker and contains the full reference and "accessed on
..." text; remove any stray fragments like "have doctests?..." or duplicated
"accessed on..." lines, ensure the footnotes are ordered by appearance and
follow GitHub-flavored Markdown footnote syntax, and verify that each [^n] is a
single coherent citation entry for rust-doctest-dry-guide.md.
In `@src/frontmatter.rs`:
- Around line 66-223: Convert the duplicated unit tests for
split_leading_yaml_frontmatter into parameterized rstest cases: add rstest to
dev-dependencies, create a fixture (e.g., a function named lines_fixture or use
#[fixture]) to provide common Vec<String> setups, then replace the many #[test]
functions (empty_input_returns_empty_slices,
no_frontmatter_returns_empty_prefix,
detects_frontmatter_with_triple_dash_closer,
detects_frontmatter_with_triple_dot_closer, unmatched_opener_treated_as_body,
frontmatter_with_empty_body, frontmatter_only_no_body,
indented_opener_not_recognized, later_dash_block_not_frontmatter,
indented_closer_not_recognized, trailing_whitespace_on_closer_is_trimmed,
multiline_yaml_values_preserved) with a single #[rstest] parameterized test that
supplies (input_lines, expected_prefix_len, expected_body_len,
optional_expected_prefix_contents or expected_body_sample) for each case and
asserts against split_leading_yaml_frontmatter(&input_lines); keep any special
content assertions (e.g., exact strings) as additional parameters per case.
In `@src/process.rs`:
- Around line 379-444: The four frontmatter tests
(preserves_yaml_frontmatter_unchanged,
frontmatter_with_triple_dot_closer_preserved, no_frontmatter_processes_normally,
unmatched_frontmatter_opener_processed_as_body) should be refactored into
parameterized rstest cases that call process_stream with different
input/expected pairs: extract the common input/expected setup into an rstest
fixture (e.g., a helper that returns Vec<String> or the expected assertions) and
replace the four individual #[test] functions with a single #[rstest] annotated
table of scenarios; move the new parameterized tests into a new tests
module/file (so src/process.rs falls under 400 lines) and ensure each scenario
references process_stream and asserts the expected preservation/formatting
behavior.
In `@tests/cli.rs`:
- Around line 394-573: Many of the tests in tests::cli.rs (e.g.,
test_cli_yaml_frontmatter_preserved, test_cli_yaml_frontmatter_with_wrap,
test_cli_yaml_frontmatter_with_breaks, test_cli_yaml_frontmatter_dot_closer,
test_cli_later_dash_block_not_frontmatter, test_cli_yaml_frontmatter_in_place,
test_cli_yaml_frontmatter_with_renumber,
test_cli_malformed_yaml_frontmatter_treated_as_body) are near-duplicates and
should be collapsed into parameterized rstest cases: create a single helper
(e.g., fn run_cli_case(flags: &[&str], input: &str) -> String or reuse
run_in_place for in-place cases) and replace individual #[test] functions with
#[rstest(flags, input, expected => case(...), ...)] entries that pass the flags,
input, and expected outputs; ensure cases cover --wrap, --breaks, --renumber,
--in-place, malformed frontmatter, and later --- scenarios and keep one focused
assertion per case (compare stdout or in-place result), and add appropriate
imports/attributes for rstest at the top of the file.
🪄 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
Run ID: d6dc4d68-09f5-42f2-b794-beeaa39f853b
📒 Files selected for processing (6)
docs/execplans/yaml-frontmatter.mddocs/rust-doctest-dry-guide.mdsrc/frontmatter.rssrc/lib.rssrc/process.rstests/cli.rs
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
- Fix escaped footnote markers in rust-testing-with-rstest-fixtures.md (change \[^n\] to [^n] format) - Remove unused footnote definitions [^6], [^14], [^23] - Extract frontmatter tests from src/process.rs to new tests/process_frontmatter.rs to bring process.rs under 400 lines Note: frontmatter module visibility kept as pub with #[doc(hidden)] rather than pub(crate) because the binary crate requires access to split_leading_yaml_frontmatter. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/process.rs (1)
258-303:⚠️ Potential issue | 🟡 MinorRemove the duplicated Rustdoc block for
process_stream_opts.Delete the second, repeated documentation section to prevent doc drift and duplicated generated docs.
Diff
-/// Runs [`process_stream_inner`] with custom [`Options`]. -/// -/// This is exposed for advanced use cases where callers want precise -/// control over the processing pipeline. Set `footnotes: true` in `opts` -/// to convert bare numeric references into GitHub-flavoured footnote -/// links. The flag defaults to `false`. -/// -/// # Examples -/// -/// ``` -/// use mdtablefix::process::{Options, process_stream_opts}; -/// let lines = vec!["text".to_string()]; -/// let opts = Options { -/// wrap: false, -/// ellipsis: false, -/// fences: false, -/// footnotes: false, -/// code_emphasis: false, -/// headings: false, -/// }; -/// let out = process_stream_opts(&lines, opts); -/// assert_eq!(out, vec!["text"]); -/// ```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/process.rs` around lines 258 - 303, The Rustdoc for process_stream_opts is duplicated; remove the repeated documentation block so there is only a single doc comment above the process_stream_opts declaration: locate the second identical doc block (the repeated triple-slash comments and the fenced code example) and delete it, leaving the first documentation block intact to prevent duplicate generated docs and doc drift.
♻️ Duplicate comments (2)
docs/rust-testing-with-rstest-fixtures.md (1)
1347-1347:⚠️ Potential issue | 🟡 MinorFix the article in the
#[rstest]description.Change “an rstest test” to “a rstest test”.
Triage:
[type:grammar]As per coding guidelines, “Comments and docs must follow en-GB-oxendict
(-ize / -yse / -our) spelling and grammar”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/rust-testing-with-rstest-fixtures.md` at line 1347, Update the documentation line for the attribute #[rstest] to use correct grammar: replace the phrase "an rstest test" with "a rstest test" in the description so it reads "Marks a function as a rstest test; enables fixture injection and parameterization."; locate the #[rstest] entry in the markdown and make this single-word article change to conform to en-GB-oxendict spelling/grammar guidelines.tests/process_frontmatter.rs (1)
5-70: 🧹 Nitpick | 🔵 TrivialReplace repeated frontmatter scenarios with parameterised
rstestcases.Collapse these near-identical tests into one
#[rstest]with#[case]inputs/expectations to cut duplication and keep behaviour updates atomic.As per coding guidelines, "Use
rstestfixtures for shared setup and to avoid repetition between tests." and "Replace duplicated tests with#[rstest(...)]parameterised cases."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/process_frontmatter.rs` around lines 5 - 70, Collapse the four near-duplicate tests (preserves_yaml_frontmatter_unchanged, frontmatter_with_triple_dot_closer_preserved, no_frontmatter_processes_normally, unmatched_frontmatter_opener_processed_as_body) into a single parameterised rstest that supplies cases of input Vec<String> and expected behaviors for process_stream; add #[rstest] and #[case] for each scenario, import rstest::rstest, and in the single test function call process_stream(&case_input) then branch assertions based on a small enum or flags per case (e.g., expect_frontmatter_unchanged, expect_body_contains, expect_joined_contains) to reproduce the original assertions while avoiding duplication and preserving checks that reference process_stream and the original expectations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/process.rs`:
- Around line 224-235: There is duplicated logic calling
split_leading_yaml_frontmatter, process_stream_inner with Options { wrap: true,
..Default::default() }, and then re-prepending the frontmatter_prefix; extract
that into a single helper (e.g., preserve_frontmatter_and_process or
process_with_frontmatter) that accepts the incoming lines, runs
split_leading_yaml_frontmatter, calls process_stream_inner(body, Options { wrap:
true, ..Default::default() }), concatenates frontmatter_prefix + out, and
returns the resulting Vec; replace the three public entry points that currently
repeat this sequence with calls to this new helper so behavior remains
centralized and synchronized (references: split_leading_yaml_frontmatter,
process_stream_inner, Options, wrap, frontmatter_prefix).
---
Outside diff comments:
In `@src/process.rs`:
- Around line 258-303: The Rustdoc for process_stream_opts is duplicated; remove
the repeated documentation block so there is only a single doc comment above the
process_stream_opts declaration: locate the second identical doc block (the
repeated triple-slash comments and the fenced code example) and delete it,
leaving the first documentation block intact to prevent duplicate generated docs
and doc drift.
---
Duplicate comments:
In `@docs/rust-testing-with-rstest-fixtures.md`:
- Line 1347: Update the documentation line for the attribute #[rstest] to use
correct grammar: replace the phrase "an rstest test" with "a rstest test" in the
description so it reads "Marks a function as a rstest test; enables fixture
injection and parameterization."; locate the #[rstest] entry in the markdown and
make this single-word article change to conform to en-GB-oxendict
spelling/grammar guidelines.
In `@tests/process_frontmatter.rs`:
- Around line 5-70: Collapse the four near-duplicate tests
(preserves_yaml_frontmatter_unchanged,
frontmatter_with_triple_dot_closer_preserved, no_frontmatter_processes_normally,
unmatched_frontmatter_opener_processed_as_body) into a single parameterised
rstest that supplies cases of input Vec<String> and expected behaviors for
process_stream; add #[rstest] and #[case] for each scenario, import
rstest::rstest, and in the single test function call process_stream(&case_input)
then branch assertions based on a small enum or flags per case (e.g.,
expect_frontmatter_unchanged, expect_body_contains, expect_joined_contains) to
reproduce the original assertions while avoiding duplication and preserving
checks that reference process_stream and the original expectations.
🪄 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
Run ID: 541d5944-a886-4066-91f6-a2116d6c4404
📒 Files selected for processing (3)
docs/rust-testing-with-rstest-fixtures.mdsrc/process.rstests/process_frontmatter.rs
- Remove HTML markdownlint disable/enable comments in rust-doctest-dry-guide.md and reflow paragraph to wrap at 80 columns - Update frontmatter documentation to clarify that YAML closers accept trailing whitespace (matching is done after trim_end()) - Remove duplicated Rustdoc for process_stream_opts in src/process.rs - Extract frontmatter CLI tests from tests/cli.rs to new tests/cli_frontmatter.rs to bring cli.rs under 400 lines (now 392 lines) Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/frontmatter.rs (1)
67-224: 🧹 Nitpick | 🔵 TrivialConsolidate repetitive unit tests into parameterised
rstestcases.The 11 individual
#[test]functions share the same arrange/act/assert pattern. Replace them withrstestparameterised cases to reduce duplication and maintenance overhead.♻️ Refactor sketch
use rstest::rstest; fn s(v: &[&str]) -> Vec<String> { v.iter().map(|s| s.to_string()).collect() } #[rstest] #[case::empty_input_returns_empty_slices(&[], 0, 0)] #[case::no_frontmatter_returns_empty_prefix(&["# Heading", "Some text"], 0, 2)] #[case::detects_frontmatter_with_triple_dash_closer( &["---", "title: Example", "author: Test", "---", "# Heading", "Body text"], 4, 2 )] #[case::detects_frontmatter_with_triple_dot_closer( &["---", "title: Example", "...", "# Heading"], 3, 1 )] #[case::unmatched_opener_treated_as_body( &["---", "Some text", "More text"], 0, 3 )] // ... additional cases fn frontmatter_detection( #[case] input: &[&str], #[case] expected_prefix_len: usize, #[case] expected_body_len: usize, ) { let lines = s(input); let (prefix, body) = split_leading_yaml_frontmatter(&lines); assert_eq!(prefix.len(), expected_prefix_len); assert_eq!(body.len(), expected_body_len); }As per coding guidelines: "Replace duplicated tests with
#[rstest(...)]parameterised cases."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontmatter.rs` around lines 67 - 224, Consolidate the 11 repetitive #[test] functions that all call split_leading_yaml_frontmatter into a single parameterised rstest: add rstest to dev-dependencies, create a helper s(&[&str]) -> Vec<String>, then replace the individual tests with one #[rstest] function (e.g. frontmatter_detection) that accepts #[case] inputs and expected prefix/body lengths and asserts on prefix.len() and body.len(); include separate #[case::...] entries for each original test scenario (empty_input_returns_empty_slices, no_frontmatter_returns_empty_prefix, detects_frontmatter_with_triple_dash_closer, detects_frontmatter_with_triple_dot_closer, unmatched_opener_treated_as_body, frontmatter_with_empty_body, frontmatter_only_no_body, indented_opener_not_recognized, later_dash_block_not_frontmatter, indented_closer_not_recognized, trailing_whitespace_on_closer_is_trimmed, multiline_yaml_values_preserved) to preserve all behaviors.src/process.rs (1)
224-235: 🛠️ Refactor suggestion | 🟠 MajorExtract a shared helper for the frontmatter-preserving entry points.
The three public functions (
process_stream,process_stream_no_wrap,process_stream_opts) duplicate the same split/process/prepend sequence. Centralise this into one helper to keep behaviour synchronised and reduce maintenance overhead.♻️ Proposed refactor
+fn process_with_frontmatter<F>(lines: &[String], process_body: F) -> Vec<String> +where + F: FnOnce(&[String]) -> Vec<String>, +{ + let (frontmatter_prefix, body) = split_leading_yaml_frontmatter(lines); + let mut result = frontmatter_prefix.to_vec(); + result.extend(process_body(body)); + result +} + pub fn process_stream(lines: &[String]) -> Vec<String> { - let (frontmatter_prefix, body) = split_leading_yaml_frontmatter(lines); - let out = process_stream_inner( - body, - Options { - wrap: true, - ..Default::default() - }, - ); - let mut result = frontmatter_prefix.to_vec(); - result.extend(out); - result + process_with_frontmatter(lines, |body| { + process_stream_inner(body, Options { wrap: true, ..Default::default() }) + }) }Apply the same pattern to
process_stream_no_wrapandprocess_stream_opts.Also applies to: 251-255, 283-287
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/process.rs` around lines 224 - 235, The three public entry points process_stream, process_stream_no_wrap, and process_stream_opts duplicate the split/process/prepend sequence (calling split_leading_yaml_frontmatter, process_stream_inner and then re-prepending frontmatter_prefix). Extract a single helper (e.g., process_with_frontmatter) that accepts the input lines and an Options value (or a closure to call process_stream_inner) and returns the combined result, then rewrite process_stream, process_stream_no_wrap, and process_stream_opts to call that helper (preserving the same behavior of split_leading_yaml_frontmatter and using process_stream_inner) so all three share the same implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/frontmatter.rs`:
- Around line 13-14: Remove the stray closing brace in the doc comment in
src/frontmatter.rs: locate the doc comment that ends with "(matching is done
after `trim_end()`)}" and delete the trailing "}" so the sentence reads
"(matching is done after `trim_end()`)". This is purely a documentation fix—no
code logic changes required; update the comment near the frontmatter parsing
documentation to remove the extraneous character.
In `@tests/cli_frontmatter.rs`:
- Around line 82-88: Rename the ambiguous local variable binding used to capture
the Command::cargo_bin("mdtablefix") assertion result to a descriptive name
(e.g., cmd_result or assert_result) in the
Command::cargo_bin(...).args([...]).write_stdin(input).assert().success() chain
and update its subsequent use where you call get_output().stdout to build the
output string; ensure the new name replaces all occurrences of binding in this
test block so the variable clearly indicates it holds the assertion/command
result.
- Around line 58-66: The variable named `binding` in the test that builds the
Command::cargo_bin("mdtablefix") pipeline should be renamed to a descriptive
name (e.g., `assertion` or `result`) to clarify intent; update the variable
declaration returned from `.assert()` (currently assigned to `binding`) and all
subsequent uses (`binding.get_output().stdout`) to the new name so the test
remains correct and readable while still asserting `.success()` and checking the
output starts with the expected frontmatter.
---
Duplicate comments:
In `@src/frontmatter.rs`:
- Around line 67-224: Consolidate the 11 repetitive #[test] functions that all
call split_leading_yaml_frontmatter into a single parameterised rstest: add
rstest to dev-dependencies, create a helper s(&[&str]) -> Vec<String>, then
replace the individual tests with one #[rstest] function (e.g.
frontmatter_detection) that accepts #[case] inputs and expected prefix/body
lengths and asserts on prefix.len() and body.len(); include separate
#[case::...] entries for each original test scenario
(empty_input_returns_empty_slices, no_frontmatter_returns_empty_prefix,
detects_frontmatter_with_triple_dash_closer,
detects_frontmatter_with_triple_dot_closer, unmatched_opener_treated_as_body,
frontmatter_with_empty_body, frontmatter_only_no_body,
indented_opener_not_recognized, later_dash_block_not_frontmatter,
indented_closer_not_recognized, trailing_whitespace_on_closer_is_trimmed,
multiline_yaml_values_preserved) to preserve all behaviors.
In `@src/process.rs`:
- Around line 224-235: The three public entry points process_stream,
process_stream_no_wrap, and process_stream_opts duplicate the
split/process/prepend sequence (calling split_leading_yaml_frontmatter,
process_stream_inner and then re-prepending frontmatter_prefix). Extract a
single helper (e.g., process_with_frontmatter) that accepts the input lines and
an Options value (or a closure to call process_stream_inner) and returns the
combined result, then rewrite process_stream, process_stream_no_wrap, and
process_stream_opts to call that helper (preserving the same behavior of
split_leading_yaml_frontmatter and using process_stream_inner) so all three
share the same implementation.
🪄 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
Run ID: 1be31c02-c572-4e29-a3a8-fc0c16ccd68e
📒 Files selected for processing (3)
src/frontmatter.rssrc/process.rstests/cli_frontmatter.rs
- Remove HTML markdownlint comments from rust-doctest-dry-guide.md and reflow paragraph to 80 columns - Update execplans/yaml-frontmatter.md status to DELIVERED and document the actual interface/visibility decisions - Fix grammar in rust-testing-with-rstest-fixtures.md (an rstest to a rstest) - Extract helper function process_with_frontmatter in src/process.rs to eliminate duplicated logic across process_stream, process_stream_no_wrap, and process_stream_opts Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
- Change process_lines in src/main.rs to use process_stream_inner instead of process_stream_opts, since frontmatter is already split by process_lines - Update imports in src/main.rs to use process::process_stream_inner - Remove stray closing brace in src/frontmatter.rs doc comment - Fix escaped footnote in docs/documentation-style-guide.md - Rename ambiguous 'binding' variables to 'cmd_result' in tests/cli_frontmatter.rs Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
- Make `frontmatter` module private (`mod frontmatter` instead of `#[doc(hidden)] pub mod frontmatter`) - properly enforces internal-only boundary since `#[doc(hidden)]` only hides from docs but doesn't restrict linkage - Re-export only `split_leading_yaml_frontmatter` function publicly while keeping the module itself private - Revert rust-toolchain.toml to nightly-2026-03-26 as the project uses unstable rustfmt features Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace standalone test functions with rstest parameterised cases: - Add `use rstest::rstest` - Merge 5 stdin→stdout equality tests into single `test_cli_yaml_frontmatter_stdin` with `#[case]` attributes - Replace `test_cli_yaml_frontmatter_in_place` with `test_cli_yaml_frontmatter_in_place_variants` parameterised rstest - Keep `test_cli_yaml_frontmatter_with_wrap` and `test_cli_yaml_frontmatter_with_breaks` as standalone `#[test]` with explanatory comments about why they cannot be parameterised All 8 tests pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace four near-duplicate test functions with a single parameterised `#[rstest]` test `frontmatter_detection_behaviour`: - Add `use rstest::rstest` - Merge `preserves_yaml_frontmatter_unchanged`, `frontmatter_with_triple_dot_closer_preserved`, `no_frontmatter_processes_normally`, and `unmatched_frontmatter_opener_processed_as_body` into a single parameterised test with descriptive `#[case]` attributes - Leave `process_stream_inner_does_not_handle_frontmatter` untouched as requested All 5 tests pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace 12 individual test functions with two parameterised rstest suites: - Add `use rstest::rstest` and helper `fn s()` for string conversion - `prefix_empty_cases`: 6 cases where no frontmatter is detected (empty input, no frontmatter, unmatched opener, indented opener, later dash block, indented closer) - `frontmatter_split_cases`: 6 cases where frontmatter is detected (triple dash closer, triple dot closer, empty body, no body, trailing whitespace trimmed, multiline YAML values) All 12 tests pass with full coverage preserved. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Conform to en-GB-oxendict spelling standard (-ize, not -ise). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 853
1.
|
Change from re-export pattern to direct module access: - Make `frontmatter` module `pub` with `#[doc(hidden)]` to hide from docs - Remove re-export of `split_leading_yaml_frontmatter` from crate root - Update `main.rs` to import directly from `mdtablefix::frontmatter` This keeps the module accessible to the binary while removing it from the public API documentation. External crates can still technically link to it, but it's not part of the supported public interface. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 2905
1.
|
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
docs/rust-testing-with-rstest-fixtures.md (1)
1347-1347:⚠️ Potential issue | 🟡 MinorFix the article: "a rstest test" not "an rstest test".
Change "an rstest test" to "a rstest test" because "rstest" begins with the consonant sound /r/.
Triage:
[type:grammar]🔧 Proposed fix
-| #[rstest] | Marks a function as an rstest test; enables fixture injection and parameterization. | +| #[rstest] | Marks a function as a rstest test; enables fixture injection and parameterization. |As per coding guidelines, documentation must use en-GB-oxendict spelling and grammar.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/rust-testing-with-rstest-fixtures.md` at line 1347, The table row describing #[rstest] uses the incorrect article "an rstest test"; update the text to "a rstest test" so it reads "Marks a function as a rstest test; enables fixture injection and parameterization." — edit the cell containing the #[rstest] description in docs/rust-testing-with-rstest-fixtures.md to replace "an rstest test" with "a rstest test" and ensure the rest of the sentence follows en-GB-oxendict spelling and punctuation conventions.docs/execplans/yaml-frontmatter.md (1)
126-131:⚠️ Potential issue | 🟡 MinorOutcomes section contradicts DELIVERED status.
Lines 128-131 state "This plan is still in draft. No implementation has started yet." This directly contradicts the "DELIVERED" status on line 8. Update this section to reflect the actual delivered outcome, including:
- The splitter was implemented in
src/frontmatter.rs- Frontmatter handling was threaded through both
src/process.rsandsrc/main.rs- Test coverage was added
Triage:
[type:docstyle]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/execplans/yaml-frontmatter.md` around lines 126 - 131, Update the "Outcomes & retrospective" paragraph to reflect the DELIVERED status: state that the frontmatter splitter was implemented (frontmatter module), frontmatter handling was integrated through the process and main modules (process module and main module), and test coverage was added; mention that tests validate both the library and CLI paths preserve the leading YAML block and remove the sentence claiming the plan is still in draft.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/execplans/yaml-frontmatter.md`:
- Line 8: The document currently shows the header "Status: DELIVERED" while the
progress checklist still has unchecked items; open the YAML/frontmatter section
and the "Progress" checklist and mark completed items by changing their list
markers from "[ ]" to "[x]" (update the specific checklist lines currently
showing unchecked boxes), and append datestamps (e.g., YYYY-MM-DD) next to each
completed item for consistency; ensure the "Status: DELIVERED" remains accurate
after you update the checklist or change the status if items remain incomplete.
- Around line 86-96: Update the checklist in the YAML frontmatter doc by marking
every remaining "[ ]" item as completed ("[x]") and add a datestamp in the same
format as the existing entry (e.g., "(2026-04-05 22:45Z)") for each: replace "-
[ ] Add a shared helper for detecting and splitting leading YAML frontmatter.",
"- [ ] Thread the helper through the library and CLI formatting pipeline so all
transforms skip the frontmatter prefix.", "- [ ] Add unit and behavioural
regression tests covering detection, wrapping, and `--breaks`.", "- [ ] Update
`README.md` and `docs/architecture.md`.", and "- [ ] Run `make check-fmt`, `make
lint`, `make test`, `make markdownlint`, and `make nixie` if Mermaid content
changes." with "- [x]" plus an appropriate datestamp to reflect completion.
In `@tests/cli_frontmatter.rs`:
- Around line 124-147: The comment strings in the test file use the en-US
spelling "parameterised" and must be corrected to en-GB-oxendict
"parameterized"; update both occurrences in the comment above the test function
test_cli_yaml_frontmatter_with_wrap (the line starting "// Cannot be
parameterised: uses partial/line-level assertions rather than stdout equality.")
and the later similar comment before the next test (the line starting "// Cannot
be parameterised: uses partial/line-level assertions rather than stdout
equality.") by replacing "parameterised" with "parameterized" to comply with the
project's spelling guidelines.
- Around line 7-17: The helper function run_in_place should be replaced by an
rstest fixture: add a #[fixture] function (e.g. in_place_runner) that returns a
closure or function pointer with signature Fn(&[&str], &str, &str) which creates
a tempfile (tempfile::NamedTempFile), writes the provided input, runs the binary
(Command::cargo_bin("mdtablefix")...
.arg("--in-place").args(args).arg(temp.path()).assert().success()), reads back
the file and asserts equality with expected; update tests that used run_in_place
to take the in_place_runner fixture parameter and call it instead, and remove
the old run_in_place definition.
- Around line 169-173: Add a length check before indexing the lines vector in
the test: verify that lines.len() >= 3 (or use assert!(lines.len() >= 3,
"expected at least 3 lines in output: {}", output)) before the existing asserts
that reference lines[0], lines[1], and lines[2] in tests/cli_frontmatter.rs so
test failures show a clear length-related assertion instead of an
index-out-of-bounds panic.
---
Duplicate comments:
In `@docs/execplans/yaml-frontmatter.md`:
- Around line 126-131: Update the "Outcomes & retrospective" paragraph to
reflect the DELIVERED status: state that the frontmatter splitter was
implemented (frontmatter module), frontmatter handling was integrated through
the process and main modules (process module and main module), and test coverage
was added; mention that tests validate both the library and CLI paths preserve
the leading YAML block and remove the sentence claiming the plan is still in
draft.
In `@docs/rust-testing-with-rstest-fixtures.md`:
- Line 1347: The table row describing #[rstest] uses the incorrect article "an
rstest test"; update the text to "a rstest test" so it reads "Marks a function
as a rstest test; enables fixture injection and parameterization." — edit the
cell containing the #[rstest] description in
docs/rust-testing-with-rstest-fixtures.md to replace "an rstest test" with "a
rstest test" and ensure the rest of the sentence follows en-GB-oxendict spelling
and punctuation conventions.
🪄 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
Run ID: 20e48d8a-0e4c-44de-b813-0a0c88f826a4
📒 Files selected for processing (11)
README.mddocs/documentation-style-guide.mddocs/execplans/yaml-frontmatter.mddocs/rust-doctest-dry-guide.mddocs/rust-testing-with-rstest-fixtures.mdsrc/frontmatter.rssrc/lib.rssrc/main.rssrc/process.rstests/cli_frontmatter.rstests/process_frontmatter.rs
|
@coderabbitai please create a new GitHub issue for the following: 1.
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 47
It captures the required change (replace |
- Update docs/execplans/yaml-frontmatter.md progress checklist with completed items and datestamps (2026-04-09) - Update Outcomes & retrospective section to reflect DELIVERED status - Fix spelling: parameterised → parameterized (en-GB-oxendict) - Replace run_in_place helper with rstest in_place_runner fixture - Add length check before indexing lines in test_cli_yaml_frontmatter_with_breaks Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib.rs`:
- Around line 30-31: The frontmatter module is currently exported publicly
(#[doc(hidden)] pub mod frontmatter) exposing internals like
split_leading_yaml_frontmatter; change the module visibility to crate-only (make
frontmatter pub(crate) rather than pub) so the parser internals are not part of
the public API, and update any external callers (e.g., CLI code) to call the
stable public processing API you expose from lib (route through the public
processing functions instead of referencing
frontmatter::split_leading_yaml_frontmatter directly).
In `@src/main.rs`:
- Around line 17-23: The CLI is importing the internal symbol
frontmatter::split_leading_yaml_frontmatter directly; replace that with the
crate's public/stable re-export so the binary depends only on the public API.
Remove the `frontmatter::split_leading_yaml_frontmatter` path from the use list
and instead import the re-exported function from the crate root (e.g., use
mdtablefix::split_leading_yaml_frontmatter) and update any call sites in main
(where process_stream_inner, Options, renumber_lists, format_breaks are used) to
reference that public symbol so the binary no longer depends on the internal
frontmatter module.
🪄 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
Run ID: 27476e65-cd0b-4e72-9c2d-fc8f0657b617
📒 Files selected for processing (2)
src/lib.rssrc/main.rs
Wrap long lines in the Progress checklist to stay within 80 columns. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/execplans/yaml-frontmatter.md`:
- Around line 315-344: Update the interface and revision notes to explicitly
mark the current visibility choice as temporary: state that the module is
declared as "pub mod frontmatter" with #[doc(hidden)] only as an interim design
to allow the binary crate to access split_leading_yaml_frontmatter, and add that
issue `#264` tracks the follow-up to change this to pub(crate) once the crates are
restructured; also update the revision note to record that the visibility
narrowing is outstanding and tracked by `#264`.
🪄 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
Run ID: 13ff600b-0acb-4f4b-968f-44f13acdc1e5
📒 Files selected for processing (2)
docs/execplans/yaml-frontmatter.mdtests/cli_frontmatter.rs
Add #[doc(hidden)] re-export of split_leading_yaml_frontmatter to lib.rs and update main.rs to import from the crate root instead of directly from the frontmatter module. This allows the binary to use the public API while keeping the function hidden from documentation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/lib.rs (1)
30-31:⚠️ Potential issue | 🟠 MajorClose the unintended public API surface for frontmatter.
Change Line 31 and Line 54 now.
#[doc(hidden)]does not enforce encapsulation; it still exports internals and locksfrontmatterparsing into the public contract.Proposed boundary fix
-#[doc(hidden)] -pub mod frontmatter; +pub(crate) mod frontmatter; @@ -#[doc(hidden)] -pub use frontmatter::split_leading_yaml_frontmatter;Based on learnings: “Do not compromise API design by exposing private items in a
detailorinternalmodule solely to make them documentable; use dedicated unit tests instead”.Also applies to: 53-54
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib.rs` around lines 30 - 31, The frontmatter module is unintentionally public; change its declaration from pub mod frontmatter to a private module (mod frontmatter) and ensure only the intended public API is re-exported from the crate root (e.g., pub use crate::frontmatter::parse_frontmatter; or create explicit public wrapper functions) and adjust any internal item visibilities as needed; also update the other occurrence referenced around lines 53-54 to remove pub visibility so internals are not exported, keeping only the specific symbols you want exposed via explicit pub re-exports or wrapper functions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/lib.rs`:
- Around line 30-31: The frontmatter module is unintentionally public; change
its declaration from pub mod frontmatter to a private module (mod frontmatter)
and ensure only the intended public API is re-exported from the crate root
(e.g., pub use crate::frontmatter::parse_frontmatter; or create explicit public
wrapper functions) and adjust any internal item visibilities as needed; also
update the other occurrence referenced around lines 53-54 to remove pub
visibility so internals are not exported, keeping only the specific symbols you
want exposed via explicit pub re-exports or wrapper functions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2474f93b-30e9-4161-9988-d530565fc9d0
📒 Files selected for processing (2)
src/lib.rssrc/main.rs
Documents that begin with a YAML frontmatter block (starting with --- and ending with --- or ...) now have that block preserved exactly while the remainder of the document is formatted normally.
Changes:
The frontmatter block is shielded from all transforms including --breaks, --wrap, --renumber, and table reflow.
Summary by Sourcery
Preserve leading YAML frontmatter blocks unchanged while continuing to format the rest of Markdown documents, and document this behavior in the user and architecture docs.
New Features:
Enhancements:
Documentation:
Tests:
Chores: