Skip to content

Fix backslash-ended code span wrapping (#349) - #351

Merged
leynos merged 5 commits into
mainfrom
issue-349-wrap-rewraps-compliant-bullets-with-code-spans-into-md013-violations
Jun 5, 2026
Merged

Fix backslash-ended code span wrapping (#349)#351
leynos merged 5 commits into
mainfrom
issue-349-wrap-rewraps-compliant-bullets-with-code-spans-into-md013-violations

Conversation

@lodyai

@lodyai lodyai Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Summary

This branch fixes issue #349 by treating closing backtick fences inside inline
code spans as literal CommonMark closers even when the preceding code content
ends with a backslash. This prevents Windows-style paths such as
C:\Program Files\...\bin\ from being misclassified as open spans and then
rewrapped into MD013-length continuation lines.

It also adds a defensive width guard around inline-code tail carry handling so
that moving an inline-code fragment onto a following content line cannot exceed
the configured wrap width.

Closes #349.

Review walkthrough

Validation

  • make check-fmt: passed
  • make lint: passed
  • make test: passed
  • coderabbit review --agent: passed with findings: 0

Notes

The second commit only moves tokenizer unit tests into sibling test modules so
that the touched tokenizer source files remain below the repository line-count
target. It does not change production behaviour.

Summary by Sourcery

Fix inline code span tokenization and wrapping so backslash-terminated code spans are parsed and wrapped correctly while preserving configured line width.

Bug Fixes:

  • Treat closing backtick fences inside inline code spans as literal closers even when span content ends with a backslash, preventing misclassification and incorrect rewrapping of paths and similar spans.
  • Guard inline-code tail carry between wrapped lines with a width check so moving code fragments onto following lines cannot exceed the configured wrap width.

Enhancements:

  • Centralize closing-fence detection for inline code spans through shared helpers used by both the tokenizer and continuation scanner, and relocate tokenizer tests into dedicated modules for clearer structure.

Documentation:

  • Document closing-fence detection semantics and width-aware inline-code tail carrying in the developer guide, including references to the relevant helper functions.

Tests:

  • Add unit, property-based, and snapshot tests exercising backslash-terminated code spans, inline-code tail carrying against wrap width, and refactored tokenizer scanning and parsing helpers.

leynos added 2 commits June 5, 2026 12:36
Treat matching backtick runs inside code span content as closing
fences even when preceded by a backslash. This follows CommonMark and
prevents Windows paths ending in `\` from being parsed as open spans.

Guard inline-code tail carry so moving a span onto a continuation line
cannot exceed the configured wrap width. Add regression and property
coverage for backslash-terminated inline code in wrapped bullets.
Move tokenizer and scanning unit tests out of the production modules
so the touched source files stay below the repository line-count target.
Keep the test coverage unchanged while making each moved test focused.
@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

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

Walkthrough

Simplify inline-code fence detection to match closing backtick runs by delimiter length only, and make whitespace-only inline-line merging width-aware so inline-code tails are not carried onto continuation lines when that would exceed the configured width. Reorganise and extend tests and fixtures to validate these behaviours.

Changes

Backtick fence simplification and width-aware wrapping

Layer / File(s) Summary
Tokenizer backtick fence simplification
src/wrap/tokenize/mod.rs, src/wrap/tokenize/parsing.rs, src/wrap/tokenize/scanning.rs
Remove odd-backslash-escape parity guards; handle_backtick_fence drops the bytes parameter and uses position_after_close to resolve closing fence runs.
Inline wrapping width-aware carry logic
src/wrap/inline.rs, src/wrap/inline/postprocess.rs
Add helpers to find the next non-whitespace content line and to check if carrying an inline-code tail plus a space fits the configured width. Update merge_whitespace_only_lines to accept width and require the width check before carrying single-space lines.
Test infrastructure reorganisation
src/wrap/tokenize/mod.rs, src/wrap/tokenize/scanning.rs
Move inline tokeniser and scanning helper tests into external test modules via #[path = "..."] mod tests;.
Comprehensive tests, fixtures and property tests
src/wrap/tokenize/mod_tests.rs, src/wrap/tokenize/scanning_tests.rs, src/wrap/inline/postprocess_tests.rs, tests/data/bullet_backslash_code_span_*, tests/wrap/lists.rs, tests/wrap_properties.rs, tests/wrap_unit/code_spans.rs
Add and update unit/property/integration tests covering multibyte tokenisation, escaped backticks, CRLF handling, fence-state detection, width-aware postprocessing, Windows backslash path fixtures, idempotent wrapping, and per-line width assertions.

Sequence Diagram(s)

sequenceDiagram
  participant InlineWrapper as Inline Wrapper
  participant MergeWhitespace as merge_whitespace_only_lines
  participant WidthCheck as Width-fit checker
  participant NextContent as Next content line
  InlineWrapper->>MergeWhitespace: lines, width
  MergeWhitespace->>WidthCheck: "can code tail + space fit on next content line?"
  WidthCheck->>NextContent: compute rendered width of nextContent + tail + space
  NextContent-->>WidthCheck: width status
  alt Fits within width
    WidthCheck-->>MergeWhitespace: allow carry
  else Exceeds width
    WidthCheck-->>MergeWhitespace: prevent carry
  end
  MergeWhitespace-->>InlineWrapper: merged lines
Loading

Possibly related PRs

  • leynos/mdtablefix#266: Modifies merge_whitespace_only_lines-adjacent behaviour; related to whitespace-carry logic adjustments.

Backticks lose their parity chain,
Width checks stop the tail from moving again,
Windows paths stay wrapped to the line,
Tests keep the fences and widths in time. 🎯

You are an AI assistant for code review. Provide targeted, factual guidance and flag places that might need attention: tokeniser changes that removed escape-parity checks (verify intended semantics and regression scope), the new width-dependent merge behaviour (confirm width calculation matches displayed Unicode width), and the extended tests/fixtures (ensure CI runs all new tests). 
🚥 Pre-merge checks | ✅ 17 | ❌ 3

❌ Failed checks (3 warnings)

Check name Status Explanation Resolution
Developer Documentation ⚠️ Warning The PR modifies internal APIs (merge_whitespace_only_lines signature, handle_backtick_fence signature, closing-fence escape logic) without documenting these changes in the developer's guide. Document the width parameter for merge_whitespace_only_lines, the handle_backtick_fence signature change, and removal of backslash-escape checks from fence-closing logic in docs/developers-guide.md.
Testing (Compile-Time / Ui) ⚠️ Warning New wrapping test lacks insta snapshot. Established pattern in codebase: similar tests like spanning_code_span_fixtures combine fixture files with snapshot assertions; this test omits snapshots. Add insta snapshot assertion to test_wrap_bullet_backslash_terminated_code_span_idempotent, following the pattern used by other code-span regression tests.
Observability ⚠️ Warning New width constraint in inline_code_tail_carry_fits() lacks observability. Similar width decisions in continuation.rs use trace logging; this critical algorithmic decision is unlogged. Add trace logging at the inline_code_tail_carry_fits decision point with constraint evaluation, projected width, and target width, matching the pattern in src/wrap/continuation.rs.
✅ Passed checks (17 passed)
Check name Status Explanation
Title check ✅ Passed The title directly addresses the issue being fixed (#349) and accurately summarises the primary change: correcting the wrapping behaviour for backslash-terminated inline code spans.
Linked Issues check ✅ Passed The PR fully addresses the requirements from issue #349 by fixing backtick fence parsing for backslash-terminated code spans and implementing width-aware carry handling to prevent continuation lines exceeding the configured wrap width.
Out of Scope Changes check ✅ Passed All changes remain tightly scoped to issue #349: tokenizer fence parsing corrections, inline-code tail width guards, test data updates, and test module reorganisation for line-count management.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Testing (Overall) ✅ Passed Unit tests validate backslash code-spans and width-guard logic. Regression test replicates issue #349 with idempotence. Property tests verify width compliance. All fail if implementations removed.
User-Facing Documentation ✅ Passed Bug fix addressing issue #349 restores documented behaviour (wrapping respects target width); fixes existing code to match specification already documented in users-guide.md.
Module-Level Documentation ✅ Passed All Rust modules in the PR—both new (mod_tests.rs, scanning_tests.rs) and modified—carry module-level docstrings explaining their purpose, utility, and relationships to other components.
Testing (Unit And Behavioural) ✅ Passed Behavioural tests exercise wrap_text API with issue #349 fixtures; unit tests verify width-guard and tokenisation logic; tests validate fence-matching invariants critical to the fix.
Testing (Property / Proof) ✅ Passed Proptest property test verifies width constraint (output ≤ WRAP_COLS) and idempotence for code-span carry logic using generative path segments across variable-length inputs.
Unit Architecture ✅ Passed Width threaded explicitly; helpers have single responsibilities; fallibility declared via Option; bytes removed; mutations visible via &mut; tests verify constraints.
Domain Architecture ✅ Passed Width parameter is a core domain concept (part of wrap_text public API), not infrastructure. Changes correctly thread width through helpers to enforce line-length invariants.
Security And Privacy ✅ Passed No secrets, credentials, injection risks, unsafe operations, or privacy exposures. Changes are text-wrapping logic with width validation and test data using only placeholder paths.
Performance And Resource Use ✅ Passed Postprocessing adds O(n) next_content_line scans per whitespace line; bounded in practice. Tokenizer improved by finding first match. No unbounded allocations or I/O introduced.
Concurrency And State ✅ Passed PR contains no concurrency code: zero async/await, locks, channels, shared mutable state, or parallelism detected. Changes are single-threaded text-processing with local-scoped mutation only.
Architectural Complexity And Maintainability ✅ Passed Three private helpers implement width-aware inline-code tail carry logic. No traits, registries, layers, or dependencies added. Complexity proportional to the bug fix.
Rust Compiler Lint Integrity ✅ Passed No lint suppressions or unused code found. Clones in postprocess.rs (line 117) and inline.rs (line 242) are justified: necessary for ownership semantics and data splitting respectively.
Description check ✅ Passed The pull request description comprehensively covers the changeset, explaining the specific issue (#349), the fix for backslash-terminated code spans, the width guard for inline-code tail handling, and the test coverage added.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

📋 Issue Planner

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

View plan used: #349

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-349-wrap-rewraps-compliant-bullets-with-code-spans-into-md013-violations

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

@sourcery-ai

sourcery-ai Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Adjusts inline code span tokenization to treat backslashes inside spans as literal when detecting closing fences, and adds a width-aware guard around inline-code tail carrying during whitespace-merge postprocessing, plus test refactors and new regression/property tests for backslash-terminated code spans and wrapping.

Flow diagram for updated inline code span closing fence detection

flowchart TD
    A[segment_inline] --> B[handle_backtick_fence]
    B --> C[position_after_close]
    C -->|closing fence found| D[closing_fence_end]
    C -->|no closing fence| E[collect_range returns opener only]
    D --> F[next_token]
    F --> G[is_closed_inline_code_span]
    G --> H[scan_code_suffix_end]
    H --> I[Token::Code]
    C --> J[reject closers in longer backtick runs]
Loading

Flow diagram for width-aware inline-code tail carry in postprocessing

flowchart TD
    A[wrap_preserving_code] --> B[wrap_first_fit]
    B --> C[merge_whitespace_only_lines]
    C --> D{line is single space and next_starts_atomic is false}
    D -->|yes| E[inline_code_tail_carry_fits]
    E -->|fits| F[carry_previous_inline_code_tail]
    F --> G[rebalance_atomic_tails]
    E -->|does not fit| G
    D -->|no| G
Loading

File-Level Changes

Change Details Files
Unify closing backtick fence detection via position_after_close and treat backslashes inside inline code spans as literal for closing-fence purposes.
  • Refactor handle_backtick_fence to no longer take the precomputed byte slice and instead delegate closing detection to position_after_close using the opener fence length.
  • Update next_token to use position_after_close when extracting Token::Code spans from a line, reusing the same closing-fence logic as the inline tokenizer.
  • Remove has_odd_backslash_escape_bytes checks from closing_fence_end and scan_continuation_span_state so backslashes inside code-span content no longer hide closing fences.
src/wrap/tokenize/mod.rs
src/wrap/tokenize/parsing.rs
src/wrap/tokenize/scanning.rs
Make whitespace-only-line merging width-aware so inline-code tail carries cannot push continuation lines past the configured wrap width.
  • Introduce next_content_line and inline_code_tail_carry_fits helpers to locate the next non-whitespace line and compute the projected width after carrying an inline-code tail across a single-space artefact.
  • Extend merge_whitespace_only_lines to accept the wrap width and consult inline_code_tail_carry_fits before attempting to move an inline-code tail onto the following line.
  • Update wrap_preserving_code to pass its width into merge_whitespace_only_lines and adjust existing postprocess tests to provide a width parameter, adding a regression test that forbids carries which would exceed the width.
src/wrap/inline/postprocess.rs
src/wrap/inline.rs
src/wrap/inline/postprocess_tests.rs
Expand and reorganize tokenizer and wrapping tests, including regression and property coverage for backslash-terminated code spans and list items.
  • Move tokenizer unit tests from inline modules into dedicated mod_tests.rs and scanning_tests.rs files while preserving coverage, and slightly restructure some rstest-based cases.
  • Add a Token::Code regression test for a backslash-terminated code span and a list-wrapping regression that asserts idempotent wrapping and snapshot output for bullets containing backslash-terminated code spans.
  • Extend wrap_properties with a path_segment_strategy-based property test that generates Windows-style paths inside inline code spans and asserts both idempotence and max-width adherence.
  • Document the new responsibilities of position_after_close and handle_backtick_fence, and describe width-aware inline-code carries in the developer guide table and invariants section.
src/wrap/tokenize/mod_tests.rs
src/wrap/tokenize/scanning_tests.rs
tests/wrap_unit/code_spans.rs
tests/wrap/lists.rs
tests/wrap_properties.rs
tests/data/bullet_backslash_code_span_input.txt
tests/data/bullet_backslash_code_span_expected.txt
tests/snapshots/bullet_backslash_terminated_code_span.snap
docs/developers-guide.md

Assessment against linked issues

Issue Objective Addressed Explanation
#349 Ensure --wrap respects the configured line-width limit (default 80 columns) when rewrapping list items with inline-code spans, so that already MD013-compliant bullets are not rewritten into MD013 violations (e.g., the provided reproducer remains a no-op).
#349 Correct handling of backslash-terminated inline code spans (e.g., Windows-style paths ending in \ inside backticks) so that they are tokenized as closed code spans and do not cause incorrect wrapping behavior.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@leynos
leynos marked this pull request as ready for review June 5, 2026 11:50

@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 @leynos, you have reached your weekly rate limit of 2500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@coderabbitai coderabbitai Bot added the Issue label Jun 5, 2026

@coderabbitai coderabbitai 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.

Actionable comments posted: 3

🤖 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/wrap/tokenize/mod.rs`:
- Around line 219-230: Replace the current naive finder that closes on the first
fence occurrence in the tokenize logic with the same checking used by the
scanner: call position_after_close on rest[delim_len..] and, if Some, compute
raw_end = delim_len + relative_end and candidate = raw_end - delim_len, then
proceed to compute token/suffix_end (using is_closed_inline_code_span and
scan_code_suffix_end) and return Token::Code as before; also apply the identical
change in handle_backtick_fence so both tokenizers use position_after_close and
therefore reject closers embedded in longer backtick runs.

In `@tests/wrap_properties.rs`:
- Around line 119-124: The test currently uses the literal 80 in calls to
wrap_text and in the width assertion; replace those literals with the WRAP_COLS
constant so the property uses the configured wrap width. Update the two
wrap_text(&input, 80) and wrap_text(&output, 80) calls to wrap_text(&input,
WRAP_COLS) and wrap_text(&output, WRAP_COLS), and change the
UnicodeWidthStr::width check to compare against WRAP_COLS instead of 80; ensure
WRAP_COLS is imported into the test module so wrap_text, output, and
UnicodeWidthStr::width use the single source of truth.

In `@tests/wrap/lists.rs`:
- Around line 161-165: Replace the hard-coded 80 literals with the test-wide
WRAP_COLS constant: change both wrap_text(&input, 80) and wrap_text(&output, 80)
to use WRAP_COLS, and ensure WRAP_COLS is imported into the test module (so the
test uses the repository-wide wrap width). Keep the call to
assert_wrapped_list_item(&output[2..4], "- ", 2) unchanged.
🪄 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: c3260cd1-9585-4e25-abe3-aff33fde27ec

📥 Commits

Reviewing files that changed from the base of the PR and between 8b79519 and a11726b.

📒 Files selected for processing (13)
  • src/wrap/inline.rs
  • src/wrap/inline/postprocess.rs
  • src/wrap/inline/postprocess_tests.rs
  • src/wrap/tokenize/mod.rs
  • src/wrap/tokenize/mod_tests.rs
  • src/wrap/tokenize/parsing.rs
  • src/wrap/tokenize/scanning.rs
  • src/wrap/tokenize/scanning_tests.rs
  • tests/data/bullet_backslash_code_span_expected.txt
  • tests/data/bullet_backslash_code_span_input.txt
  • tests/wrap/lists.rs
  • tests/wrap_properties.rs
  • tests/wrap_unit/code_spans.rs

Comment thread src/wrap/tokenize/mod.rs Outdated
Comment thread tests/wrap_properties.rs Outdated
Comment thread tests/wrap/lists.rs Outdated
leynos added 2 commits June 5, 2026 15:50
Route public and internal inline-code token parsing through
`position_after_close` so both paths reject closing fences embedded in
longer backtick runs.

Use `WRAP_COLS` in the issue #349 regression and property tests so the
coverage follows the repository wrap-width constant.
Document the internal tokenizer and inline post-processing contracts that
changed for issue #349.

Add snapshot coverage for the backslash-terminated bullet regression and
trace the width decision that prevents inline-code tail carries from
exceeding the configured wrap width.
@leynos

leynos commented Jun 5, 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.

❌ Failed checks (3 warnings)

Check name Status Explanation Resolution
Developer Documentation ⚠️ Warning The PR modifies internal APIs (merge_whitespace_only_lines signature, handle_backtick_fence signature, closing-fence escape logic) without documenting these changes in the developer's guide. Document the width parameter for merge_whitespace_only_lines, the handle_backtick_fence signature change, and removal of backslash-escape checks from fence-closing logic in docs/developers-guide.md.
Testing (Compile-Time / Ui) ⚠️ Warning New wrapping test lacks insta snapshot. Established pattern in codebase: similar tests like spanning_code_span_fixtures combine fixture files with snapshot assertions; this test omits snapshots. Add insta snapshot assertion to test_wrap_bullet_backslash_terminated_code_span_idempotent, following the pattern used by other code-span regression tests.
Observability ⚠️ Warning New width constraint in inline_code_tail_carry_fits() lacks observability. Similar width decisions in continuation.rs use trace logging; this critical algorithmic decision is unlogged. Add trace logging at the inline_code_tail_carry_fits decision point with constraint evaluation, projected width, and target width, matching the pattern in src/wrap/continuation.rs.

@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== docs/developers-guide.md: relevant sections ==="
grep -n -i "merge_whitespace_only_lines\|handle_backtick_fence\|backslash\|escape\|width" docs/developers-guide.md | head -60

echo ""
echo "=== tests/wrap/lists.rs: test_wrap_bullet_backslash ==="
grep -n -A 30 "test_wrap_bullet_backslash" tests/wrap/lists.rs

echo ""
echo "=== src/wrap/inline/postprocess.rs: inline_code_tail_carry_fits ==="
grep -n -A 20 "inline_code_tail_carry_fits\|trace\|log" src/wrap/inline/postprocess.rs

echo ""
echo "=== src/wrap/continuation.rs: trace logging pattern for reference ==="
grep -n "trace\!" src/wrap/continuation.rs | head -20

Length of output: 10578



@leynos All three warnings are resolved in the current branch.

Check Status Evidence
Developer Documentation ✅ Resolved docs/developers-guide.md now documents the width parameter for merge_whitespace_only_lines (lines 232–236, 387–391), the handle_backtick_fence delegation to position_after_close (line 324), and the removal of backslash-escape checks from fence-closing logic (lines 381–383).
Testing (insta snapshot) ✅ Resolved test_wrap_bullet_backslash_terminated_code_span_idempotent now includes an insta::assert_snapshot! call (with snapshot_path and prepend_module_to_snapshot settings) matching the established pattern.
Observability ✅ Resolved inline_code_tail_carry_fits emits trace! events on every code path — three early-return guards and the main decision point — reporting fits, projected_width, target_width, previous_tail_width, and next_line_width, consistent with the pattern in src/wrap/continuation.rs.

No further work is required for these items.


🧠 Learnings used
Learnt from: CR
Repo: leynos/dear-diary PR: 0
File: docs/adr-001-whitaker-lint-contract.md:0-0
Timestamp: 2026-05-23T12:12:21.541Z
Learning: Ensure Rustdoc, Clippy, and Whitaker findings are resolved before pull requests reach review

Learnt from: CR
Repo: leynos/rust-skill PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-05-29T00:19:17.815Z
Learning: Reviewers should ask whether the change makes unused code easier or harder for rustc and Clippy to detect. If harder, request a narrower module boundary, removal of stale code, or properly justified #[expect(...)] tied to planned work before merge.

Learnt from: CR
Repo: leynos/rust-prover-tools PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-05-24T02:45:37.463Z
Learning: Reviewers should ask whether the change makes unused code easier or harder for `rustc` and Clippy to detect. If the answer is 'harder', request a narrower module boundary, removal of stale code, or a properly justified `#[expect(...)]` tied to planned work before merge.

Learnt from: CR
Repo: leynos/df12-documentation-skills PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-05-29T00:20:49.681Z
Learning: Reviewers should ask whether the change makes unused code easier or harder for `rustc` and Clippy to detect. If the answer is 'harder', request a narrower module boundary, removal of stale code, or a properly justified `#[expect(...)]` tied to planned work before merge.

Learnt from: CR
Repo: leynos/tei-rapporteur PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-05-25T12:07:25.566Z
Learning: Reviewers should ask whether the change makes unused code easier or harder for `rustc` and Clippy to detect. If the answer is "harder", request a narrower module boundary, removal of stale code, or a properly justified `#[expect(...)]` tied to planned work before merge.

Learnt from: CR
Repo: leynos/spycatcher-harness PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-05-24T21:29:27.390Z
Learning: Reviewers should ask whether the change makes unused code easier or harder for `rustc` and Clippy to detect. If the answer is 'harder', request a narrower module boundary, removal of stale code, or a properly justified `#[expect(...)]` before merge.

Learnt from: CR
Repo: leynos/css-view PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-06-03T00:35:46.625Z
Learning: Applies to **/*.rs : Ask whether the change makes unused code, stale helpers, and unnecessary ownership work easier or harder for `rustc`, Clippy, and human reviewers to detect. If the answer is 'harder', request a narrower module boundary, removal of stale code, removal or justification of unnecessary clones, or a properly justified `#[expect(...)]` before merge.

Learnt from: CR
Repo: leynos/falcon-correlate PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-05-31T20:26:11.798Z
Learning: Reviewers should ask whether the change makes unused code, stale helpers, and unnecessary ownership work easier or harder for `rustc`, Clippy, and human reviewers to detect; if the answer is 'harder', request a narrower module boundary, removal of stale code, removal or justification of unnecessary clones, or a properly justified `#[expect(...)]` tied to planned work before merge.

Learnt from: CR
Repo: leynos/stilyagi PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-06-04T21:53:21.943Z
Learning: Applies to **/*.rs : Ask whether the change makes unused code, stale helpers, and unnecessary ownership work easier or harder for `rustc`, Clippy, and human reviewers to detect; if harder, request a narrower module boundary, removal of stale code, removal or justification of unnecessary clones, or proper `#[expect(...)]` justification

Learnt from: CR
Repo: leynos/wireframe PR: 0
File: docs/formal-verification-methods-in-wireframe.md:0-0
Timestamp: 2026-05-19T22:44:22.416Z
Learning: Applies to docs/src/frame/**/*.rs : Add Kani harnesses for unsupported-width behavior to serve as a regression harness after the project decides whether widths 3, 5, 6, 7 are valid or invalid

Learnt from: CR
Repo: leynos/mdtablefix PR: 0
File: docs/users-guide.md:0-0
Timestamp: 2026-06-02T12:18:56.824Z
Learning: Applies to docs/**/*.{md,markdown} : When joining split inline code spans across soft-wrapped source lines with `--wrap`, treat the joined span as an indivisible unit and never split the closing backtick onto a different line

Learnt from: CR
Repo: leynos/mdtablefix PR: 0
File: docs/users-guide.md:0-0
Timestamp: 2026-06-02T12:18:56.824Z
Learning: Applies to docs/**/*.{md,markdown} : Keep inflectional affixes and possessives on the same line as the closing inline code fence during wrapping (e.g., `` `VarGuard`s ``, `` `class`'s ``, `` `fetch`ed ``)

Learnt from: CR
Repo: leynos/mdtablefix PR: 0
File: docs/users-guide.md:0-0
Timestamp: 2026-06-02T12:18:56.824Z
Learning: Applies to docs/**/*.{md,markdown} : Keep opening brackets and punctuation (`(`, `[`, and CJK openers) coupled to the following inline code span or Markdown link during wrapping to prevent lone openers from being stranded

Learnt from: CR
Repo: leynos/mdtablefix PR: 0
File: docs/developers-guide.md:0-0
Timestamp: 2026-06-05T11:51:09.121Z
Learning: Applies to docs/{src/wrap/tokenize/scanning.rs,src/wrap/inline/fragment.rs} : Inflectional affixes (`s`, `'s`, `ed`, `ing`) and hyphenated compounds that immediately follow a closed backtick fence must be absorbed into the code token by `scan_code_suffix_end`, and the combined token must be recognized as atomic by `has_inline_code_structure` so wrapping treats the full string as one unit

Learnt from: CR
Repo: leynos/mdtablefix PR: 0
File: docs/users-guide.md:0-0
Timestamp: 2026-06-02T12:18:56.824Z
Learning: Applies to docs/**/*.{md,markdown} : Preserve hard line breaks (two trailing spaces at the end of a line) on the final wrapped line when using `mdtablefix --wrap` so hard-break semantics are not lost after reformatting

Learnt from: CR
Repo: leynos/mdtablefix PR: 0
File: docs/users-guide.md:0-0
Timestamp: 2026-06-02T12:18:56.824Z
Learning: Applies to docs/**/*.{md,markdown} : Keep hyphenated compounds where a word ends with a hyphen immediately before an inline code span on the same line during wrapping (e.g., `` pre-`LLMPort` ``)

Learnt from: CR
Repo: leynos/mdtablefix PR: 0
File: docs/developers-guide.md:0-0
Timestamp: 2026-06-05T11:51:09.121Z
Learning: Fenced code blocks must pass through unchanged; every line inside an open fence must preserve its byte content verbatim so post-wrap transforms such as `--ellipsis`, `--renumber`, `--breaks`, and `--fences` cannot mutate fenced code block bodies

Learnt from: CR
Repo: leynos/mdtablefix PR: 0
File: docs/trailing-spaces.md:0-0
Timestamp: 2026-04-23T22:07:14.054Z
Learning: Applies to docs/**/*.rs : The textwrap-backed inline wrapper preserves trailing spaces on the final line, allowing Markdown hard breaks (two spaces at end of line) to be retained during text wrapping

Learnt from: CR
Repo: leynos/mdtablefix PR: 0
File: docs/architecture.md:0-0
Timestamp: 2026-06-01T23:44:02.797Z
Learning: Applies to docs/**/*.rs : Group trailing punctuation that follows an inline code span with the code when wrapping to prevent full stops from splitting onto a new line, preserving code span readability

Learnt from: CR
Repo: leynos/mdtablefix PR: 0
File: docs/users-guide.md:0-0
Timestamp: 2026-06-02T12:18:56.824Z
Learning: Applies to docs/**/*.{md,markdown} : Preserve outer fence delimiters when normalization would turn an inner literal fence into a structural close to maintain the integrity of nested code blocks

@leynos

leynos commented Jun 5, 2026

Copy link
Copy Markdown
Owner

@coderabbitai pause

@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Reviews paused.

@leynos

leynos commented Jun 5, 2026

Copy link
Copy Markdown
Owner

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 5, 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 commented Jun 5, 2026

Copy link
Copy Markdown
Owner

@sourcery-ai review

sourcery-ai[bot]

This comment was marked as resolved.

Add focused coverage for backslash-terminated inline code spans through
`segment_inline`, and assert that wrapped output keeps those spans closed
and contiguous.

Cover the exact-width boundary for inline-code tail carries so the width
guard cannot regress into an off-by-one rejection.
@leynos

leynos commented Jun 5, 2026

Copy link
Copy Markdown
Owner

@coderabbitai Have the following now been resolved?

Annul any requirements that violate the en-GB-oxendict spelling (-ize / -yse / -our) conventions (for example a request to replace "normalize" with "normalise" or "artefact" with "artifact"), or where the requirement unnecessarily increases cyclomatic complexity.

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="tests/wrap_unit/code_spans.rs" line_range="73-71" />
<code_context>
 #[test]
 fn merge_keeps_content_lines_unchanged() {
     let lines = vec![vec![fragment("hello")], vec![fragment("world")]];
-    assert_eq!(merge_whitespace_only_lines(&lines), lines);
+    assert_eq!(merge_whitespace_only_lines(&lines, 80), lines);
 }

 #[test]
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a `segment_inline` regression alongside the `tokenize_markdown` one for backslash‑terminated code spans

This currently exercises only `tokenize_markdown`. To cover the regression end‑to‑end, please also add a case that runs the same input through `segment_inline` and asserts that it yields a single code token (e.g. for `segment_inline(r"Install to `C:\\path\\bin\\` and add")`). That way we verify both tokenizer and inline segmentation behavior for this scenario.

Suggested implementation:

```rust
use mdtablefix::wrap::{Token, tokenize_markdown, segment_inline, wrap_text};

```

```rust
    assert_eq!(code_tokens.len(), 1);
    assert!(matches!(
        code_tokens[0],
        Token::Code { .. }
    ));
}

#[test]
fn test_segment_inline_backslash_terminated_code_span() {
    let tokens = segment_inline(r"Install to `C:\path\bin\` and add");
    let code_tokens = tokens
        .iter()
        .filter(|token| matches!(token, Token::Code { .. }))
        .collect::<Vec<_>>();

    assert_eq!(code_tokens.len(), 1);
    assert!(matches!(
        code_tokens[0],
        Token::Code { .. }
    ));
}

```
</issue_to_address>

### Comment 2
<location path="src/wrap/inline/postprocess_tests.rs" line_range="54-60" />
<code_context>
 #[test]
 fn merge_keeps_content_lines_unchanged() {
     let lines = vec![vec![fragment("hello")], vec![fragment("world")]];
</code_context>
<issue_to_address>
**suggestion (testing):** Add a borderline case test where the projected inline-code tail width is exactly equal to the wrap width

You already cover the `projected_width > width` path; please also add a case where `projected_width == width` and the carry is allowed. For example, choose fragment widths so that `previous_tail.width + 1 + next_line_width == width`, and assert the tail is moved. This helps guard against off‑by‑one regressions in `inline_code_tail_carry_fits`.

Suggested implementation:

```rust
#[test]
fn inline_code_tail_carry_fits_when_projected_width_equals_wrap_width() {
    // previous line tail "aaaa" (width 4)
    // next line "bbbb" (width 4)
    // projected_width = 4 (tail) + 1 (space) + 4 (next) = 9
    let lines = vec![
        vec![fragment("aaaa")],
        vec![fragment("bbbb")],
    ];

    let merged = merge_whitespace_only_lines(&lines, 9);

    // When projected_width == width, the tail carry should be allowed and the lines merged.
    assert_eq!(
        merged,
        vec![vec![fragment("aaaa"), fragment("bbbb")]]
    );
}

```

Depending on the exact behavior of `merge_whitespace_only_lines` and how inline-code tails are represented in your test helpers, you may want to:

1. Replace `"aaaa"` and `"bbbb"` with whatever fragments actually represent an inline-code tail and the following content (for example, inline-code fragments like `` "`code`" `` and text fragments).
2. If tail-carry only happens across a whitespace-only line, change `lines` to include that intermediate whitespace-only line and adjust the expected output accordingly, e.g.:

```rust
let lines = vec![
    vec![inline_code_tail_fragment("code")],
    vec![whitespace_fragment(" ")],
    vec![text_fragment("next")],
];
```

and then assert the merged single line matches your actual fragment constructors and spacing behavior.

3. Ensure the chosen fragment contents (or explicit widths, if you have width-aware constructors) satisfy `previous_tail.width + 1 + next_line_width == width` according to your real width calculation (accounting for backticks, styling markers, etc.).
</issue_to_address>

### Comment 3
<location path="tests/wrap_properties.rs" line_range="64-57" />
<code_context>
 #[test]
 fn merge_keeps_content_lines_unchanged() {
     let lines = vec![vec![fragment("hello")], vec![fragment("world")]];
-    assert_eq!(merge_whitespace_only_lines(&lines), lines);
+    assert_eq!(merge_whitespace_only_lines(&lines, 80), lines);
 }

 #[test]
</code_context>
<issue_to_address>
**suggestion (testing):** Strengthen the property test to assert that backslash-terminated code spans remain intact and properly closed

The existing property already checks idempotence and width, but it would help to also assert that the inline code span remains a single, closed span after wrapping. For example, for each output line you could:
- count backticks and assert they’re even, and/or
- assert that the `` `C:\\...\\bin\\` `` substring (or equivalent pattern: backtick + path + trailing backslash + backtick) is still present as one contiguous span.
This directly guards against the original misclassification of a backslash‑terminated span as left open during wrapping.

Suggested implementation:

```rust
fn path_segment_strategy() -> impl Strategy<Value = String> {
    "[A-Za-z][A-Za-z0-9]{1,10}".prop_map(String::from)
}

/// Assert that:
/// 1. Every line has an even number of backticks (no inline code span left open on that line),
/// 2. The expected inline code span is still present as a single, contiguous substring
///    in the wrapped text (guards against splitting the code span across lines).
fn assert_closed_backslash_terminated_code_span<L, S>(lines: L, expected_span: &str)
where
    L: IntoIterator<Item = S>,
    S: AsRef<str>,
{
    let mut joined = String::new();

    for line in lines.into_iter() {
        let line_ref = line.as_ref();

        // Each line must have an even number of backticks so that we don't
        // mis-classify a backslash-terminated span as "left open".
        let backtick_count = line_ref.chars().filter(|&c| c == '`').count();
        assert!(
            backtick_count % 2 == 0,
            "line {:?} has an odd number of backticks ({}) after wrapping",
            line_ref,
            backtick_count
        );

        if !joined.is_empty() {
            joined.push('\n');
        }
        joined.push_str(line_ref);
    }

    // The backslash-terminated inline code span must remain a single, contiguous span.
    assert!(
        joined.contains(expected_span),
        "wrapped text no longer contains the expected inline code span {:?}",
        expected_span
    );
}

proptest! {

```

To fully implement your suggestion inside `wrap_text_backslash_terminated_code_spans_stay_within_width`, update the property body to:

1. Construct the original inline code span string, including the trailing backslash, e.g.:

```rust
let code_span = format!("`C:\\\\{}\\\\{}\\\\{}\\\\bin\\\\`", vendor, product, scope);
```

(or whatever exact path format your test currently uses).

2. After you call `wrap_text` (or your wrapping function) and obtain the wrapped lines (e.g. `let wrapped_lines: Vec<String> = wrap_text(&code_span, width);`), invoke:

```rust
assert_closed_backslash_terminated_code_span(&wrapped_lines, &code_span);
```

3. Keep the existing idempotence and width assertions as they are; this helper is meant to be an additional assertion layer.

Because I cannot see the current body of `wrap_text_backslash_terminated_code_spans_stay_within_width`, you’ll need to insert the `code_span` construction and the `assert_closed_backslash_terminated_code_span` call at the appropriate place (after wrapping, before the test returns), adjusting variable names and the exact path format to match your existing code.
</issue_to_address>

### Comment 4
<location path="tests/wrap/lists.rs" line_range="156-165" />
<code_context>
 #[test]
 fn merge_keeps_content_lines_unchanged() {
     let lines = vec![vec![fragment("hello")], vec![fragment("world")]];
</code_context>
<issue_to_address>
**suggestion (testing):** Consider asserting the width and code-span integrity for the bullet regression as well

This test already has solid coverage via fixtures, snapshots, idempotence, and list‑shape checks. Since the regression was about MD013 width and code span handling, consider also asserting that all `output` lines are `<= WRAP_COLS` in display width and, optionally, that the list item still contains exactly one backslash‑terminated code span (e.g., by counting backticks or matching the path substring).
</issue_to_address>

@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== tests/wrap_unit/code_spans.rs ==="
cat tests/wrap_unit/code_spans.rs

echo ""
echo "=== src/wrap/inline/postprocess_tests.rs ==="
cat src/wrap/inline/postprocess_tests.rs

echo ""
echo "=== tests/wrap_properties.rs ==="
cat tests/wrap_properties.rs

echo ""
echo "=== tests/wrap/lists.rs ==="
cat tests/wrap/lists.rs

Length of output: 41818


🏁 Script executed:

#!/bin/bash
echo "=== segment_inline visibility in tokenize/mod.rs ==="
grep -n "pub.*segment_inline\|fn segment_inline" src/wrap/tokenize/mod.rs

echo ""
echo "=== wrap/mod.rs re-exports ==="
grep -n "segment_inline\|pub use" src/wrap/mod.rs 2>/dev/null || grep -rn "segment_inline\|pub use" src/wrap.rs 2>/dev/null

echo ""
echo "=== lib.rs re-exports ==="
grep -n "segment_inline\|pub use.*wrap" src/lib.rs

Length of output: 1014


🏁 Script executed:

#!/bin/bash
echo "=== src/wrap/tokenize/mod_tests.rs: backslash / segment_inline coverage ==="
grep -n "backslash\|segment_inline\|C:\\\\\\|bin\\\\" src/wrap/tokenize/mod_tests.rs

echo ""
echo "=== full mod_tests.rs ==="
cat src/wrap/tokenize/mod_tests.rs

Length of output: 4311



@leynos All four comments are now resolved.

# Comment Status Evidence
1 segment_inline regression for backslash-terminated code spans ✅ Resolved segment_inline_handles_backslash_terminated_code_span in src/wrap/tokenize/mod_tests.rs (lines 41–58) covers exactly the suggested input and asserts the single r"\C:\path\bin`" token. The suggested integration-test location (tests/wrap_unit/code_spans.rs) was architecturally infeasible — segment_inlineispub(super)and not re-exported frommdtablefix::wrap, so the unit test in mod_tests.rs` is the correct and only feasible home for this coverage.
2 Borderline projected_width == width case ✅ Resolved merge_moves_inline_code_tail_at_exact_width_boundary in src/wrap/inline/postprocess_tests.rs sets width = fragment("code").width + 1 + fragment("tail").width and asserts the tail is moved.
3 Property test: assert code span stays intact and closed ✅ Resolved wrap_text_backslash_terminated_code_spans_stay_within_width now asserts rendered.contains(&code_span), per-line backtick parity (backtick_count % 2 == 0), and width compliance.
4 Bullet regression: width and code-span integrity assertions ✅ Resolved test_wrap_bullet_backslash_terminated_code_span_idempotent asserts all lines <= WRAP_COLS, checks rendered.contains(r"\C:\Program Files<Vendor><Product>\bin`")`, and asserts total backtick count equals 2.

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.

wrap: rewraps compliant bullets into MD013 violations when an inline-code span sits near the original break

1 participant