Skip to content

Add renumber restart test#82

Merged
leynos merged 4 commits intomainfrom
codex/add-test-for-list-renumbering-behavior
Jul 17, 2025
Merged

Add renumber restart test#82
leynos merged 4 commits intomainfrom
codex/add-test-for-list-renumbering-behavior

Conversation

@leynos
Copy link
Copy Markdown
Owner

@leynos leynos commented Jul 17, 2025

Summary

  • check list renumbering resets after lower indentation paragraphs
  • implement paragraph detection in renumber_lists

Testing

  • make lint
  • make test

https://chatgpt.com/codex/tasks/task_e_68783b2c18b883228ee08234b4788f4f

Summary by Sourcery

Improve list renumbering to correctly restart numbering after paragraphs with lower indentation and add corresponding integration test.

Enhancements:

  • Update list renumbering logic to reset numbering after paragraphs with lower indentation.

Tests:

  • Add integration test to verify list renumbering resets after paragraphs.

@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented Jul 17, 2025

Reviewer's Guide

This pull request enhances the list renumbering logic to correctly reset numbering after paragraphs with lower indentation and adds a corresponding integration test to verify the behavior.

Sequence diagram for list renumbering reset after paragraph

sequenceDiagram
    participant User
    participant renumber_lists
    participant is_plain_paragraph_line
    User->>renumber_lists: Provide lines with lists and paragraphs
    renumber_lists->>is_plain_paragraph_line: Check if line is a plain paragraph
    is_plain_paragraph_line-->>renumber_lists: Return true/false
    renumber_lists-->>User: Output lines with renumbered lists (reset after paragraph)
Loading

File-Level Changes

Change Details Files
Improve list renumbering to reset after lower-indented paragraphs.
  • Add is_plain_paragraph_line function to detect paragraph lines.
  • Update renumber_lists to reset counters when a paragraph with lower indentation is detected after a blank line.
  • Track previous blank lines to determine paragraph boundaries.
src/lists.rs
Add integration test for list renumbering reset after paragraphs.
  • Add test_renumber_restart_after_paragraph to verify renumbering behavior.
  • Include new input and expected output data files for the test.
tests/integration.rs
tests/data/renumber_paragraph_restart_input.txt
tests/data/renumber_paragraph_restart_expected.txt

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

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jul 17, 2025

Summary by CodeRabbit

  • New Features
    • Improved handling of list renumbering when a paragraph interrupts a numbered list, ensuring numbering restarts correctly after blank lines and paragraphs.
  • Tests
    • Added integration and unit tests to verify correct list renumbering behaviour with paragraphs, blank lines, headings, and nested content.
    • Introduced new test data files covering complex list and paragraph scenarios.

Summary by CodeRabbit

  • New Features

    • Improved handling of numbered lists so that numbering restarts correctly after a paragraph interrupts a list, even when mixed with tables or code blocks.
    • Added helper functions to detect plain paragraph lines and to pop list counters up to a given indentation level, enhancing control over list renumbering logic.
    • Introduced tracking of blank lines to influence when list counters reset during renumbering.
  • Tests

    • Added new integration test and test data to verify correct list renumbering after paragraphs, including scenarios with nested paragraphs, tables, and code blocks.
    • Added unit tests for the new helper functions and for the refined renumbering behaviour, validating list numbering restarts and indentation-sensitive counter management.

Walkthrough

This update enhances the list renumbering logic by adding detection of plain paragraph lines and tracking blank lines to reset list counters appropriately. It introduces helper functions for managing counters by indentation and adds comprehensive tests to ensure numbering restarts correctly after paragraphs interrupt lists, including complex nested content.

Changes

Files Change Summary
src/lists.rs Added is_plain_paragraph_line and pop_counters_upto functions; enhanced renumber_lists with blank line tracking and conditional counter popping; added handle_paragraph_restart to manage paragraph-induced counter resets.
tests/data/renumber_paragraph_restart_input.txt Added input test file with numbered lists containing nested paragraphs, tables, and code blocks.
tests/data/renumber_paragraph_restart_expected.txt Added expected output test file reflecting correct renumbering after paragraphs.
tests/integration.rs Added integration test test_renumber_restart_after_paragraph to verify renumbering behaviour with paragraphs.
tests/lists.rs Added unit tests for pop_counters_upto and renumber_lists covering blank line and paragraph scenarios.

Possibly related PRs

  • Add --renumber option to mdtablefix #54: Introduced the initial renumber_lists function and CLI support for list renumbering; this PR builds upon and modifies the same function with enhanced logic for paragraph interruptions.
  • Add renumber list table test #75: Added tests for renumbering lists containing tables without resetting counters; related by focus on list renumbering behaviour with complex content.
  • Refactor lib into smaller modules #79: Refactored the crate into modules and moved renumber_lists into src/lists.rs without changing its internal logic; this PR extends that modular foundation with new helper functions and improved renumbering behaviour.

Poem

📜 Lists once marched in steady line,
But paragraphs caused their decline.
Now counters pop and restart anew,
With blank lines watched and paragraphs true.
Markdown dances, neat and bright,
Numbering restored, just right! 🎉✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 19e5f93 and baa2d4b.

📒 Files selected for processing (1)
  • src/lists.rs (3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

Instructions used from:

Sources:
📄 CodeRabbit Inference Engine

  • AGENTS.md

⚙️ CodeRabbit Configuration File

🔇 Additional comments (1)
src/lists.rs (1)

71-71: Blank line tracking implementation is correct.

The modifications properly track blank line state throughout the function. The initialisation correctly handles empty input, and the updates are logically placed to maintain accurate state. The call to handle_paragraph_restart is appropriately positioned before drop_deeper.

Also applies to: 77-77, 83-83, 101-101, 111-111, 114-114

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex/add-test-for-list-renumbering-behavior

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai auto-generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0edd434 and 11622a3.

📒 Files selected for processing (4)
  • src/lists.rs (3 hunks)
  • tests/data/renumber_paragraph_restart_expected.txt (1 hunks)
  • tests/data/renumber_paragraph_restart_input.txt (1 hunks)
  • tests/integration.rs (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

Instructions used from:

Sources:
📄 CodeRabbit Inference Engine

  • AGENTS.md

⚙️ CodeRabbit Configuration File

🧬 Code Graph Analysis (2)
src/lists.rs (1)
src/wrap.rs (1)
  • is_fence (107-107)
tests/integration.rs (1)
src/lists.rs (1)
  • renumber_lists (37-96)
🔇 Additional comments (6)
tests/data/renumber_paragraph_restart_expected.txt (1)

1-27: Test data correctly demonstrates list restart behaviour.

The expected output properly shows list numbering restarting from 1 after the plain paragraph at line 9, which validates the core functionality described in the PR objectives.

tests/data/renumber_paragraph_restart_input.txt (1)

1-27: Input test data properly sets up the renumbering scenario.

The sequential numbering 1-7 provides a clear baseline to test against, with the plain paragraph at line 9 serving as the trigger point for list restart.

src/lists.rs (3)

41-41: Initialisation of prev_blank is correct.

Starting with true makes sense as we want to treat the beginning of input as if there was a preceding blank line.


47-47: prev_blank state tracking is correctly implemented.

The boolean is properly updated after processing fence lines, code blocks, numbered list lines, and other lines to maintain accurate blank line tracking.

Also applies to: 53-53, 71-71, 92-92


29-34: Add documentation for the new helper function.

The is_plain_paragraph_line function lacks Rustdoc comments. Document its purpose and behaviour according to the coding guidelines.

+/// Determines whether a line represents a plain paragraph by checking if it starts with an
+/// alphanumeric character after trimming leading whitespace.
+///
+/// # Arguments
+/// * `line` - The line to examine
+///
+/// # Returns
+/// `true` if the line starts with an alphanumeric character, `false` otherwise
 fn is_plain_paragraph_line(line: &str) -> bool {

Likely an incorrect or invalid review comment.

tests/integration.rs (1)

1013-1024: Integration test properly validates the new functionality.

The test correctly loads external test data, processes it through renumber_lists, and validates the expected restart behaviour. The implementation follows established patterns in the codebase.

Comment thread src/lists.rs Outdated
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @leynos - I've reviewed your changes and they look great!

Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments

### Comment 1
<location> `src/lists.rs:41` </location>
<code_context>
     let mut out = Vec::with_capacity(lines.len());
     let mut counters: Vec<(usize, usize)> = Vec::new();
     let mut in_code = false;
+    let mut prev_blank = true;

     for line in lines {
</code_context>

<issue_to_address>
Initializing prev_blank to true may cause the first line to be treated as following a blank line.

Consider initializing prev_blank based on whether the first line is blank to avoid unintended behavior at the start of the document.
</issue_to_address>

<suggested_fix>
<<<<<<< SEARCH
    let mut in_code = false;
    let mut prev_blank = true;

    for line in lines {
=======
    let mut in_code = false;
    let mut prev_blank = lines.first().map(|l| l.trim().is_empty()).unwrap_or(true);

    for line in lines {
>>>>>>> REPLACE

</suggested_fix>

### Comment 2
<location> `src/lists.rs:81` </location>
<code_context>
             .map_or_else(|| line.len(), |(i, _)| i);
         let indent_str = &line[..indent_end];
         let indent = indent_len(indent_str);
+        if prev_blank
+            && counters
+                .last()
</code_context>

<issue_to_address>
Consider extracting the paragraph-restart logic into its own function to simplify the main loop and improve code clarity.

Consider extracting the paragraph-restart logic into a dedicated function to reduce main loop complexity and improve readability. For example:

```rust
fn handle_paragraph_restart(
    line: &str,
    prev_blank: bool,
    counters: &mut Vec<(usize, usize)>
) {
    let indent_end = line
        .char_indices()
        .find(|&(_, c)| !c.is_whitespace())
        .map_or_else(|| line.len(), |(i, _)| i);
    let indent = indent_len(&line[..indent_end]);
    if prev_blank
        && counters
            .last()
            .is_some_and(|(d, _)| indent <= *d && is_plain_paragraph_line(line))
    {
        while counters.last().is_some_and(|(d, _)| *d >= indent) {
            counters.pop();
        }
    }
}
```

Then, in the main loop, replace the nested logic with a single call:

```rust
handle_paragraph_restart(line, prev_blank, &mut counters);
drop_deeper(indent, &mut counters);
out.push(line.clone());
prev_blank = line.trim().is_empty();
```

This keeps the main loop focused and makes the paragraph-restart logic easier to test and maintain.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src/lists.rs
Comment on lines 40 to 43
let mut in_code = false;
let mut prev_blank = true;

for line in lines {
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.

suggestion: Initializing prev_blank to true may cause the first line to be treated as following a blank line.

Consider initializing prev_blank based on whether the first line is blank to avoid unintended behavior at the start of the document.

Suggested change
let mut in_code = false;
let mut prev_blank = true;
for line in lines {
let mut in_code = false;
let mut prev_blank = lines.first().map(|l| l.trim().is_empty()).unwrap_or(true);
for line in lines {

Comment thread src/lists.rs Outdated
.map_or_else(|| line.len(), |(i, _)| i);
let indent_str = &line[..indent_end];
let indent = indent_len(indent_str);
if prev_blank
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.

issue (complexity): Consider extracting the paragraph-restart logic into its own function to simplify the main loop and improve code clarity.

Consider extracting the paragraph-restart logic into a dedicated function to reduce main loop complexity and improve readability. For example:

fn handle_paragraph_restart(
    line: &str,
    prev_blank: bool,
    counters: &mut Vec<(usize, usize)>
) {
    let indent_end = line
        .char_indices()
        .find(|&(_, c)| !c.is_whitespace())
        .map_or_else(|| line.len(), |(i, _)| i);
    let indent = indent_len(&line[..indent_end]);
    if prev_blank
        && counters
            .last()
            .is_some_and(|(d, _)| indent <= *d && is_plain_paragraph_line(line))
    {
        while counters.last().is_some_and(|(d, _)| *d >= indent) {
            counters.pop();
        }
    }
}

Then, in the main loop, replace the nested logic with a single call:

handle_paragraph_restart(line, prev_blank, &mut counters);
drop_deeper(indent, &mut counters);
out.push(line.clone());
prev_blank = line.trim().is_empty();

This keeps the main loop focused and makes the paragraph-restart logic easier to test and maintain.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (3)
src/lists.rs (3)

36-41: Address the past review feedback correctly.

This implementation addresses the past review suggestion to extract the helper function, but the #[doc(hidden)] attribute contradicts the requirement for public API documentation.

Apply this fix:

-#[doc(hidden)]
+/// Removes all counters with indentation greater than or equal to the specified level.
+/// 
+/// This function is used to reset list numbering when encountering paragraph content
+/// that interrupts nested lists at a given indentation level.
+/// 
+/// # Arguments
+/// * `counters` - Mutable reference to the counter stack
+/// * `indent` - The indentation level threshold for removal
 pub fn pop_counters_upto(counters: &mut Vec<(usize, usize)>, indent: usize) {

48-48: Fix the problematic initialisation.

The prev_blank = true initialisation creates incorrect behaviour for the first line, as noted in past review feedback.

Apply this correction:

-    let mut prev_blank = true;
+    let mut prev_blank = lines.first().map_or(true, |line| line.trim().is_empty());

88-94: Extract the complex conditional logic into a dedicated function.

This nested conditional violates the complexity guidelines and duplicates the suggestion from past review feedback.

Extract into a helper function above the main loop:

/// Handles paragraph restart logic by popping counters when appropriate conditions are met.
fn handle_paragraph_restart(
    line: &str,
    prev_blank: bool,
    counters: &mut Vec<(usize, usize)>,
    indent: usize,
) {
    if prev_blank
        && counters
            .last()
            .is_some_and(|(d, _)| indent <= *d && is_plain_paragraph_line(line))
    {
        pop_counters_upto(counters, indent);
    }
}

Then replace the nested logic:

-        if prev_blank
-            && counters
-                .last()
-                .is_some_and(|(d, _)| indent <= *d && is_plain_paragraph_line(line))
-        {
-            pop_counters_upto(&mut counters, indent);
-        }
+        handle_paragraph_restart(line, prev_blank, &mut counters, indent);
📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 11622a3 and 2e6292e.

📒 Files selected for processing (2)
  • src/lists.rs (3 hunks)
  • tests/lists.rs (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

Instructions used from:

Sources:
📄 CodeRabbit Inference Engine

  • AGENTS.md

⚙️ CodeRabbit Configuration File

🧬 Code Graph Analysis (1)
src/lists.rs (1)
src/wrap.rs (1)
  • is_fence (107-107)
🔇 Additional comments (4)
src/lists.rs (4)

54-60: Ensure consistent prev_blank tracking.

The prev_blank assignment logic appears correct for fence and code block handling, maintaining proper state tracking.


78-78: Correct prev_blank assignment for numbered lists.

The assignment correctly sets prev_blank = false after processing numbered list items, maintaining proper state tracking.


97-97: Verify prev_blank logic for general lines.

The assignment correctly determines if the current line is blank for the next iteration's logic.


29-34: Document the helper function purpose.

The function lacks documentation explaining its purpose and usage, violating the Rustdoc requirement for public APIs.

Add proper documentation:

+/// Determines if a line represents a plain paragraph (starts with alphanumeric after whitespace).
+/// 
+/// Returns `true` if the line's first non-whitespace character is alphanumeric,
+/// indicating it's likely the start of a paragraph rather than a special markdown element.
 fn is_plain_paragraph_line(line: &str) -> bool {

Likely an incorrect or invalid review comment.

Comment thread tests/lists.rs
Comment on lines +3 to +7
macro_rules! lines_vec {
($($line:expr),* $(,)?) => {
vec![$($line.to_string()),*]
};
}
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.

🧹 Nitpick (assertive)

Extract the macro into a test utility module.

The lines_vec! macro is useful but should be moved to a shared test utilities module to promote reusability across test files.

Apply this refactor:

-macro_rules! lines_vec {
-    ($($line:expr),* $(,)?) => {
-        vec![$($line.to_string()),*]
-    };
-}

Create tests/common/mod.rs:

#[macro_export]
macro_rules! lines_vec {
    ($($line:expr),* $(,)?) => {
        vec![$($line.to_string()),*]
    };
}

Then import it:

+mod common;
 use mdtablefix::{lists::pop_counters_upto, renumber_lists};
+use common::lines_vec;
🤖 Prompt for AI Agents
In tests/lists.rs around lines 3 to 7, the lines_vec! macro should be moved to a
shared test utility module for reuse. Create a new file tests/common/mod.rs and
define the lines_vec! macro there with #[macro_export]. Then, in tests/lists.rs
and other test files, import the macro by adding `#[macro_use] extern crate
common;` or the appropriate import statement to use lines_vec! from the common
module.

Comment thread tests/lists.rs
Comment on lines +9 to +21
#[test]
fn pop_counters_removes_deeper_levels() {
let mut counters = vec![(0usize, 1usize), (4, 2), (8, 3)];
pop_counters_upto(&mut counters, 4);
assert_eq!(counters, vec![(0, 1)]);
}

#[test]
fn pop_counters_no_change_when_indent_deeper() {
let mut counters = vec![(0usize, 1usize), (4, 2)];
pop_counters_upto(&mut counters, 6);
assert_eq!(counters, vec![(0, 1), (4, 2)]);
}
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.

🛠️ Refactor suggestion

Add edge case tests for pop_counters_upto.

The unit tests cover basic scenarios but miss important edge cases that could reveal bugs.

Add these additional test cases:

#[test]
fn pop_counters_empty_vec() {
    let mut counters = vec![];
    pop_counters_upto(&mut counters, 0);
    assert_eq!(counters, vec![]);
}

#[test]
fn pop_counters_exact_match() {
    let mut counters = vec![(0usize, 1usize), (4, 2)];
    pop_counters_upto(&mut counters, 4);
    assert_eq!(counters, vec![(0, 1)]);
}
🤖 Prompt for AI Agents
In tests/lists.rs around lines 9 to 21, add two new unit tests for
pop_counters_upto to cover edge cases: one test with an empty counters vector to
ensure no changes occur, and another test where the indent exactly matches an
element in counters to verify correct removal behavior. Implement these tests by
initializing the counters vector accordingly, calling pop_counters_upto with the
specified indent, and asserting the expected resulting vector.

Comment thread tests/lists.rs
Comment on lines +23 to +56
#[test]
fn restart_after_lower_paragraph() {
let input = lines_vec!("1. One", "", "Paragraph", "3. Next");
let expected = lines_vec!("1. One", "", "Paragraph", "1. Next");
assert_eq!(renumber_lists(&input), expected);
}

#[test]
fn no_restart_without_blank() {
let input = lines_vec!("1. One", "Paragraph", "3. Next");
let expected = lines_vec!("1. One", "Paragraph", "2. Next");
assert_eq!(renumber_lists(&input), expected);
}

#[test]
fn no_restart_for_indented_paragraph() {
let input = lines_vec!("1. One", "", " Indented", "3. Next");
let expected = lines_vec!("1. One", "", " Indented", "2. Next");
assert_eq!(renumber_lists(&input), expected);
}

#[test]
fn no_restart_for_non_plain_line() {
let input = lines_vec!("1. One", "", "# Heading", "3. Next");
let expected = lines_vec!("1. One", "", "# Heading", "2. Next");
assert_eq!(renumber_lists(&input), expected);
}

#[test]
fn restart_after_nested_paragraph() {
let input = lines_vec!("1. One", " 1. Sub", "", "Paragraph", "3. Next");
let expected = lines_vec!("1. One", " 1. Sub", "", "Paragraph", "1. Next");
assert_eq!(renumber_lists(&input), expected);
}
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.

🛠️ Refactor suggestion

Use rstest parameterised tests to reduce duplication.

The integration tests follow a repetitive pattern that violates DRY principles. Replace with parameterised tests as mandated by the coding guidelines.

Apply this refactor:

use rstest::rstest;

#[rstest]
#[case(
    lines_vec!("1. One", "", "Paragraph", "3. Next"),
    lines_vec!("1. One", "", "Paragraph", "1. Next"),
    "restart_after_lower_paragraph"
)]
#[case(
    lines_vec!("1. One", "Paragraph", "3. Next"),
    lines_vec!("1. One", "Paragraph", "2. Next"),
    "no_restart_without_blank"
)]
#[case(
    lines_vec!("1. One", "", "  Indented", "3. Next"),
    lines_vec!("1. One", "", "  Indented", "2. Next"),
    "no_restart_for_indented_paragraph"
)]
#[case(
    lines_vec!("1. One", "", "# Heading", "3. Next"),
    lines_vec!("1. One", "", "# Heading", "2. Next"),
    "no_restart_for_non_plain_line"
)]
#[case(
    lines_vec!("1. One", "    1. Sub", "", "Paragraph", "3. Next"),
    lines_vec!("1. One", "    1. Sub", "", "Paragraph", "1. Next"),
    "restart_after_nested_paragraph"
)]
fn test_renumber_lists_scenarios(
    #[case] input: Vec<String>,
    #[case] expected: Vec<String>,
    #[case] _description: &str,
) {
    assert_eq!(renumber_lists(&input), expected);
}
🤖 Prompt for AI Agents
In tests/lists.rs from lines 23 to 56, the test functions for renumber_lists are
repetitive and violate DRY principles. Refactor by replacing the multiple
individual #[test] functions with a single parameterized test using the rstest
crate. Define a #[rstest] function with multiple #[case] attributes, each
containing the input vector, expected output vector, and a description string.
This will consolidate all test cases into one function that asserts equality for
each case, reducing duplication and improving maintainability.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2e6292e and 19e5f93.

📒 Files selected for processing (1)
  • src/lists.rs (3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

Instructions used from:

Sources:
📄 CodeRabbit Inference Engine

  • AGENTS.md

⚙️ CodeRabbit Configuration File

🧬 Code Graph Analysis (1)
src/lists.rs (1)
src/wrap.rs (1)
  • is_fence (107-107)
🔇 Additional comments (2)
src/lists.rs (2)

64-64: Excellent implementation of paragraph restart tracking.

The prev_blank initialization correctly addresses the past review comment about first line handling. The implementation properly handles empty input.


104-107: Well-integrated paragraph restart functionality.

The integration of handle_paragraph_restart into the main loop is correctly positioned and maintains proper state tracking. The consistent prev_blank updates across all code paths ensure reliable behaviour.

Comment thread src/lists.rs
Comment on lines +43 to +57
fn handle_paragraph_restart(line: &str, prev_blank: bool, counters: &mut Vec<(usize, usize)>) {
let indent_end = line
.char_indices()
.find(|&(_, c)| !c.is_whitespace())
.map_or_else(|| line.len(), |(i, _)| i);
let indent = indent_len(&line[..indent_end]);

if prev_blank
&& counters
.last()
.is_some_and(|(d, _)| indent <= *d && is_plain_paragraph_line(line))
{
pop_counters_upto(counters, indent);
}
}
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.

🧹 Nitpick (assertive)

Add documentation for the extracted logic.

Excellent work extracting the complex paragraph restart logic as suggested in past reviews. Add documentation to explain the function's purpose.

+/// Handles resetting list counters when a paragraph line follows a blank line
+/// at or before the indentation level of the last counter.
 fn handle_paragraph_restart(line: &str, prev_blank: bool, counters: &mut Vec<(usize, usize)>) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fn handle_paragraph_restart(line: &str, prev_blank: bool, counters: &mut Vec<(usize, usize)>) {
let indent_end = line
.char_indices()
.find(|&(_, c)| !c.is_whitespace())
.map_or_else(|| line.len(), |(i, _)| i);
let indent = indent_len(&line[..indent_end]);
if prev_blank
&& counters
.last()
.is_some_and(|(d, _)| indent <= *d && is_plain_paragraph_line(line))
{
pop_counters_upto(counters, indent);
}
}
/// Handles resetting list counters when a paragraph line follows a blank line
/// at or before the indentation level of the last counter.
fn handle_paragraph_restart(line: &str, prev_blank: bool, counters: &mut Vec<(usize, usize)>) {
let indent_end = line
.char_indices()
.find(|&(_, c)| !c.is_whitespace())
.map_or_else(|| line.len(), |(i, _)| i);
let indent = indent_len(&line[..indent_end]);
if prev_blank
&& counters
.last()
.is_some_and(|(d, _)| indent <= *d && is_plain_paragraph_line(line))
{
pop_counters_upto(counters, indent);
}
}
🤖 Prompt for AI Agents
In src/lists.rs around lines 43 to 57, the function handle_paragraph_restart
lacks documentation explaining its purpose. Add a clear doc comment above the
function describing that it handles restarting paragraphs based on indentation
and blank line context, detailing the role of its parameters and the effect on
the counters vector.

Comment thread src/lists.rs
Comment on lines +29 to +34
fn is_plain_paragraph_line(line: &str) -> bool {
line.trim_start()
.chars()
.next()
.is_some_and(char::is_alphanumeric)
}
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.

🧹 Nitpick (assertive)

Add documentation for the helper function.

Add a doc comment explaining the function's purpose and behaviour.

+/// Determines whether a line represents a plain paragraph by checking if it starts
+/// with an alphanumeric character after any leading whitespace.
 fn is_plain_paragraph_line(line: &str) -> bool {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fn is_plain_paragraph_line(line: &str) -> bool {
line.trim_start()
.chars()
.next()
.is_some_and(char::is_alphanumeric)
}
/// Determines whether a line represents a plain paragraph by checking if it starts
/// with an alphanumeric character after any leading whitespace.
fn is_plain_paragraph_line(line: &str) -> bool {
line.trim_start()
.chars()
.next()
.is_some_and(char::is_alphanumeric)
}
🤖 Prompt for AI Agents
In src/lists.rs around lines 29 to 34, the helper function
is_plain_paragraph_line lacks documentation. Add a doc comment above the
function that clearly explains its purpose, which is to check if a line starts
with an alphanumeric character after trimming leading whitespace, indicating it
is a plain paragraph line.

Comment thread src/lists.rs Outdated
Comment on lines +36 to +41
#[doc(hidden)]
pub fn pop_counters_upto(counters: &mut Vec<(usize, usize)>, indent: usize) {
while counters.last().is_some_and(|(d, _)| *d >= indent) {
counters.pop();
}
}
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.

🛠️ Refactor suggestion

Fix visibility and documentation issues.

Address the following issues with this function:

  1. Remove #[doc(hidden)] and add proper documentation since this is a public function
  2. The function uses >= comparison while drop_deeper uses > - ensure this difference is intentional
  3. Consider making this function private if it's only used internally
-#[doc(hidden)]
-pub fn pop_counters_upto(counters: &mut Vec<(usize, usize)>, indent: usize) {
+/// Removes all counters from the stack whose indentation is greater than or equal
+/// to the specified indentation level.
+fn pop_counters_upto(counters: &mut Vec<(usize, usize)>, indent: usize) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#[doc(hidden)]
pub fn pop_counters_upto(counters: &mut Vec<(usize, usize)>, indent: usize) {
while counters.last().is_some_and(|(d, _)| *d >= indent) {
counters.pop();
}
}
/// Removes all counters from the stack whose indentation is greater than or equal
/// to the specified indentation level.
fn pop_counters_upto(counters: &mut Vec<(usize, usize)>, indent: usize) {
while counters.last().is_some_and(|(d, _)| *d >= indent) {
counters.pop();
}
}
🤖 Prompt for AI Agents
In src/lists.rs around lines 36 to 41, remove the #[doc(hidden)] attribute and
add clear, proper documentation explaining the purpose and behavior of the
pop_counters_upto function. Review the comparison operator in the while loop to
confirm if using >= is intentional compared to drop_deeper's >; if not, align
them accordingly. Finally, if this function is only used internally within the
module, change its visibility from public to private by removing the pub
keyword.

@leynos
Copy link
Copy Markdown
Owner Author

leynos commented Jul 17, 2025

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jul 17, 2025

✅ 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
Copy link
Copy Markdown
Owner Author

leynos commented Jul 17, 2025

@coderabbitai resume

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jul 17, 2025

✅ Actions performed

Reviews resumed.

@leynos leynos merged commit 171aae2 into main Jul 17, 2025
2 checks passed
@leynos leynos deleted the codex/add-test-for-list-renumbering-behavior branch July 17, 2025 01:25
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.

1 participant