Document SEP_RE constant and ADR for table reflow pipeline - #267
Conversation
Introduces Table Reflow Pipeline with Unicode Width, Continuation-Row Preservation, Ellipsis Ordering, and Comprehensive Tests & DocsThis PR implements a staged Markdown table reflow pipeline with Unicode display-width alignment, explicit handling for leading-empty continuation rows, ellipsis-before-reflow ordering, separator validation, and broad unit/integration test coverage. The changes are documented by ADR 0001 and covered in updated architecture, developer, user guides and an updated execplan document. Key Behavioural Changes
Utilities & API (internal / pub(crate))
Tests
Documentation
Backward Compatibility & Fallbacks
Notes from Review Thread
CI / Developer guidance
WalkthroughAugment the Markdown table reflow pipeline to protect leading-empty continuation cells with a marker, compute column widths using Unicode display width, enforce a minimum separator width of three dashes, and apply ellipsis replacement to buffered table lines before reflow. Update process buffering, parsing/cleaning, separator formatting, tests, and documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Process Entry
participant Buffer as ProcessBuffer
participant Protector as protect_leading_empty_cells()
participant Parser as parse_rows()
participant Cleaner as clean_rows()
participant WidthCalc as calculate_widths()
participant Formatter as format_rows()
participant TableFmt as format_separator_cells()
participant Emitter as Output
Client->>Buffer: call flush(buffered_lines, ellipsis?)
alt Detected as table block
Buffer->>Buffer: replace_ellipsis(buffered) if ellipsis enabled
Buffer->>Protector: insert leading-empty markers per line
Protector->>Parser: feed protected lines
Parser->>Cleaner: parsed rows + split flag
Cleaner->>WidthCalc: rows (markers restored, empty rows removed)
WidthCalc->>Formatter: column widths (unicode display width)
Formatter->>TableFmt: separator cells + widths
TableFmt->>Formatter: formatted separator (or signal invalid)
alt Separator valid
Formatter->>Emitter: emit reflowed table lines
else Separator invalid
Formatter->>Emitter: emit original input lines (fallback)
end
else Not a table
Buffer->>Emitter: emit buffered lines unchanged
end
Poem
🚥 Pre-merge checks | ✅ 8 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Reviewer's GuideRefactors table reflow to preserve leading empty cells in continuation rows, add optional ellipsis-aware processing, and use Unicode-aware width/padding with minimum column widths for separator tables, along with regression tests and updated expectations. Sequence diagram for ellipsis-aware table processing in process_stream_innersequenceDiagram
actor User
participant CLI as cli_main
participant Proc as process_stream
participant Core as process_stream_inner
participant Buf as ProcessBuffer
participant Reflow as reflow_table
User->>CLI: invoke_mdtablefix(input, opts{ellipsis})
CLI->>Proc: process_stream(lines, opts)
Proc->>Core: process_stream_inner(lines, opts)
Core->>Buf: init ProcessBuffer{out, buf, in_table=false, ellipsis=opts.ellipsis}
loop for each line
Core->>Buf: push line into buf
Core->>Buf: maybe_set in_table
end
Core->>Buf: flush()
alt in_table == true
Buf->>Buf: buffered = take(buf)
alt ellipsis == true
Buf->>Reflow: replace_ellipsis(buffered)
Reflow-->>Buf: table_lines
else ellipsis == false
Buf-->>Buf: table_lines = buffered
end
Buf->>Reflow: reflow_table(table_lines)
Reflow-->>Buf: reflowed_lines
Buf-->>Core: out.extend(reflowed_lines)
else in_table == false
Buf-->>Core: out.extend(buffered)
end
Core-->>Proc: out
Proc-->>CLI: final_lines
CLI-->>User: formatted_markdown
Class diagram for ProcessBuffer and table reflow helpers with Unicode-aware widthsclassDiagram
class ProcessBuffer {
Vec_String out
Vec_String buf
bool in_table
bool ellipsis
flush()
}
class Options {
bool ellipsis
}
class ReflowModule {
<<module>>
parse_rows(trimmed_lines)
clean_rows(rows)
calculate_widths(rows, max_cols) Vec_usize
format_rows(rows, widths, indent) Vec_String
protect_leading_empty_cells(line) String
pad_cell_to_width(cell, width) String
insert_separator(out_rows, sep_cells, widths, indent) Vec_String
reflow_table(lines) Vec_String
}
class TableModule {
<<module>>
split_cells(line) Vec_String
format_separator_cells(sep_cells, widths) Vec_String
calculate_and_format(parsed_table, indent) Vec_String
}
class ParsedTable {
Vec_Vec_String cleaned
Vec_Vec_String output_rows
Option_Vec_String sep_cells
usize max_cols
}
class UnicodeWidthStr {
<<external>>
width(s) usize
}
class ProcessStream {
<<function>>
process_stream_inner(lines, opts) Vec_String
}
Options --> ProcessBuffer : initializes
ProcessStream --> ProcessBuffer : owns
ProcessStream ..> ReflowModule : uses
ProcessStream ..> TableModule : uses
ReflowModule ..> TableModule : split_cells
ReflowModule ..> UnicodeWidthStr : width()
TableModule --> ParsedTable : returns
TableModule ..> ReflowModule : calculate_widths\nformat_rows\ninsert_separator
ParsedTable --> ReflowModule : cleaned\noutput_rows\nmax_cols
ParsedTable --> TableModule : sep_cells
Flow diagram for parse_rows with leading empty cell protection and cleanupflowchart TD
A["Input trimmed table lines"] --> B["Map each line via protect_leading_empty_cells"]
B --> C["Join protected lines with space to raw"]
C --> D["Split raw into chunks using SENTINEL_RE"]
D --> E["Convert chunks into cells and rows"]
E --> F["clean_rows"]
subgraph ProtectLeadingEmptyCells
B --> G["split_cells(line)"]
G --> H["Count leading empty cells"]
H --> I{Any leading
empty cells?}
I -- Yes --> J["Replace each leading empty cell with LEADING_EMPTY_CELL_MARKER"]
J --> K["Rebuild line as | cells |"]
I -- No --> L["Return original line"]
end
subgraph CleanRows
F --> M["For each row: map cells"]
M --> N{cell == LEADING_EMPTY_CELL_MARKER?}
N -- Yes --> O["Replace with empty string"]
N -- No --> P["Keep cell unchanged"]
O --> Q["Row reconstructed"]
P --> Q
Q --> R{Row has any
nonempty cell?}
R -- Yes --> S["Keep row"]
R -- No --> T["Drop row"]
end
S --> U["Cleaned rows returned"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/reflow.rs`:
- Around line 157-175: When reconstructing the protected row after split_cells,
re-escape any literal pipe characters in non-leading cells so they don't become
new delimiters on a second parse; in the protected_cells mapping (the code that
builds protected_cells using split_cells, leading_empty_cells and
LEADING_EMPTY_CELL_MARKER), replace any '|' characters in the cell string with
the escaped sequence (e.g., '\|') for idx >= leading_empty_cells before joining
and formatting the final row returned by format!("| {} |", ...).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: fa98a083-5c64-4cb1-9864-a157663e9cdf
📒 Files selected for processing (4)
src/process.rssrc/reflow.rssrc/table.rstests/table_continuations.rs
|
@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. |
|
@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. ❌ Failed checks (3 warnings, 1 inconclusive)
|
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/execplans/yaml-frontmatter.md`:
- Around line 111-115: The decision-log bullet "Decision: use a shared internal
splitter for leading YAML frontmatter rather than adding special cases
separately in each transform. Rationale: one detector keeps the delimiter rules
consistent and reduces the chance that a later pipeline stage mutates the
protected prefix. Date/Author: 2026-04-05 22:45Z / Droid" (and the other
decision entry referenced) must be terminated with a full stop; update those
decision entries in yaml-frontmatter.md so each sentence ends with a period (add
missing trailing punctuation to the "Decision:", "Rationale:" and "Date/Author:"
fragments where appropriate) to conform to the en-GB-oxendict grammar rule.
- Around line 92-94: Update the checklist entry that currently allows skipping
`make nixie` so that `make nixie` is listed as a required validator (remove any
conditional/skip wording); modify the line containing the run commands ("Run
`make check-fmt`, `make lint`, `make test`, `make markdownlint`, and `make
nixie`") to present all five commands as mandatory and apply the same change to
the duplicate occurrence referenced (lines ~259-260).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c33172d2-c52f-4487-932f-f5ce3dfec7fc
📒 Files selected for processing (9)
docs/adrs/0001-table-reflow-pipeline.mddocs/architecture.mddocs/documentation-style-guide.mddocs/execplans/yaml-frontmatter.mddocs/rust-doctest-dry-guide.mddocs/rust-testing-with-rstest-fixtures.mdsrc/reflow.rssrc/table.rstests/table_continuations.rs
This comment was marked as resolved.
This comment was marked as resolved.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 37366
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 62432 🏁 Script executed: Length of output: 4824
✅ Testing — ResolvedDirect unit tests for all three private helpers are present in
✅ User-Facing Documentation — Resolved
✅ Developer Documentation — Resolved
✅ Module-Level Documentation — ResolvedAll seven
|
…d ellipsis support - Added support to preserve leading empty cells in table continuation rows by marking them during parsing and cleaning up markers before output. - Improved table reflow to calculate column widths using unicode widths and pad cells accordingly. - Added an option to replace ellipsis occurrences in tables during processing. - Introduced tests covering Markdown tables with continuation rows and ellipsis handling. - Ensured separator rows have a minimum column width for consistent formatting. These changes improve Markdown table formatting fidelity, especially for continued rows and truncated content representation. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
* feat(table-reflow): preserve table structure during reflow with continuation rows - Protect leading empty continuation cells with a private marker before row splitting. - Re-escape literal pipe characters in continuation row cells to preserve boundaries. - Measure column widths using Unicode display width for proper alignment. - Ensure separator columns have minimum width of three dashes with correct alignment markers. - Apply ellipsis replacements before reflow to align by final rendered width. This change fixes malformed table regressions affecting continuation rows, escaped pipes, and ellipsis handling, maintaining deterministic and testable behavior as documented in ADR 0001. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * docs(adr): clarify ADR title and terminology; update ellipsis test - Changed ADR title to full form 'Architecture Decision Record (ADR) 0001' in the ADR file and references. - Replaced 'CJK' abbreviation with full 'Chinese, Japanese, and Korean (CJK)' in architecture.md for clarity. - Updated a test in table_continuations.rs to use process_stream_opts with ellipsis option disabled, improving test accuracy. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * fix(reflow): properly escape literal pipes in table cell formatting Previously, literal pipe characters in table cells were not being escaped correctly when calculating widths and formatting output rows, causing misalignment and rendering issues. This change introduces functions to escape literal pipes and account for their width correctly when calculating cell widths and padding. Tests have been added and updated to verify escaped pipes are handled properly. Also fixed table continuation rendering to preserve escaped pipes in output. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> --------- Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
…oper guides - Implement a staged table reflow pipeline in `src/reflow.rs` supporting continuation rows, separator detection, Unicode width-based column sizing, escaping of literal pipes, and reinsertion of separator rows. - Add detailed internal API documentation and design rationale in `docs/developers-guide.md`. - Add a comprehensive user guide on table reflow and ellipsis handling in `docs/users-guide.md`. - Enhance process functions with examples for table reflow and ellipsis flag. - Improve formatting of separator cells preserving Markdown alignment markers in `src/table.rs`. - Fix minor markdown lint and formatting updates in execution plans documentation. - Include tests verifying escaping logic, width calculations, padding, and ellipsis transformations. This feature improves Markdown table formatting robustness, ensuring visually aligned columns with proper handling of Unicode and special characters, while documenting the implementation and usage thoroughly. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
d71e050 to
07d5d05
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/table.rs (1)
65-68:⚠️ Potential issue | 🟠 MajorReject over-wide separator rows instead of re-emitting them.
insert_separatorinsrc/reflow.rspads only short separator vectors, so this branch is the path for explicit separators with extra columns. Returning them unchanged still reformats the header/body rows, which breaks thereflow_tablecontract that invalid tables are returned unchanged. Surface this mismatch as a parse failure instead of reinserting it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/table.rs` around lines 65 - 68, The function format_separator_cells currently returns sep_cells unchanged when sep_cells.len() != widths.len(), which re-inserts over-wide separator rows instead of signaling a parse failure; change that branch to reject mismatched separator vectors (do not return the provided sep_cells) and return a clear failure sentinel (e.g., an empty Vec<String>) so callers like insert_separator and reflow_table detect the parse failure and leave the table unchanged; update any caller logic if needed to treat an empty Vec from format_separator_cells as a parse failure rather than valid formatted cells.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/adrs/0001-table-reflow-pipeline.md`:
- Line 1: Update the ADR heading "# Architecture Decision Record (ADR) 0001:
preserve table structure during reflow" to use sentence case after the colon by
capitalizing the first word "preserve" (i.e., change "preserve" to "Preserve")
so the title reads "Architecture Decision Record (ADR) 0001: Preserve table
structure during reflow".
In `@src/reflow.rs`:
- Around line 350-460: The file exceeds the 400-line cap because the
#[cfg(test)] mod tests { ... } block (which contains tests referencing functions
like protect_leading_empty_cells, clean_rows, escape_literal_pipes,
emitted_cell_width, pad_cell_to_width, calculate_widths, format_rows and uses
rstest and UnicodeWidthStr) is embedded in src/reflow.rs; extract that entire
tests module into a new sibling file (e.g., create a new src/reflow_tests.rs)
and replace the inline block in reflow.rs with a single line declaration
#[cfg(test)] mod tests; so the tests still use super::* and rstest imports but
live in the separate file, keeping reflow.rs under 400 lines.
In `@src/table.rs`:
- Around line 172-179: The width calculation is using parsed.cleaned which still
includes a promoted separator row, causing columns to be sized from the
separator; update calculate_and_format to call crate::reflow::calculate_widths
with parsed.output_rows (not parsed.cleaned) so widths are derived from
header/body, then continue to clamp widths to max(3) when
parsed.sep_cells.is_some(), and leave the subsequent crate::reflow::format_rows
and crate::reflow::insert_separator calls unchanged (references:
calculate_and_format, crate::reflow::calculate_widths, parsed.output_rows,
parsed.cleaned, parsed.sep_cells).
---
Outside diff comments:
In `@src/table.rs`:
- Around line 65-68: The function format_separator_cells currently returns
sep_cells unchanged when sep_cells.len() != widths.len(), which re-inserts
over-wide separator rows instead of signaling a parse failure; change that
branch to reject mismatched separator vectors (do not return the provided
sep_cells) and return a clear failure sentinel (e.g., an empty Vec<String>) so
callers like insert_separator and reflow_table detect the parse failure and
leave the table unchanged; update any caller logic if needed to treat an empty
Vec from format_separator_cells as a parse failure rather than valid formatted
cells.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 32f98260-edef-413a-9656-3e1f6d65ff22
📒 Files selected for processing (10)
docs/adrs/0001-table-reflow-pipeline.mddocs/architecture.mddocs/developers-guide.mddocs/execplans/yaml-frontmatter.mddocs/users-guide.mdsrc/process.rssrc/reflow.rssrc/table.rssrc/wrap/tokenize/mod.rstests/table_continuations.rs
…reflow - Changed format_separator_cells to return empty vector if length mismatch, so invalid separator rows are detected. - Added handling in calculate_and_format to return None when separator formatting is invalid, preventing table reflow. - Made reflow_table return original input lines if separator row is invalid, preserving original table. - Refactored tests by moving them out of reflow.rs into reflow/tests.rs with expanded coverage. - Minor docs fix in ADR title case. This prevents malformed Markdown tables with invalid separator rows from being reflowed incorrectly, improving stability and correctness of the table reflow pipeline. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
Introduce `SEP_RE`, a static regular expression matching Markdown table separator lines composed only of pipes, colons, dashes, and whitespace. This enables accurate detection and extraction of the alignment row during table parsing. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/table.rs (1)
77-85:⚠️ Potential issue | 🟠 MajorEnforce a three-dash minimum after applying alignment markers.
Fix Line 77 logic so aligned separator cells still contain at least three dashes. The current flow can emit
:-:for centred alignment at width3, which violates Markdown separator rules and the PR’s minimum-dash intent. Add a regression case in the rstest block.Patch
@@ - let mut dashes = "-".repeat(widths[i].max(3)); + let min_len = 3 + usize::from(left) + usize::from(right); + let target_len = widths[i].max(min_len); + let mut dashes = "-".repeat(target_len); if left { dashes.remove(0); dashes.insert(0, ':'); } if right { dashes.pop(); dashes.push(':'); } dashes }) .collect() } @@ #[rstest] #[case(vec![2], vec!["---".to_string()], vec!["---".to_string()])] + #[case(vec![3], vec![":--:".to_string()], vec![":---:".to_string()])] #[case(vec![5], vec![":---".to_string()], vec![":----".to_string()])] #[case(vec![5], vec!["---:".to_string()], vec!["----:".to_string()])] #[case(vec![5], vec![":--:".to_string()], vec![":---:".to_string()])]Also applies to: 269-274
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/table.rs` around lines 77 - 85, The separator-generation logic currently builds dashes with "-".repeat(widths[i].max(3)) and then mutates them for left/right alignment, which can produce fewer than three interior dashes (e.g., ":-:"); change it to compute min_len = 3 + usize::from(left) + usize::from(right) and use target_len = widths[i].max(min_len) when creating dashes so alignment markers are included in the minimum width, then apply the existing left/right insertions on that string (referencing widths[i], left, right, and dashes). Also add the rstest regression case that expects ":---:" for an input width of 3 to the existing test block that contains cases like vec![2] and vec![5].
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/table.rs`:
- Around line 77-85: The separator-generation logic currently builds dashes with
"-".repeat(widths[i].max(3)) and then mutates them for left/right alignment,
which can produce fewer than three interior dashes (e.g., ":-:"); change it to
compute min_len = 3 + usize::from(left) + usize::from(right) and use target_len
= widths[i].max(min_len) when creating dashes so alignment markers are included
in the minimum width, then apply the existing left/right insertions on that
string (referencing widths[i], left, right, and dashes). Also add the rstest
regression case that expects ":---:" for an input width of 3 to the existing
test block that contains cases like vec![2] and vec![5].
Summary
Changes
Documentation
Tests
Test Plan
Potential Impact
📎 Task: https://www.devboxer.com/task/bdf32d63-a27d-41fc-8a83-b480791b5ddc
📎 Task: https://www.devboxer.com/task/76f086fe-74e1-4c40-b366-aa9edf3940ad
📎 Task: https://www.devboxer.com/task/d99b9fbc-b00d-4c00-aab1-1b14e4c9a375
📎 Task: https://www.devboxer.com/task/22be2748-8bcc-4708-8429-541b372e777e
📎 Task: https://www.devboxer.com/task/e2d0b3a9-732f-498e-ba08-625f76da0beb