Skip to content

Guard reflow row and width access - #549

Open
leynos wants to merge 1 commit into
harden-lint-ellipsis-protectedfrom
harden-lint-reflow
Open

leynos wants to merge 1 commit into
harden-lint-ellipsis-protectedfrom
harden-lint-reflow

Conversation

@leynos

@leynos leynos commented Sep 24, 2026 •

Copy link
Copy Markdown
Owner

Summary

This layer guards row and width access in reflow and replaces string conversions that the proposed stricter Clippy baseline would reject. It preserves the current table output and remains under the existing lint configuration. It is stacked on #548 (harden-lint-ellipsis-protected); later source-fix layers and final lint enforcement remain separate.

Review walkthrough

Validation

At commit a55bf7d4fa95aa0e353647f03b9a1eff766e372d on PR #548
28b60698a86abe2838c330d6c80bd648a9f5bdf0, the one child commit
retains stable patch ID f52ae145cf6c9dfed9054517df2708d292ccd535,
a 1:1 range-diff, and an identical raw child diff. The replay had no
textual conflicts or extra paths.

All eight Make gates passed sequentially: make check-fmt, make lint,
make test, make typecheck, make markdownlint, make nixie,
make verus, and make verus-selftest. Captured logs are under
/tmp/pr549-restack-<gate>-a55bf7d.out. Local
cs delta 28b60698a86abe2838c330d6c80bd648a9f5bdf0 a55bf7d4fa95aa0e353647f03b9a1eff766e372d
reported no issues. Independent review of the original child patch found
no correctness issue in the checked-access replacements.

Hosted CI, Verus, and CodeScene checks are pending for this rewritten head.
The prior head's green hosted results are historical. Managed CodeRabbit
request ae5d7093 remains queued; a skipped automatic check is not a review.

Notes

The production caller derives max_cols from the maximum parsed row length, removes at most the separator row, then calculates and passes a widths vector of that same length to formatting. The short-width and overflow fallbacks therefore defend against inconsistent internal calls without changing the production output. Zero-width row splitting returns before chunking; the divisibility guard keeps the checked ranges valid.

Phase-0 proposed-baseline measurement reported at least 997 Clippy sites before source remediation. Integration targets behind a root compilation failure and the Whitaker gate remain unmeasured; this PR does not claim overall baseline compliance. The selected Whitaker binary/pin and generated spelling policy are tracked separately.

References

@coderabbitai

coderabbitai Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

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

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Team

Run ID: b1e5c4d2-db6e-4d97-b4ad-d9c42e565c57

📥 Commits

Reviewing files that changed from the base of the PR and between 28b6069 and a55bf7d.

📒 Files selected for processing (3)
  • src/reflow/mod.rs
  • src/reflow/row_parsing.rs
  • src/reflow/tests/mod.rs
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • leynos/netsuke (auto-detected)
  • leynos/df12-dylint-builds (auto-detected)
  • leynos/typos-config-builder (auto-detected)
  • leynos/falcon-correlate (auto-detected)
  • leynos/msgspec-crockford (auto-detected)
  • leynos/vk (auto-detected)
  • leynos/simulacat-core (auto-detected)
  • leynos/agent-template-python (auto-detected)
  • leynos/shared-actions (auto-detected)
  • leynos/cuprum (auto-detected)

Included review availability: 3 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour.


  • Replace direct row and width indexing in reflow with checked access. Default missing width entries to zero, and fail the corresponding checks when row ranges are missing.
  • Check trailing cells before truncating the first row. Use checked slices when detecting concatenated rows and separator rows.
  • Replace .to_string() with .to_owned() in examples and tests. Keep test inputs and expected values unchanged.
  • Align these parser safeguards with ADR 0001: Preserve table structure during reflow.
  • No public declarations changed.

Walkthrough

Reflow width, formatting, separator detection and row parsing now use checked access for missing rows, cells or ranges. Examples and tests replace .to_string() with .to_owned(); test inputs and assertions remain unchanged.

Changes

Reflow bounds checks

Layer / File(s) Summary
Bounds-safe reflow operations
src/reflow/mod.rs, src/reflow/tests/mod.rs
Width calculation skips cells without a corresponding width. Formatting uses a zero width for those cells. Separator detection uses optional row lookups. Related examples and test strings use .to_owned().
Checked row-parsing ranges
src/reflow/row_parsing.rs, src/reflow/mod.rs, src/reflow/tests/mod.rs
Row parsing uses checked ranges for padding, embedded separators, inter-row separators and row contents. Parser examples and test strings use .to_owned().

Priority: ⬇️ Low

Change: Bug fix

Merge Risk: ⚪ Minimal · up to a55bf

This change does not appear to alter valid table parsing or formatting. The reviewed callers preserve matching dimensions, and no actionable mergeability risk was identified.


Caution

Pre-merge checks failed

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

  • Ignore

❌ Failed checks (2 errors)

Check name Status Explanation Resolution
Testing (Overall) ❌ Error The changed bounds fallbacks are not tested. The test diff only changes string conversions, and existing width tests pass matching dimensions. They do not exercise rows wider than max_cols in `calcu… Add focused tests for both mismatched-dimension cases. Assert that calculate_widths ignores excess cells without panicking and that format_rows formats cells with missing widths using the documented zero-width fallback, while preserving…
Testing (Unit And Behavioural) ❌ Error The existing suite checks normal width calculation, formatting, row parsing, and generated concatenated rows. However, the test diff changes only string conversions. No test exercises the new mismatch… Add focused unit tests that pass a row wider than max_cols to calculate_widths and a width slice shorter than a row to format_rows. Assert the documented fallback results and verify that neither call panics. Add tests for any other ne…
✅ Passed checks (13 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the checked row and width access changes. The description references stacked PR #548, not an existing issue that requires an issue number in this title.
Description check ✅ Passed The description explains the reflow access changes, string conversion updates, and reported validation. It is related to the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 90.91% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 22 functions across 3 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
User-Facing Documentation ✅ Passed No new user-facing behaviour needs documentation. The diff changes only crate-private reflow helpers and replaces string conversions in examples and tests. The production caller derives max_cols fro…
Developer Documentation ✅ Passed Keep the existing developer documentation. The diff changes only bounds-checked access in private reflow helpers and .to_string() conversions; it adds no API, architectural boundary, abstraction, to…
Module-Level Documentation ✅ Passed PASS. The changed Rust modules have module-level //! documentation. src/reflow/mod.rs explains the reflow helpers and their relationship to reflow_table; src/reflow/row_parsing.rs describes re…
Testing (Property / Proof) ✅ Passed The change does not introduce a complex invariant that needs property testing. Existing proptest cases exercise row-boundary preservation and recovery across generated table widths and row counts. The…
Testing (Compile-Time / Ui) ✅ Passed Pass. Treat the compile-time test requirement as not applicable: the diff changes runtime reflow access and string conversions, with no compile-time behaviour. Keep the existing focused exact-output t…
Unit Architecture ✅ Passed Keep the existing separation. The diff changes only in-memory row parsing, width calculation, and formatting helpers, and replaces test/example string conversions. The checked accesses add no external…
Domain Architecture ✅ Passed Keep the existing domain boundary. The changed production code remains limited to Markdown table row parsing, separator detection, width calculation and formatting; it adds checked slice and width acc…
Observability ✅ Passed The changed code adds bounds checks to internal reflow helpers and defaults to zero or false only when an expected row or width range is absent. The production caller derives max_cols from parsed ro…
Full details: Testing (Overall)

Explanation

The changed bounds fallbacks are not tested. The test diff only changes string conversions, and existing width tests pass matching dimensions. They do not exercise rows wider than max_cols in calculate_widths or fewer widths than cells in format_rows.

Resolution

Add focused tests for both mismatched-dimension cases. Assert that calculate_widths ignores excess cells without panicking and that format_rows formats cells with missing widths using the documented zero-width fallback, while preserving output for columns with supplied widths.

Full details: Testing (Unit And Behavioural)

Explanation

The existing suite checks normal width calculation, formatting, row parsing, and generated concatenated rows. However, the test diff changes only string conversions. No test exercises the new mismatched-dimension behaviour: calculate_widths receiving more cells than max_cols, or format_rows receiving fewer widths than cells. The changed guards therefore lack tests for the edge cases they were added to handle.

Resolution

Add focused unit tests that pass a row wider than max_cols to calculate_widths and a width slice shorter than a row to format_rows. Assert the documented fallback results and verify that neither call panics. Add tests for any other newly guarded input shapes that can be constructed at the helper boundary; retain the existing parser property tests for valid concatenated rows.


Rows meet their bounds with care
Widths default when none are there
Separators meet checked eyes
Missing ranges cannot surprise
Owned strings now fill the tests
Reflow follows safer rests

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

@sourcery-ai

sourcery-ai Bot commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

This PR hardens reflow against malformed or inconsistent internal row/width shapes by using checked access throughout parsing, width calculation, formatting, and separator detection, while replacing rejected string conversions and retaining existing behavioral coverage.

Flow diagram for guarded reflow processing

flowchart TD
    Rows[Parsed rows] --> Parse[split_physical_rows]
    Parse --> Separator[detect_separator]
    Separator --> Widths[calculate_widths]
    Widths --> Format[format_rows]
    Format --> Output[Formatted table output]
    Parse -. checked slices .-> Guard[Safe fallback on inconsistent shapes]
    Widths -. checked width access .-> Guard
    Format -. missing width defaults to zero .-> Guard
    Separator -. checked second-row access .-> Guard
Loading

File-Level Changes

Change Details Files
Guard reflow parsing and formatting against inconsistent row and width shapes by replacing unchecked indexing and slicing with checked access.
  • Use checked slices and element access when trimming, validating, and detecting concatenated rows.
  • Avoid panics from missing width entries and absent second rows while preserving normal output behavior.
src/reflow/row_parsing.rs
src/reflow/mod.rs
Replace explicit string conversions with idiomatic owned-string construction to satisfy the stricter Clippy baseline.
  • Convert production documentation examples and test fixtures from .to_string() to .to_owned().
src/reflow/mod.rs
src/reflow/tests/mod.rs
Retain regression and property coverage for reflow behavior after the safety and lint-oriented changes.
  • Keep exact-output tests for parsing, structural row recovery, Unicode widths, and pipe escaping.
  • Preserve property-based coverage for generated cell and legacy concatenated-row inputs.
src/reflow/tests/mod.rs

Tips and commands

Interacting with Sourcery

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

Customizing Your Experience

Access your dashboard to:

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

Getting Help

codescene-access[bot]

This comment was marked as outdated.

@leynos
leynos marked this pull request as ready for review September 24, 2026 02:18

@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've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 8 hours and 39 minutes by commenting @sourcery-ai review. Upgrade to get a review now.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-24T02:20:14.442228Z 0061954 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Replace direct row and width indexing with checked access, preserving the existing table output while making internal dimension mismatches safe. Use owned string conversions in the reflow tests and examples to pre-empt the stricter lint baseline.
@leynos
leynos added this pull request to stack #557 September 24, 2026 16:15
@buzzybee-df12

Copy link
Copy Markdown

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

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

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

Pre-merge checks failed. Please resolve the failing checks before merging.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants