Stabilize cross-line code-span wrapping (#375) (#370) - #405
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRework cross-line inline-code wrapping so joined spans reflow trailing prose in one pass, continuation-prefix mismatches fall back to normal handling, and overlong spans preserve conforming authored boundaries. Add regression, property, architecture, user-guide, and changelog coverage. ChangesInline-code wrapping
Sequence Diagram(s)sequenceDiagram
participant Input
participant wrap_text
participant ParagraphWriter
participant spanning_code
participant Output
Input->>wrap_text: provide markdown lines
wrap_text->>ParagraphWriter: process pending continuation
ParagraphWriter->>spanning_code: inspect cross-line code span
spanning_code-->>ParagraphWriter: return fitted or preserved source lines
ParagraphWriter->>Output: emit wrapped paragraph
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 20✅ Passed checks (20 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@coderabbitai Have the following failed checks now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures. Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. (Treat documentation and validation coverage as in scope). ❌ Failed checks (1 error, 2 warnings)
|
This comment was marked as resolved.
This comment was marked as resolved.
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
6b56c9c to
6440976
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@coderabbitai review |
✅ Action performedReview finished.
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
02bd624 to
bd71fe9
Compare
|
@coderabbitai Have the following now been resolved? src/wrap/paragraph.rs (1)
🤖 Detailed instructionsVerify each finding against current code. Fix only still-valid issues, skip the In Source: MCP tools |
This comment was marked as resolved.
This comment was marked as resolved.
|
@coderabbitai review |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai resume |
|
@coderabbitai Have the following now been resolved? docs/developers-guide.md (1)
🤖 Detailed instructionsVerify each finding against current code. Fix only still-valid issues, skip the In Sources: Coding guidelines, Path instructions, Pipeline failures |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Relevant headings and their line numbers =='
rg -n -C 2 \
'^## (1\. Stateful pipeline helpers|2\. Test infrastructure|3\. Breaks module . Cow allocation strategy)$|^### ' \
docs/developers-guide.md || true
echo
echo '== Earlier retained section (reported Lines 789–940) =='
nl -ba docs/developers-guide.md | sed -n '770,950p'
echo
echo '== Reported duplicate range and following context =='
nl -ba docs/developers-guide.md | sed -n '930,1110p'
echo
echo '== Exact occurrence counts for duplicated section headings =='
for heading in \
'## 1. Stateful pipeline helpers' \
'## 2. Test infrastructure' \
'## 3. Breaks module – Cow allocation strategy'
do
printf '%s: ' "$heading"
grep -Fxc "$heading" docs/developers-guide.md || true
done
echo
echo '== Markdownlint configuration and changed documentation diff =='
fd -HI -t f -g '.markdownlint*' -g 'markdownlint*' -g 'Makefile' . -x sh -c '
echo "--- $1"
sed -n "1,220p" "$1"
' sh {} || true
git diff --unified=20 HEAD^ HEAD -- docs/developers-guide.md || trueLength of output: 5491 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Relevant top-level heading occurrences =='
for heading in \
'## 1. Stateful pipeline helpers' \
'## 2. Test infrastructure' \
'## 3. Breaks module – Cow allocation strategy'
do
printf '%s: ' "$heading"
awk -v heading="$heading" '$0 == heading { count++ } END { print count + 0 }' \
docs/developers-guide.md
done
echo
echo '== Relevant section, with line numbers =='
awk 'NR >= 770 && NR <= 1110 { printf "%6d\t%s\n", NR, $0 }' \
docs/developers-guide.md
echo
echo '== Heading spacing violations in the relevant section =='
awk '
NR >= 770 && NR <= 1110 {
lines[NR] = $0
}
END {
for (n = 771; n < 1110; n++) {
if (lines[n] ~ /^#{1,6} / && lines[n - 1] != "") {
printf "Missing blank line before heading at Line %d: %s\n", n, lines[n]
}
if (lines[n] ~ /^#{1,6} / && lines[n + 1] != "") {
printf "Missing blank line after heading at Line %d: %s\n", n, lines[n]
}
}
}
' docs/developers-guide.md
echo
echo '== Current PR diff for the document =='
git diff --unified=12 HEAD^ HEAD -- docs/developers-guide.md || true
echo
echo '== Markdownlint targets/configuration references =='
rg -n -C 3 --glob 'Makefile' --glob '.markdownlint*' --glob 'markdownlint*' \
'markdownlint|MD022|MD024' . || trueLength of output: 26318
Run |
|
@coderabbitai Have the following now been resolved? src/wrap/paragraph.rs (2)
🐛 Proposed fixif *hard_break {
self.push_wrapped_segment(&state.indent, &segment);
- if !state.indent.is_empty() {
- self.ensure_trailing_hard_break_on_last_line();
- }
+ self.ensure_trailing_hard_break_on_last_line();
segment.clear();
}Add a regression for an unindented paragraph ending in two trailing spaces. 🤖 Detailed instructionsVerify each finding against current code. Fix only still-valid issues, skip the In Source: Learnings ♻️ Proposed extraction+fn hard_break_marker_len(line: &str) -> usize {
+ if line.ends_with(" ") {
+ return 2;
+ }
+ let trailing_backslashes = line
+ .chars()
+ .rev()
+ .take_while(|character| *character == '\\')
+ .count();
+ usize::from(trailing_backslashes % 2 == 1)
+}As per coding guidelines, "Reuse equivalent in-repository implementations... instead of creating parallel or bespoke implementations." 🤖 Detailed instructionsVerify each finding against current code. Fix only still-valid issues, skip the In Source: Coding guidelines src/wrap/paragraph/spanning_code.rs (2)
♻️ Proposed fix- let mut found_overlong_span = false;
let groups = hard_break_groups(segments)
.map(|group| {
let (joined, boundaries) = join_with_boundaries(group);
let spans = if group.len() < 2 || group.iter().any(|(line, _)| line.width() > available)
{
Vec::new()
} else {
overlong_code_spans_crossing_boundaries(&joined, &boundaries, available)
};
- found_overlong_span |= !spans.is_empty();
let has_hard_break = group.last().is_some_and(|(_, hard_break)| *hard_break);
(joined, spans, has_hard_break)
})
.collect::<Vec<_>>();
- if !found_overlong_span {
+ if groups.iter().all(|(_, spans, _)| spans.is_empty()) {
return None;
}As per coding guidelines, "Separate queries, commands, fallible operations, and side effects. Queries must be read-only." 🤖 Detailed instructionsVerify each finding against current code. Fix only still-valid issues, skip the In Source: Coding guidelines ♻️ Proposed traceelse {
+ trace!(
+ mode = "preserve_authored_boundaries",
+ boundary = "span_not_located",
+ span_len = span_text.len(),
+ line_count = lines.len(),
+ "skipped boundary preservation because the span was absent from wrapped output"
+ );
return;
};As per coding guidelines, "Operationally significant changes must provide meaningful, bounded, non-sensitive logs, metrics, tracing across relevant boundaries." 🤖 Detailed instructionsVerify each finding against current code. Fix only still-valid issues, skip the In Source: Coding guidelines tests/wrap_code_span_reflow_properties.rs (1)
💚 Proposed strengtheninglet once = wrap_text(&input, width);
+ prop_assert!(
+ once.iter()
+ .any(|line| line.contains(&format!("`{code_first} {code_second}`")))
+ );
+ prop_assert!(once.join("\n").contains(&tail));
prop_assert_eq!(wrap_text(&once, width), once);As per coding guidelines, "tests must not be vacuous, over-mocked, tautological, or implementation-detail-focused." 🤖 Detailed instructionsVerify each finding against current code. Fix only still-valid issues, skip the In Source: Coding guidelines tests/wrap/code_span_reflow.rs (1)
💚 Proposed additionsassert!(lines_conform(&output));
+ assert_eq!(wrap_text(&output, WRAP_COLS), output);
}As per coding guidelines, "New functionality and behavioral changes require substantive tests that would fail for plausible incorrect implementations." 🤖 Detailed instructionsVerify each finding against current code. Fix only still-valid issues, skip the In Source: Coding guidelines src/wrap/paragraph_tests.rs (1)
♻️ Proposed parameterisation-#[test]
-fn handle_prefix_line_can_repeat_or_change_the_continuation_prefix() {
- let mut out = Vec::new();
- let mut writer = ParagraphWriter::new(&mut out, 14);
- ...
-}
+#[rstest]
+#[case::task_list(14, "- [ ] ", "alpha beta", false, None, &["- [ ] alpha", " beta"])]
+#[case::blockquote(10, "> ", "alpha beta gamma", true, None, &["> alpha", "> beta", "> gamma"])]
+#[case::quoted_list(10, "> - ", "alpha beta gamma", false, Some("> "), &["> - alpha", "> beta", "> gamma"])]
+fn handle_prefix_line_can_repeat_or_change_the_continuation_prefix(
+ #[case] width: usize,
+ #[case] prefix: &str,
+ #[case] rest: &str,
+ #[case] repeat_prefix: bool,
+ #[case] outer_prefix: Option<&str>,
+ #[case] expected: &[&str],
+) {
+ let mut out = Vec::new();
+ let mut writer = ParagraphWriter::new(&mut out, width);
+ let mut state = ParagraphState::default();
+ writer.handle_prefix_line(
+ &mut state,
+ &PrefixLine {
+ prefix: Cow::Borrowed(prefix),
+ rest,
+ repeat_prefix,
+ outer_prefix: outer_prefix.map(Cow::Borrowed),
+ },
+ );
+ assert_eq!(out, expected.iter().map(ToString::to_string).collect::<Vec<_>>());
+}As per coding guidelines, "Replace duplicated Rust tests with 🤖 Detailed instructionsVerify each finding against current code. Fix only still-valid issues, skip the In Source: Coding guidelines |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Relevant outlines =='
ast-grep outline src/wrap/paragraph.rs --items all
ast-grep outline src/wrap/paragraph/spanning_code.rs --items all
ast-grep outline src/wrap/paragraph_tests.rs --items all
ast-grep outline tests/wrap/code_span_reflow.rs --items all
ast-grep outline tests/wrap_code_span_reflow_properties.rs --items all
echo
echo '== Paragraph hard-break and stable-tail paths =='
rg -n -C 18 \
'append_stable_pending_prefix|ensure_trailing_hard_break_on_last_line|hard_break_marker_len|tail_reflow|line_break_parts|push_wrapped_segment' \
src/wrap/paragraph.rs
echo
echo '== Spanning-code group detection and lookup miss trace =='
rg -n -C 16 \
'found_overlong_span|hard_break_groups|collect::<Vec|spans\.is_empty|find_map|span_not_located|preserve_authored_boundaries|trace!' \
src/wrap/paragraph/spanning_code.rs
echo
echo '== Property assertions for first-pass span/tail behaviour =='
sed -n '1,180p' tests/wrap_code_span_reflow_properties.rs
echo
echo '== Code-span regression idempotence coverage =='
rg -n -C 16 \
'fn later_differently_sized_fence_preserves_overlong_span_boundaries|fn overlong_spanning_code_preserves_paragraph_indent|fn mixed_hard_break_groups_preserve_eligible_span_boundaries|assert_eq!\(wrap_text' \
tests/wrap/code_span_reflow.rs
echo
echo '== Continuation-prefix test parameterization =='
sed -n '1,150p' src/wrap/paragraph_tests.rs
rg -n -C 4 'rstest|handle_prefix_line_can_repeat_or_change_the_continuation_prefix|case::|use rstest' \
src/wrap/paragraph_tests.rs Cargo.toml
echo
echo '== Current diff for all reported files =='
git diff --unified=35 HEAD^ HEAD -- \
src/wrap/paragraph.rs \
src/wrap/paragraph/spanning_code.rs \
src/wrap/paragraph_tests.rs \
tests/wrap/code_span_reflow.rs \
tests/wrap_code_span_reflow_properties.rsLength of output: 62735
Run the relevant Rust tests, formatter, and lint commands in CI or a local checkout to complete runtime validation; this inspection environment only supports static analysis. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/wrap/paragraph.rs`:
- Around line 433-441: Extract the tail-reflow and hard-break functions
append_stable_pending_prefix, emit_tail_segment, trailing_hard_break_marker_len,
and ensure_trailing_hard_break_on_last_line from paragraph.rs into a dedicated
submodule, updating visibility, imports, and call sites as needed while
preserving behavior. Keep paragraph.rs under the 400-line limit.
- Around line 232-248: Split the oversized paragraph-wrapping implementation
into smaller focused modules, moving related functionality from the surrounding
paragraph code while preserving behavior and interfaces. Keep emit_tail_segment
and its callers working unchanged, and ensure each resulting module remains
cohesive and within the 400-line limit.
🪄 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: 6f8d574b-e4b4-4511-a2a3-6aca019a513f
📒 Files selected for processing (8)
docs/architecture.mddocs/developers-guide.mdsrc/wrap/paragraph.rssrc/wrap/paragraph/spanning_code.rssrc/wrap/paragraph_tests.rstests/wrap/code_span_reflow.rstests/wrap/hard_line_breaks.rstests/wrap_code_span_reflow_properties.rs
💤 Files with no reviewable changes (1)
- docs/developers-guide.md
✅ Action performedReviews resumed. |
6510fcb to
1cb6628
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@CHANGELOG.md`:
- Around line 24-29: Update the changelog entry for issue `#370` to qualify that
authored line boundaries are preserved only when the joined inline-code span
exceeds the wrap width and every authored source line already fits within that
width; retain the existing intent about preventing new MD013 violations.
In `@src/wrap/paragraph/spanning_code.rs`:
- Around line 241-247: Update restore_last_hard_break to use the shared
hard-break parity helper instead of checking only for a two-space suffix,
preserving existing hard-break markers such as a final backslash. Move the
helper from tail_reflow.rs into a shared location and call it from both
restore_last_hard_break and the existing tail-reflow path.
In `@src/wrap/paragraph/tail_reflow.rs`:
- Around line 69-75: Add a Rustdoc comment above
ensure_trailing_hard_break_on_last_line documenting that it mutates the last
emitted line in place, does nothing when the output is empty, and considers an
odd trailing-backslash run an existing hard break. Use the required /// syntax.
In `@tests/wrap_code_span_reflow_properties.rs`:
- Around line 22-25: Move the UnicodeWidthStr::width(joined_span.as_str()) <=
width - 2 prop_assume! check to immediately after constructing input and before
calling wrap_text in the first property. Keep the existing joined_span
construction and assertion unchanged, matching the assumption-before-action
ordering used by the second property.
🪄 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: 75f94c59-4a55-4b24-9eee-8355aeec1109
📒 Files selected for processing (17)
CHANGELOG.mddocs/architecture.mddocs/developers-guide.mddocs/state-machine-abstractions-roadmap.mddocs/users-guide.mdsrc/wrap.rssrc/wrap/continuation.rssrc/wrap/continuation_tests.rssrc/wrap/paragraph.rssrc/wrap/paragraph/pending.rssrc/wrap/paragraph/spanning_code.rssrc/wrap/paragraph/tail_reflow.rssrc/wrap/paragraph_tests.rstests/wrap/code_span_reflow.rstests/wrap/hard_line_breaks.rstests/wrap/mod.rstests/wrap_code_span_reflow_properties.rs
Keep resolved list continuations buffered through their paragraph boundary so trailing prose reaches its fixed-point layout in one formatter run. Preserve authored source breaks when joining an inline-code span would create an overlong line, while retaining the existing fallback for ambiguous close-and-reopen spans. Cover both reported examples with integration and property tests, and document the wrapping contract for users and maintainers.
Retain only authored boundaries inside inline-code spans that would exceed the wrap width when joined. Keep surrounding prose eligible for greedy reflow, account for paragraph indentation, and continue scanning after an unmatched fence run. Cover the fallback with concrete prefixed-context regressions and targeted properties. Reconcile the user and developer documentation with the boundary-preserving guarantee and document its state and tracing decisions.
Detect qualifying overlong spans before invoking the greedy wrapper, avoid cloning span text, and use binary authored-boundary lookup. Document the helper contract and complexity so its fallback cost is explicit. Number the developer guide's remaining long-form hierarchy for stable review cross-references.
Keep an over-width source line from disabling authored-boundary preservation in a later conforming hard-break group. Leave each ineligible group on the ordinary atomic wrapping path. Cover the mixed-group behaviour with a regression combining an intentionally over-width atomic span and a qualifying cross-line span.
Keep the reopened-span trace boundary categorical and expose its byte position through a dedicated offset field. Document why pending-prefix flushing deliberately rewraps its tail in a second stage to preserve deterministic, idempotent output.
Keep double-space and odd-backslash boundaries intact while independently rewrapping pending-prefix tail segments, including stable continuation alignment on subsequent formatter passes. Clarify continuation dispatch helpers, remove the avoidable spanning-code piece clone, and align the developer source map with the pending submodule.
Reuse one hard-break marker detector while emitting stable pending-prefix tails, and preserve authored markers for top-level paragraphs as well as indented continuations. Strengthen fixed-point and content coverage, consolidate continuation-prefix cases, and add bounded diagnostics for failed span-boundary lookup. Remove duplicated developer guidance and restore the malformed architecture example so the documentation remains formatter- and lint-stable.
Move `append_stable_pending_prefix`, `emit_tail_segment`, `ensure_trailing_hard_break_on_last_line`, and `trailing_hard_break_marker_len` out of `paragraph.rs` into a dedicated `tail_reflow` submodule. This isolates the deterministic tail-rewrap and hard-break logic from the buffer-management code and brings `paragraph.rs` back under the 400-line limit (442 -> 366 lines). Behaviour is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rebasing onto origin/main auto-merged docs/architecture.md and docs/developers-guide.md with the Weave driver, which introduced two semantic corruptions that failed markdownlint: - developers-guide.md duplicated the entire "Stateful pipeline helpers" through "Breaks module" section verbatim; remove the duplicate copy. - architecture.md lost the "After:" label and its opening fence for the footnote-rewrite example, folding the "After" footnote references into the "Before" block and duplicating the trailing paragraph; restore the two distinct Before/After ```markdown examples. Both files now pass make markdownlint and make nixie. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review feedback on the cross-line code-span wrapping work: - Extract `trailing_hard_break_marker_len` into a new `hard_break` submodule so both the stable tail-reflow path and the spanning-code fallback share one parity check. - Fix `restore_last_hard_break` to detect an existing hard break via the shared helper instead of a two-space suffix, so a final backslash hard-break marker is preserved rather than doubled. - Document `ensure_trailing_hard_break_on_last_line`, noting the in-place mutation, the empty-output no-op, and the odd trailing-backslash case. - Reorder the first reflow property so its `prop_assume!` precedes `wrap_text`, matching the assumption-before-action ordering of the second property. - Qualify the #370 changelog entry: authored line boundaries are preserved only when joining the span exceeds the wrap width and every authored source line already fits within that width. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
89bd85c to
ff6ff3a
Compare
3 participants
Summary
This branch makes cross-line inline-code wrapping stable in a single
mdtablefix --wraprun. It keeps a resolved prefixed span buffered until itscontinuation ends, then emits the tail using the same boundary model that a
subsequent formatter run would see.
When joining a cross-line code span would exceed the configured width, the
wrapper preserves only already-conforming source boundaries inside that span.
It restores paragraph indentation, greedily reflows surrounding prose, and
continues scanning after unmatched fence runs so later differently sized spans
are still recognized. Dedicated traces expose the fallback, prefix mismatch,
and tail-reflow decisions without recording document content.
Closes #375.
Closes #370.
Review walkthrough
src/wrap/paragraph/spanning_code.rs
for the indentation-aware, boundary-scoped fallback and fence scanner.
src/wrap/continuation.rs,
src/wrap.rs,
and
src/wrap/paragraph.rs
for continuation state, decision traces, and fallback integration.
tests/wrap/code_span_reflow.rs
and
tests/wrap_code_span_reflow_properties.rs
for list, repeated-blockquote, footnote, indentation, mismatched-fence,
surrounding-prose, idempotence, and width coverage.
user's guide,
developer's guide,
and
state-machine roadmap.
Validation
make check-fmt: passedmake typecheck: passedmake lint: passedmake test: passedmake markdownlint: passed with 0 errorsmake nixie: passedmbake validate Makefile: passedcoderabbit review --agent: passed with 0 findingsgit diff --check: passedReferences