Skip to content

Guard thematic-break test access - #554

Open
leynos wants to merge 1 commit into
harden-lint-ellipsis-after-spanfrom
harden-lint-breaks-after-ellipsis
Open

leynos wants to merge 1 commit into
harden-lint-ellipsis-after-spanfrom
harden-lint-breaks-after-ellipsis

Conversation

@leynos

@leynos leynos commented Sep 24, 2026 •

Copy link
Copy Markdown
Owner

Summary

This branch replaces unchecked output indexing in thematic-break unit, property,
and integration tests with slice patterns and checked lookups. The assertions
still distinguish input-borrowed lines, the shared canonical break, and fenced
content; production formatting is unchanged. It follows #553 in the Rust
baseline train.

Review walkthrough

  • Start with src/breaks.rs for unit and property assertions, including the LazyLock pointer-identity check.
  • Review tests/breaks.rs for integration test lookups and borrowing assertions.

Validation

  • make fmt: passed; no unrelated files changed.
  • make check-fmt, make lint, make test, make typecheck, make markdownlint, make nixie, make verus, and make verus-selftest: passed sequentially on 64db4be's tree (attempt 2 logs under /tmp/mdtablefix-breaks-combined-20260924-attempt2-*.out).
  • cs delta: no issues found.
  • Independent static review: no behavioural finding; the borrowing, fence, output-order, and pointer-identity assertions remain intact.

Notes

The #550 proposed-final-lint checkpoint mapped 19 findings in the source test
module to this layer. A separate static count found 10 integration-test
indexing sites. Clearance under the final configuration remains to be
remeasured; the checkpoint could not compile through the library to any of 56
integration roots. Final lint and Whitaker enforcement belong to later train
layers, including the recorded Whitaker binary-pin blocker.

References

Replace unchecked output indexing in unit, property, and integration tests with slice patterns and checked lookups. Preserve the assertions that breaks borrow from the shared static, ordinary lines borrow from the input, and fenced content stays unchanged.
@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: 5cd921f9-03f3-4873-b225-31f891c68fba

📥 Commits

Reviewing files that changed from the base of the PR and between be95204 and 64db4be.

📒 Files selected for processing (2)
  • src/breaks.rs
  • tests/breaks.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.


Summary

  • Replace unchecked indexing in thematic-break tests with slice patterns and checked lookups.
  • Preserve assertions for normalized break values, borrowing from input and the shared canonical break, and fenced-content preservation.
  • Match Cow values directly in the borrowed-value assertion macro.
  • No production formatting changes or public API changes are reported.

Walkthrough

The test updates change how break-formatting outputs are inspected. They use slice destructuring, direct Cow matching, and checked element access. Existing expectations for formatted values, borrowing, fences, and shared static pointers remain.

Changes

Break formatting tests

Layer / File(s) Summary
Break-formatting assertions
src/breaks.rs, tests/breaks.rs
Tests inspect output shapes with slice destructuring and checked element access. They match Cow values directly and retain existing value, borrowing, fence, and pointer checks.

Suggested labels: Issue

Priority: ⬇️ Low

Change: Refactor

Merge Risk: ⚪ Minimal · up to 64db4

The test updates retain their existing checks, and no merge-blocking issue remains after normal checks.

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: guarding access in thematic-break tests. No roadmap item or issue fix requires an additional reference in the title.
Description check ✅ Passed The description directly explains the test refactor, preserved assertions, validation results, and related references.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 2 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.
Testing (Overall) ✅ Passed Pass this check. The pull request changes only test access and assertion syntax in src/breaks.rs and tests/breaks.rs; the format_breaks implementation is identical at the base and head refs. The…
User-Facing Documentation ✅ Passed Pass the documentation check. The diff changes only test code in src/breaks.rs and tests/breaks.rs. It adds guarded test access and does not change format_breaks, exported entities, CLI behaviou…
Developer Documentation ✅ Passed Pass the developer documentation check. The authoritative diff changes only test code in src/breaks.rs and tests/breaks.rs. It changes assertion access and test input construction. It does not cha…
Module-Level Documentation ✅ Passed Pass the module-level documentation check. The changed modules already have module docstrings: src/breaks.rs documents thematic-break formatting, tests/breaks.rs documents the integration tests, a…
Testing (Unit And Behavioural) ✅ Passed Pass the testing check. The PR changes only test access and assertion syntax; it does not change production behaviour or remove tests. Unit and property tests retain coverage for normalisation, fence …
Testing (Property / Proof) ✅ Passed Keep the check passed. The pull request changes only test code in src/breaks.rs and tests/breaks.rs; format_breaks production code is unchanged. The existing proptest suite already covers arbi…
Testing (Compile-Time / Ui) ✅ Passed Pass the check. The authoritative diff changes only test assertions and test data in src/breaks.rs and tests/breaks.rs; it does not change production behaviour, public APIs, compile-time diagnosti…
Unit Architecture ✅ Passed Pass the Unit Architecture check. The authoritative diff changes only test code in src/breaks.rs and tests/breaks.rs; the production prefix containing format_breaks is identical at base and head…
Domain Architecture ✅ Passed Pass this check. The pull request changes only test code in src/breaks.rs and tests/breaks.rs. The production section of src/breaks.rs is unchanged. The changes add checked test-output access an…
Observability ✅ Passed The pull request changes only test code in src/breaks.rs and tests/breaks.rs. format_breaks and other production behaviour remain unchanged. No new operational failure mode, process boundary, me…

Breaks line up in slices, neat and clear
Borrowed values stay in view
Fences keep their edges true
Tests check each step with care
No changed expectation slips through there

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

The PR hardens thematic-break unit, property, and integration tests by replacing unchecked output indexing with slice patterns or checked lookups, while preserving all output ordering, fence handling, borrowing, and canonical pointer-identity assertions; production code is unchanged.

File-Level Changes

Change Details Files
Replace unchecked test-output indexing with shape-aware slice matching while preserving borrowing and identity assertions.
  • Destructure fixed-size outputs in unit tests and require the expected number of lines.
  • Match property-test outputs and zipped source/output entries without indexing, with explicit failure cases for missing or unexpected shapes.
  • Keep assertions for input borrowing, canonical thematic-break borrowing, fenced content, output lengths, and pointer identity intact.
src/breaks.rs
Harden integration-test output access with checked lookups and descriptive failure messages.
  • Replace direct indices with first() and get(...).expect(...) in basic, fenced, and single-line cases.
  • Preserve borrowed-value and shared thematic-break assertions.
tests/breaks.rs
Make incidental test setup and bindings clearer while retaining behavior.
  • Use array mapping with to_owned() for small test inputs.
  • Rename the thread barrier clone and simplify the borrowed-value macro match.
src/breaks.rs
tests/breaks.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

@leynos
leynos marked this pull request as ready for review September 24, 2026 03:57

@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 6 hours and 49 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-24T03:59:19.180211Z 64db4be 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.

@buzzybee-df12

Copy link
Copy Markdown

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 25, 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 added the Issue label Sep 25, 2026

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants