Skip to content

Fix cross-line inline code span reflow (#297) - #304

Merged
leynos merged 35 commits into
mainfrom
issue-297-reflow-widens-closing-backtick-onto-its-own-line-and-orphans-words-inside-soft-wrapped-code-spans
May 29, 2026
Merged

Fix cross-line inline code span reflow (#297)#304
leynos merged 35 commits into
mainfrom
issue-297-reflow-widens-closing-backtick-onto-its-own-line-and-orphans-words-inside-soft-wrapped-code-spans

Conversation

@leynos

@leynos leynos commented May 27, 2026

Copy link
Copy Markdown
Owner

Summary

Closes #297

Reflow no longer wraps prefixed lines while an inline code span remains open
across soft-wrapped source lines. Continuations (including blockquote > lines)
are buffered until the span closes, then wrapped as one atomic segment so
closing backticks stay on the same line as their content.

  • Adds has_unclosed_code_span to mirror tokenizer backtick matching.
  • Introduces PendingPrefix deferral in the wrap orchestrator.
  • Closes digit-only spans before plain-text continuations (for example 4.1.1).
  • Adds fixture and regression tests for bullet, ordered, blockquote, and
    footnote cases.

Test plan

  • make check-fmt
  • make lint
  • make test
  • coderabbit review --agent (0 findings)
  • Fixture pairs for issue examples 1 and 2
  • scan_code_spans guard via assert_wrapped_list_item

@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 commented May 27, 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

Buffer prefixed lines that open an unclosed inline-code backtick run into PendingPrefix; join continuations while tracking exact fence lengths and hard-breaks; update span state incrementally; flush buffered prefixed output only when the span closes or a hard-break forces emission.

Changes

Inline code span detection and soft-wrap deferral

Layer / File(s) Summary
Code-span scanning and helpers
src/wrap/tokenize/scanning.rs, src/wrap/tokenize/mod.rs
Add exact-length, unescaped backtick-run detection and closing-fence checks; rewrite parse_open_code_span; add has_unclosed_code_span(), position_after_close(), scan_continuation_span_state() and test-only continuation_begins_with_closing_fence(); expose required helpers as pub(crate) and add deterministic unit tests.
Paragraph contract and prefixed wrapping
src/wrap/paragraph.rs
Introduce PendingPrefix and ParagraphState.pending_prefix; detect open inline spans in handle_prefix_line and buffer pending segments; refactor append_wrapped_with_prefix into append_wrapped_with_prefix_width; implement ensure_trailing_hard_break_on_last_line; make flush_paragraph consume and emit pending-prefix segments with stored first-line width and hard-break handling.
Continuation state machine
src/wrap/continuation.rs
Add continuation handling: apply_continuation_chunk, join_pending_continuation, split_reopen_span, leading_run_needs_space, emit_pending_prefix_segment, update_span_state and SpanStateUpdate; join continuation chunks into the pending buffer, detect close-then-reopen splits, and choose defer/emit/flush actions.
Wrap integration and routing
src/wrap.rs
Add continuation module import and apply_continuation_chunk usage; implement handle_pending_continuation to route lines when state.pending_prefix is set (matching repeatable prefixes, passthrough detection including link reference definitions and link-title window updates, or converting line into continuation chunk with line_break_parts); add #[doc(inline)] to tokenize_markdown and test-visible pub(crate) re-exports for span-test helpers.
Unit/integration tests, fixtures, docs and gitignore
tests/wrap_unit.rs, tests/wrap/spanning_code_spans.rs, tests/wrap/mod.rs, tests/wrap_properties.rs, src/wrap/tests/span_state.rs, tests/data/*, docs/developers-guide.md, docs/users-guide.md, .gitignore
Add rstest and proptest tests for spanning and deferred inline-code behaviour, property tests validating invariants, snapshot fixtures and expected outputs, span-state unit tests, developer and user documentation describing PendingPrefix deferral and cross-line code-span constraints, test-module wiring, and ignore *.proptest-regressions in .gitignore.

Sequence Diagram(s)

sequenceDiagram
  participant PrefixHandler
  participant Scanning
  participant ContinuationModule
  participant ParagraphWriter
  PrefixHandler->>Scanning: parse_open_code_span(prefix_line.rest)
  Scanning-->>PrefixHandler: Option<(open_fence_len, remainder)>
  PrefixHandler->>ContinuationModule: store PendingPrefix and await continuations
  ContinuationModule->>Scanning: scan_continuation_span_state(continuation, fence_len)
  ContinuationModule->>ParagraphWriter: emit_pending_prefix_segment(...) on Flush
Loading
  • Possibly related PRs
    • leynos/mdtablefix#294: Overlaps on passthrough link-reference handling used by pending-continuation logic.
    • leynos/mdtablefix#271: Intersects on paragraph/prefix wrapping pipeline changes that the pending-prefix flow extends.

Hold the backtick, fold the span tight,
Join the next line till the fence is right.
Flush when fences match or two spaces call,
Keep inline code whole—prevent the fall. ✨


Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore

❌ Failed checks (2 errors, 4 warnings)

Check name Status Explanation Resolution
Testing (Overall) ❌ Error Tests are rigorous (10 property tests, 9 unit tests with exact assertions, 2 snapshots), but unaddressed review comment: docs/developers-guide.md line 282 uses "Centralised" instead of "Centralized". Change "Centralised" to "Centralized" on line 282 of docs/developers-guide.md per en-GB-oxendict rules.
Unit Architecture ❌ Error apply_continuation_chunk silently returns when join/update fails, hiding fallibility from callers and violating the requirement that fallibility be explicit in the API. Change return type to bool or enum to make silent-return outcomes explicit; document why silence is correct in docstring.
Developer Documentation ⚠️ Warning Documentation comprehensively covers the new PendingPrefix architecture and cross-line code spans; however, "Centralised" violates en-GB-oxendict spelling rules. Change "Centralised" to "Centralized" in docs/developers-guide.md line 282 per Oxford English Dictionary -ize suffix rules.
Testing (Property / Proof) ⚠️ Warning The PR introduces span-state-machine invariants with only 2-line property tests, missing multi-chunk continuation and complex reopen scenarios. Add proptest for 3+ line continuations, ClosedAndReopened state branch, join_pending_continuation edge cases, and split_reopen_span across all fence/prefix combinations.
Domain Architecture ⚠️ Warning Domain architecture is clean with no infrastructure leakage; docs/developers-guide.md line 282 uses "Centralised" instead of "Centralized". Change "Centralised" to "Centralized" on line 282 of docs/developers-guide.md to comply with -ize suffix rules.
Architectural Complexity And Maintainability ⚠️ Warning Documentation uses incorrect British English spelling: "Centralised" should be "Centralized" per en-GB-oxendict rules (-ize suffix). In docs/developers-guide.md line 282, change "Centralised" to "Centralized" to comply with en-GB-oxendict spelling standards.
✅ Passed checks (14 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix cross-line inline code span reflow (#297)' directly addresses the linked issue #297 and accurately summarises the main change: fixing how inline code spans are reflowed when split across multiple source lines.
Description check ✅ Passed The description comprehensively documents the changes, closing the linked issue and explaining the deferral mechanism, new helpers, and test coverage in relation to the changeset.
Linked Issues check ✅ Passed The code implements all primary objectives from issue #297: detects and buffers unclosed inline code spans via PendingPrefix, treats spans atomically with no inserted breaks, applies behaviour across prefixed contexts (lists, blockquotes, footnotes), enforces exact fence-length matching through new scanning helpers, and preserves hard-break markers.
Out of Scope Changes check ✅ Passed All changes directly support the core objective of fixing cross-line inline code span reflow: new span-state helpers, PendingPrefix deferral mechanism, continuation handling, test coverage, and documentation updates are scoped to issue #297.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
User-Facing Documentation ✅ Passed docs/users-guide.md documents inline code span joining, atomic treatment, and application across bulleted/ordered lists, blockquotes, and footnotes, addressing all issue #297 requirements.
Module-Level Documentation ✅ Passed All new and modified modules carry proper module-level docstrings explaining purpose, utility, and relationships to other components.
Testing (Unit And Behavioural) ✅ Passed Unit tests verify local scanning logic; integration tests exercise public wrap_text API; property tests validate invariants. No tests disguise as unit tests via internal seams.
Testing (Compile-Time / Ui) ✅ Passed Snapshot tests encode code-span atomicity assertions, unit tests verify invariants with exact checks, proptest covers generated scenarios. No compile-time tests required.
Observability ✅ Passed Batch CLI tool with no logging infrastructure. Failures are visible as wrong markdown output. Safe Rust patterns (no unwrap) and comprehensive test coverage provide adequate observability.
Security And Privacy ✅ Passed No secrets, credentials, injection risks, unsafe operations, or bounds violations found. Code performs Markdown text processing with safe bounds checking and no privileged operations.
Performance And Resource Use ✅ Passed Memory is bounded by paragraph boundaries; scanning is O(n) per chunk not O(n²); only one format! call in rare close-reopen paths; no unbounded loops or allocations in hot paths.
Concurrency And State ✅ Passed Single-threaded library with stack-owned state exclusively borrowed via &mut; explicit SpanStateUpdate state machine; comprehensive test coverage; no concurrency primitives or aliasing possible.
Rust Compiler Lint Integrity ✅ Passed All new pub(crate) functions are actively used in production code or properly gated with #[cfg(test)]. No inappropriate lint suppressions found. Module boundaries reflect real use.
📋 Issue Planner

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

View plan used: #297

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-297-reflow-widens-closing-backtick-onto-its-own-line-and-orphans-words-inside-soft-wrapped-code-spans

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

@leynos
leynos marked this pull request as ready for review May 27, 2026 20:30
@coderabbitai coderabbitai Bot added the Issue label May 27, 2026
chatgpt-codex-connector[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

@leynos

leynos commented May 27, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

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

@coderabbitai 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: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
tests/wrap/spanning_code_spans.rs (2)

90-98: 🧹 Nitpick | 🔵 Trivial | 💤 Low value

Add more cases or convert to a regular test.

This rstest has only one case. Add variations (multiple open spans, different prefixes, escaped backticks) or convert to #[test].

🤖 Prompt for 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.

In `@tests/wrap/spanning_code_spans.rs` around lines 90 - 98, The rstest
test_wrap_defers_while_any_span_stays_open currently supplies only one case and
should either be expanded with additional #[case] inputs or converted to a plain
#[test]; update the test to include multiple variations (e.g., multiple open
spans, different leading prefixes, escaped backticks) by adding more #[case]
parameters to the function signature or replace the #[rstest] attribute with
#[test] and iterate through a small vector of inputs that call wrap_text(&input,
80) and assert the same conditions (presence of "`done`" and "`open span`" or
their expected equivalents) so the behavior around spanning inline code is
exercised more thoroughly.

77-88: 🧹 Nitpick | 🔵 Trivial | 💤 Low value

Add more cases or convert to a regular test.

This rstest has only one case. Either add variations (different list markers, indentation, multi-backtick fences) or convert to #[test] to avoid rstest overhead for a single scenario.

🤖 Prompt for 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.

In `@tests/wrap/spanning_code_spans.rs` around lines 77 - 88, The test rstest
test_wrap_joins_unclosed_span_continuation currently declares a single #[case]
and should either be expanded with additional cases or simplified to a plain
unit test: either add more #[case] entries covering other
markers/indentation/backtick scenarios and keep the #[rstest] signature (e.g.,
extra #[case] tuples passed into the function), or replace the #[rstest]
attribute and #[case] parameters with a single #[test] function that constructs
the input, calls wrap_text(&input, 80), and asserts the expected behavior;
update/remove the #[case] annotations and function signature accordingly so the
test compiles and exercises multiple scenarios or is a normal unit test.
🤖 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.rs`:
- Around line 193-204: The code drops the hard_break value returned by
line_break_parts which loses explicit Markdown hard-breaks when closing a
buffered span (see functions line_break_parts, has_unclosed_code_span,
join_pending_continuation and writer.flush_paragraph and the PendingPrefix /
state.pending_prefix usage); modify PendingPrefix to carry the hard_break flag
(or, alternatively, when has_unclosed_code_span(pending.rest) becomes false,
first flush the pending span back into the main paragraph buffer and then
process the hard_break so writer.flush_paragraph sees the break, updating
join_pending_continuation/_rest handling as needed; add a regression test that
exercises a buffered code span ending with a hard break (example: "- `foo` 
\nbar`  \n`baz") to ensure the break is preserved.

---

Outside diff comments:
In `@tests/wrap/spanning_code_spans.rs`:
- Around line 90-98: The rstest test_wrap_defers_while_any_span_stays_open
currently supplies only one case and should either be expanded with additional
#[case] inputs or converted to a plain #[test]; update the test to include
multiple variations (e.g., multiple open spans, different leading prefixes,
escaped backticks) by adding more #[case] parameters to the function signature
or replace the #[rstest] attribute with #[test] and iterate through a small
vector of inputs that call wrap_text(&input, 80) and assert the same conditions
(presence of "`done`" and "`open span`" or their expected equivalents) so the
behavior around spanning inline code is exercised more thoroughly.
- Around line 77-88: The test rstest test_wrap_joins_unclosed_span_continuation
currently declares a single #[case] and should either be expanded with
additional cases or simplified to a plain unit test: either add more #[case]
entries covering other markers/indentation/backtick scenarios and keep the
#[rstest] signature (e.g., extra #[case] tuples passed into the function), or
replace the #[rstest] attribute and #[case] parameters with a single #[test]
function that constructs the input, calls wrap_text(&input, 80), and asserts the
expected behavior; update/remove the #[case] annotations and function signature
accordingly so the test compiles and exercises multiple scenarios or is a normal
unit test.
🪄 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: fe5aede4-9125-4f75-9984-4eaf09f7d4e4

📥 Commits

Reviewing files that changed from the base of the PR and between 3f7913f and 621fda5.

📒 Files selected for processing (5)
  • src/wrap.rs
  • src/wrap/tokenize/mod.rs
  • src/wrap/tokenize/scanning.rs
  • tests/wrap/spanning_code_spans.rs
  • tests/wrap_unit.rs

Comment thread src/wrap.rs Outdated
@leynos

leynos commented May 28, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai Have the following now been resolved?

tests/wrap/spanning_code_spans.rs (2)

90-98: 🧹 Nitpick | 🔵 Trivial | 💤 Low value
Add more cases or convert to a regular test.
This rstest has only one case. Add variations (multiple open spans, different prefixes, escaped backticks) or convert to #[test].

🤖 Detailed instructions

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

In @tests/wrap/spanning_code_spans.rs around lines 90 - 98, The rstest
test_wrap_defers_while_any_span_stays_open currently supplies only one case and
should either be expanded with additional #[case] inputs or converted to a plain
#[test]; update the test to include multiple variations (e.g., multiple open
spans, different leading prefixes, escaped backticks) by adding more #[case]
parameters to the function signature or replace the #[rstest] attribute with
#[test] and iterate through a small vector of inputs that call wrap_text(&input,
80) and assert the same conditions (presence of "done" and "open span" or
their expected equivalents) so the behavior around spanning inline code is
exercised more thoroughly.

77-88: 🧹 Nitpick | 🔵 Trivial | 💤 Low value
Add more cases or convert to a regular test.
This rstest has only one case. Either add variations (different list markers, indentation, multi-backtick fences) or convert to #[test] to avoid rstest overhead for a single scenario.

🤖 Detailed instructions

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

In @tests/wrap/spanning_code_spans.rs around lines 77 - 88, The test rstest
test_wrap_joins_unclosed_span_continuation currently declares a single #[case]
and should either be expanded with additional cases or simplified to a plain
unit test: either add more #[case] entries covering other
markers/indentation/backtick scenarios and keep the #[rstest] signature (e.g.,
extra #[case] tuples passed into the function), or replace the #[rstest]
attribute and #[case] parameters with a single #[test] function that constructs
the input, calls wrap_text(&input, 80), and asserts the expected behavior;
update/remove the #[case] annotations and function signature accordingly so the
test compiles and exercises multiple scenarios or is a normal unit test.

@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
cat tests/wrap/spanning_code_spans.rs

Length of output: 3483



@leynos Both findings are resolved.

  • test_wrap_joins_unclosed_span_continuation — converted from #[rstest]/#[case] to a plain #[test].
  • test_wrap_defers_while_any_span_stays_open — likewise converted to a plain #[test].

An additional test, test_wrap_preserves_hard_break_when_buffered_span_closes, has been introduced alongside them, which provides extra coverage of the buffered-span close behaviour. All three are clean, self-contained unit tests with no unnecessary rstest overhead.

@leynos

This comment was marked as resolved.

@coderabbitai

This comment was marked as resolved.

@coderabbitai

This comment was marked as resolved.

@leynos

leynos commented May 28, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai Please suggest a fix for the following concern and provide an AI coding agent prompt for the fix:

PR introduces invariants over ranges (fence lengths, spans, escaping) requiring property-based testing, yet uses only hardcoded rstest cases without proptest strategies. | Add proptest properties for fence matching, span atomicity across arbitrary widths/line counts, and exact-length fence matching rejection of mismatches.

@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: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/developers-guide.md`:
- Around line 156-159: The documentation incorrectly states that flush_paragraph
emits buffered text when either has_unclosed_code_span returns false for the
accumulated rest or the text ends with a Markdown hard break; update the wording
to match the implementation: describe that flush_paragraph only emits the
buffered segment after the code span closes (i.e., when has_unclosed_code_span
becomes false), and note that hard_break only affects the emission timing once
closure is detected; reference the functions has_unclosed_code_span,
flush_paragraph, and append_wrapped_with_prefix_width and reword the “or”
phrasing to indicate closure is required before append_wrapped_with_prefix_width
is called (hard_break influences only when closure has been observed).
🪄 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: 64f317ec-b107-4853-b374-81d64bbf8763

📥 Commits

Reviewing files that changed from the base of the PR and between dbcd78c and dca0c31.

📒 Files selected for processing (4)
  • docs/developers-guide.md
  • src/wrap.rs
  • src/wrap/tokenize/mod.rs
  • src/wrap/tokenize/scanning.rs

Comment thread docs/developers-guide.md Outdated
@leynos

leynos commented May 28, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai Please suggest a fix for the following concern and provide an AI coding agent prompt for the fix:

New tests use fixtures with manual assertions rather than snapshot tests; existing codebase uses insta for similar output validation (link_ref_snapshots.rs). Convert spanning_code_spans.rs fixtures to snapshot tests with insta::assert_snapshot!, matching the pattern in link_ref_snapshots.rs for text-based output testing.

@coderabbitai

coderabbitai Bot commented May 28, 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/spanning_code_spans.rs ==="
cat tests/wrap/spanning_code_spans.rs

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

echo ""
echo "=== Cargo.toml dev-deps ==="
grep -A2 'insta\|proptest\|rstest' Cargo.toml

Length of output: 5217


🏁 Script executed:

#!/bin/bash
echo "=== Snapshot files for spanning_code_span ==="
fd -e snap . tests/snapshots/ | grep spanning || echo "(none found)"

echo ""
echo "=== All snapshot files ==="
fd -e snap . tests/ | head -30

Length of output: 2063



@leynos This concern is already resolved in the current branch — no further work is needed.

Specifically:

  1. test_wrap_spanning_code_span_fixtures already uses insta::with_settings! and insta::assert_snapshot!, matching the link_ref_snapshots.rs pattern exactly. The assert_eq! comparison against the *_expected.txt files has been removed.
  2. Both snapshot files are committed:
    • tests/snapshots/spanning_code_span_-.snap
    • tests/snapshots/spanning_code_span_1_.snap
  3. The focused unit tests (test_wrap_spanning_code_span_nested_blockquote_prefixes_do_not_merge, etc.) remain as plain #[test] functions with structural assertions — correctly left un-snapshotted, as they test invariants rather than full output fidelity.

No agent prompt is required. ✅

@coderabbitai

This comment was marked as resolved.

@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: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
docs/developers-guide.md (1)

144-162: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Wrap the pending-prefix deferral paragraph at 80 columns.

The paragraph from line 144 through line 162 violates the 80-column wrapping requirement for Markdown prose.

Reflow the paragraph to wrap at 80 columns. Run make fmt after making the change to ensure table markup and other formatting remain correct.

Triage: [type:syntax/md]

As per coding guidelines: "Markdown paragraphs and bullet points must be wrapped at 80 columns."

🤖 Prompt for 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.

In `@docs/developers-guide.md` around lines 144 - 162, The Markdown paragraph
describing pending prefix deferral exceeds the 80-column wrap limit; please
reflow that block (the text referencing handle_prefix_line,
has_unclosed_code_span, ParagraphState::pending_prefix/PendingPrefix,
handle_pending_continuation in src/wrap.rs, join_pending_continuation,
continuation_begins_with_closing_fence, flush_paragraph,
append_wrapped_with_prefix_width, parse_open_code_span, and clear()) so all
lines are ≤80 characters, preserving wording and references, then run `make fmt`
to ensure tables/markup remain correct.
♻️ Duplicate comments (1)
docs/developers-guide.md (1)

157-162: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clarify flush-condition semantics to match the implementation.

The text states "a deferred open span (detected by parse_open_code_span before the join) suppresses emission unless a pending Markdown hard break forces it," which implies hard_break can force emission whilst the span remains unclosed. The implementation (src/wrap/paragraph.rs:233-254) shows flush_paragraph only emits once has_unclosed_code_span returns false. Rewrite to state that hard_break affects trailing-space emission after closure, not that it forces emission before closure.

Triage: [type:docstyle]

As per coding guidelines: "Keep the developer's guide synchronised with decision records, roadmap items, and the codebase, as a stale developer's guide is worse than a shorter one."

🤖 Prompt for 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.

In `@docs/developers-guide.md` around lines 157 - 162, Update the paragraph to
match the implementation: clarify that a deferred open span (detected by
parse_open_code_span) prevents flush_paragraph from emitting until the span is
closed (flush_paragraph checks has_unclosed_code_span and only emits after
closure), and that hard_break only controls whether two trailing spaces are
appended after the paragraph is emitted (i.e., it affects trailing-space
emission after closure), not that hard_break forces emission while a code span
remains unclosed; also mention that ParagraphState.clear() resets pending_prefix
to None as before.
🤖 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 `@tests/wrap_unit.rs`:
- Around line 423-464: The proptest property block (the proptest! tests
referencing has_unclosed_code_span and wrap_text) must be moved out of the
oversized tests/wrap_unit.rs into a new test file to meet the 400-line rule;
create a new test module (e.g., tests/wrap_properties.rs) that imports
proptest::prelude::* and mdtablefix::wrap::{has_unclosed_code_span, wrap_text},
paste the entire proptest! { ... } block there, remove the original block from
tests/wrap_unit.rs, and run cargo test to ensure the new file is auto-discovered
(or add a mod declaration if your test harness requires it).

---

Outside diff comments:
In `@docs/developers-guide.md`:
- Around line 144-162: The Markdown paragraph describing pending prefix deferral
exceeds the 80-column wrap limit; please reflow that block (the text referencing
handle_prefix_line, has_unclosed_code_span,
ParagraphState::pending_prefix/PendingPrefix, handle_pending_continuation in
src/wrap.rs, join_pending_continuation, continuation_begins_with_closing_fence,
flush_paragraph, append_wrapped_with_prefix_width, parse_open_code_span, and
clear()) so all lines are ≤80 characters, preserving wording and references,
then run `make fmt` to ensure tables/markup remain correct.

---

Duplicate comments:
In `@docs/developers-guide.md`:
- Around line 157-162: Update the paragraph to match the implementation: clarify
that a deferred open span (detected by parse_open_code_span) prevents
flush_paragraph from emitting until the span is closed (flush_paragraph checks
has_unclosed_code_span and only emits after closure), and that hard_break only
controls whether two trailing spaces are appended after the paragraph is emitted
(i.e., it affects trailing-space emission after closure), not that hard_break
forces emission while a code span remains unclosed; also mention that
ParagraphState.clear() resets pending_prefix to None as before.
🪄 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: d2540caa-42cb-4efb-94ec-3f62d6f8b5dd

📥 Commits

Reviewing files that changed from the base of the PR and between dca0c31 and 4fe74f2.

⛔ Files ignored due to path filters (2)
  • tests/snapshots/spanning_code_span_-.snap is excluded by !**/*.snap
  • tests/snapshots/spanning_code_span_1_.snap is excluded by !**/*.snap
📒 Files selected for processing (4)
  • .gitignore
  • docs/developers-guide.md
  • tests/wrap/spanning_code_spans.rs
  • tests/wrap_unit.rs

Comment thread tests/wrap_unit.rs Outdated

@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: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/wrap_properties.rs`:
- Around line 101-113: The test
continuation_begins_with_closing_fence_rejects_length_mismatch only checks
longer closing fences; update it to exercise both longer and shorter mismatched
fences by constructing two variants: one with close_fence_long = "`".repeat(n +
delta) (keep the existing assertion) and, when n > delta, also build
close_fence_short = "`".repeat(n - delta) and assert
continuation_begins_with_closing_fence(&existing,
&format!("{close_fence_short}{suffix}")) is false; reference the test function
continuation_begins_with_closing_fence_rejects_length_mismatch and the helper
function continuation_begins_with_closing_fence to locate the code. Ensure you
only create the short-fence case when n > delta to avoid zero-length fences.
🪄 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: a3b039a8-2500-4a0c-a8d1-a91a1dffb3d4

📥 Commits

Reviewing files that changed from the base of the PR and between 4fe74f2 and 2db8d9c.

📒 Files selected for processing (3)
  • src/wrap.rs
  • tests/wrap_properties.rs
  • tests/wrap_unit.rs
💤 Files with no reviewable changes (1)
  • tests/wrap_unit.rs

Comment thread tests/wrap_properties.rs Outdated
@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Actionable comments posted: 0

@leynos

leynos commented May 28, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai Please suggest a fix for the following concern and provide an AI coding agent prompt for the fix:

| Testing (Property / Proof) | ⚠️ Warning | Property tests are insufficient. The PendingPrefix deferral mechanism—the PR's core feature—lacks property testing with generated prefixes, widths, and hard-breaks. | Add proptest for: (1) deferral with randomised bullet/ordered/blockquote prefixes; (2) hard-break preservation during deferred spans; (3) cross-line span atomicity with generated inputs. |

@leynos

leynos commented May 28, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai Please suggest a fix for the following concern and provide an AI coding agent prompt for the fix:

| Performance And Resource Use | ⚠️ Warning | Rescans pending.rest with parse_open_code_span and has_unclosed_code_span on each continuation line, creating O(N²) complexity where N is continuation line count. | Cache the open code span fence length and check only appended text, reducing complexity from O(N²) to O(N). |

@coderabbitai

This comment was marked as resolved.

@coderabbitai

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

@coderabbitai

This comment was marked as resolved.

`apply_continuation_chunk` was walking the continuation chunk twice for
its opening backtick run: once via `continuation_needs_leading_space`
to derive the offset, and again inside `join_pending_continuation`
when deciding whether to insert a separating space. Both call sites
operated on the same `text`, so the second scan was redundant.

Compute `tokenize::opening_fence_run_len(text.as_bytes(), text)` once
in `apply_continuation_chunk` and thread the resulting `Option<usize>`
into a renamed `join_pending_continuation` and into a small
`leading_run_needs_space` helper that replaces
`continuation_needs_leading_space`. The behaviour is unchanged: the
comparison `run_len != open_fence_len` and the `None ⇒ true` fallback
both move into the helper, and `join_pending_continuation` consults
the same value rather than rescanning.

Also adopt en-GB Oxford `-ize` spelling in the module docstring and
the `update_span_state` comment: "synthesises"/"synthesise" become
"synthesizes"/"synthesize" so the comments match the
en-GB-oxendict guideline in `AGENTS.md`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/wrap/continuation.rs (1)

196-209: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Keep the reopened span live instead of synthesizing a closer.

Return ClosedAndReopened here rather than appending a synthetic fence and clearing open_fence_len. Line 207 manufactures a closer, but Line 208 drops the only state that can consume the real closing fence from the next continuation, so that next chunk is joined back as literal text or a fresh opener. That reintroduces the duplicated/orphaned backtick path for cases such as `4.1.1` split across lines.

Suggested change
-            if let Some((_, new_len)) =
-                split_reopen_span(continuation, continuation_offset, raw_fence)
-            {
-                pending.rest.push_str(&"`".repeat(new_len));
-                pending.open_fence_len = None;
-                return SpanStateUpdate::StillOpen;
-            }
+            if let Some((split_at, new_len)) =
+                split_reopen_span(continuation, continuation_offset, raw_fence)
+            {
+                return SpanStateUpdate::ClosedAndReopened { split_at, new_len };
+            }

Based on learnings: "Inline code spans, Markdown links, and GFM footnote references must be atomic fragments that are never split across lines and move as a unit when they would overflow the target width."

🤖 Prompt for 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.

In `@src/wrap/continuation.rs` around lines 196 - 209, The code currently detects
a reopened span via split_reopen_span(continuation, continuation_offset,
raw_fence) but synthesizes a closing fence by appending to pending.rest and
clears pending.open_fence_len before returning SpanStateUpdate::StillOpen;
instead, preserve the reopened span state so the real closer in the next
continuation can be consumed: remove the synthetic fence append and do not clear
pending.open_fence_len, and return SpanStateUpdate::ClosedAndReopened (replace
the StillOpen return) when split_reopen_span returns Some, so the reopened span
remains live and the next chunk can close it properly.
🤖 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.

Outside diff comments:
In `@src/wrap/continuation.rs`:
- Around line 196-209: The code currently detects a reopened span via
split_reopen_span(continuation, continuation_offset, raw_fence) but synthesizes
a closing fence by appending to pending.rest and clears pending.open_fence_len
before returning SpanStateUpdate::StillOpen; instead, preserve the reopened span
state so the real closer in the next continuation can be consumed: remove the
synthetic fence append and do not clear pending.open_fence_len, and return
SpanStateUpdate::ClosedAndReopened (replace the StillOpen return) when
split_reopen_span returns Some, so the reopened span remains live and the next
chunk can close it properly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 755c84d3-d024-4490-90b6-16f03004d348

📥 Commits

Reviewing files that changed from the base of the PR and between e8d33ac and d3cdd4c.

📒 Files selected for processing (1)
  • src/wrap/continuation.rs

`join_pending_continuation` moved from `src/wrap.rs` to
`src/wrap/continuation.rs` in commit e8d33ac, but the key-types table
still pointed at the old location. The new module — which owns the
join/update/dispatch state machine for soft-wrapped continuations —
was undocumented, and the prose paragraph on pending-prefix deferral
did not mention `apply_continuation_chunk` as the entry point.

Fix the file location on the existing `join_pending_continuation`
row, add an `apply_continuation_chunk` row above it with a one-line
description, and expand the deferral paragraph to record that
`handle_pending_continuation` delegates each soft-wrapped chunk to
`apply_continuation_chunk` in the new module, that the helper drives
a `SpanStateUpdate` (`StillOpen`, `ClosedAndReopened`, `Flush`), and
that a same-chunk close-and-reopen synthesizes a closer for the new
span so it stays atomic in the buffer.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@leynos

leynos commented May 29, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai Have the following now been resolved?

Please address the following concerns and ensure all commit gates succeed:

⚠️ Outside diff range comments (1)

src/wrap/continuation.rs (1)

196-209: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win
Keep the reopened span live instead of synthesizing a closer.
Return ClosedAndReopened here rather than appending a synthetic fence and clearing open_fence_len. Line 207 manufactures a closer, but Line 208 drops the only state that can consume the real closing fence from the next continuation, so that next chunk is joined back as literal text or a fresh opener. That reintroduces the duplicated/orphaned backtick path for cases such as 4.1.1 split across lines.

Suggested change
-            if let Some((_, new_len)) =
-                split_reopen_span(continuation, continuation_offset, raw_fence)
-            {
-                pending.rest.push_str(&"`".repeat(new_len));
-                pending.open_fence_len = None;
-                return SpanStateUpdate::StillOpen;
-            }
+            if let Some((split_at, new_len)) =
+                split_reopen_span(continuation, continuation_offset, raw_fence)
+            {
+                return SpanStateUpdate::ClosedAndReopened { split_at, new_len };
+            }

Based on learnings: "Inline code spans, Markdown links, and GFM footnote references must be atomic fragments that are never split across lines and move as a unit when they would overflow the target width."

🤖 Detailed instructions

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

In @src/wrap/continuation.rs around lines 196 - 209, The code currently detects
a reopened span via split_reopen_span(continuation, continuation_offset,
raw_fence) but synthesizes a closing fence by appending to pending.rest and
clears pending.open_fence_len before returning SpanStateUpdate::StillOpen;
instead, preserve the reopened span state so the real closer in the next
continuation can be consumed: remove the synthetic fence append and do not clear
pending.open_fence_len, and return SpanStateUpdate::ClosedAndReopened (replace
the StillOpen return) when split_reopen_span returns Some, so the reopened span
remains live and the next chunk can close it properly.

@coderabbitai

This comment was marked as resolved.

@leynos

leynos commented May 29, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

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 May 29, 2026

Copy link
Copy Markdown
Owner Author

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

The developers-guide.md lists join_pending_continuation in src/wrap.rs, but it's actually in src/wrap/continuation.rs. The new continuation module itself is undocumented. Correct join_pending_continuation file location to src/wrap/continuation.rs and add documentation for the new continuation module covering apply_continuation_chunk.

@coderabbitai

coderabbitai Bot commented May 29, 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 — Key types table and continuation module docs ==="
fd -i 'developers-guide.md' --exec cat {}

echo ""
echo "=== src/wrap/continuation.rs — module-level doc and exported functions ==="
cat src/wrap/continuation.rs

Length of output: 37697



@leynos Both documentation concerns are resolved.

Concern Status Evidence
join_pending_continuation file location ✅ Resolved Key types table now maps join_pending_continuationsrc/wrap/continuation.rs.
New continuation module undocumented ✅ Resolved src/wrap/continuation.rs carries a full module-level //! doc comment; apply_continuation_chunk appears in the Key types table with a description ("Centralised join/update/dispatch entry point…"); the Step 2 "Pending prefix deferral" prose in the guide references both the function and the module by name.

One carry-over item from the previous round remains outstanding: update_span_state in src/wrap/continuation.rs still synthesises a closing fence and returns StillOpen instead of ClosedAndReopened in the Some(n) if n > 0 arm — the exact defect identified in the last failed-checks prompt. That is a code-correctness issue, not a documentation one, and it is the direct cause of the snapshot still asserting broken output. The agent prompt from the previous round (replacing the synthetic-closer block with return SpanStateUpdate::ClosedAndReopened { split_at, new_len };) remains the fix required.

@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: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/developers-guide.md`:
- Line 282: Update the documentation entry for apply_continuation_chunk by
changing the word "Centralised" to "Centralized" in the descriptive text that
reads "Centralised join/update/dispatch entry point that reconciles a single
continuation chunk with the active `PendingPrefix` buffer." Ensure the corrected
spelling appears in the line mentioning `apply_continuation_chunk` and the file
reference `src/wrap/continuation.rs`.
🪄 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: 3641ca41-7995-4a21-8d92-f2ffe8aa6604

📥 Commits

Reviewing files that changed from the base of the PR and between d3cdd4c and 340f074.

📒 Files selected for processing (1)
  • docs/developers-guide.md

Comment thread docs/developers-guide.md Outdated
@leynos

leynos commented May 29, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai pause

@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Reviews paused.

"Centralised" → "Centralized" in the apply_continuation_chunk row,
consistent with the project's -ize suffix convention throughout.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.

Reflow widens closing backtick onto its own line and orphans words inside soft-wrapped code spans

1 participant