crates/sre_engine/Cargo.toml (1)

17-19: Remove the redundant explicit [[test]] entry or document the reason for making it explicit.

In Rust 2024 edition with autotests = true (the default), Cargo automatically discovers tests/tests.rs as an integration test target. The explicit [[test]] name = "tests" without a path field resolves to the same file that would already be discovered, making this entry redundant. If the intent is to make it explicit for documentation or consistency with the [[bench]] declaration above, that's acceptable—but if it serves no purpose, removing it would clean up unnecessary boilerplate.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/sre_engine/Cargo.toml` around lines 17 - 19, Remove the redundant
explicit test table: delete the `[[test]]` block that only contains `name =
"tests"` (the `[[test]] name = "tests"` entry) from Cargo.toml, or if you intend
it for documentation/consistency, add a short comment above that block
explaining why it's explicit (e.g., clarifying it's intentional to mirror the
`[[bench]]` declaration); do not add a `path` field since Cargo already
autodiscovers tests in Rust 2024 with `autotests = true`.

extra_tests/snippets/stdlib_re.py (1)

80-80: Unescaped . in pattern matches any character, not only literal dot.

(?:.[0-9]+) will match 1X25Y38 just as readily as 1.25.38. The test still exercises the correct code path for the issue, but using \. better expresses intent.

✏️ Proposed fix

-assert re.fullmatch(r"([0-9]++(?:.[0-9]+)*+)", "1.25.38")
-assert re.fullmatch(r"([0-9]++(?:.[0-9]+)*+)", "1.25.38").group(0) == "1.25.38"
+assert re.fullmatch(r"([0-9]++(?:\.[0-9]+)*+)", "1.25.38")
+assert re.fullmatch(r"([0-9]++(?:\.[0-9]+)*+)", "1.25.38").group(0) == "1.25.38"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@extra_tests/snippets/stdlib_re.py` at line 80, The regex in the re.fullmatch
call uses an unescaped dot in the non-capturing group (pattern
"([0-9]++(?:.[0-9]+)*+)"), so it will match any character instead of a literal
period; update the pattern passed to re.fullmatch to escape the dot in the group
(i.e., change the "(?:.[0-9]+)" portion to "(?:\.[0-9]+)") so the test enforces
literal dots (reference the re.fullmatch invocation and the pattern string).

crates/sre_engine/src/engine.rs (1)

473-473: Comments read as change descriptions rather than code explanations.

// modified next.toplevel from inherited to false describes what was changed (commit-message style) rather than why the invariant exists. A future reader maintaining this code won't immediately understand the significance.

✏️ Suggested wording (apply to all three sites)

-            // modified next.toplevel from inherited to false
-            let mut next = ctx.next_offset(4, Jump::PossessiveRepeat2);
-            next.toplevel = false;
+            let mut next = ctx.next_offset(4, Jump::PossessiveRepeat2);
+            next.toplevel = false; // sub-pattern must not apply match_all/must_advance constraints

-            let mut next = ctx.next_offset(4, Jump::PossessiveRepeat4);
-            next.toplevel = false; // modified next.toplevel from inherited to false
+            let mut next = ctx.next_offset(4, Jump::PossessiveRepeat4);
+            next.toplevel = false; // sub-pattern must not apply match_all/must_advance constraints

-            let mut next_ctx = ctx.next_offset(2, Jump::AtomicGroup1);
-            next_ctx.toplevel = false; // modified next.toplevel from inherited to false
+            let mut next_ctx = ctx.next_offset(2, Jump::AtomicGroup1);
+            next_ctx.toplevel = false; // sub-pattern must not apply match_all/must_advance constraints

Also applies to: 501-501, 841-841

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/sre_engine/src/engine.rs` at line 473, Replace the commit-style
comment "// modified next.toplevel from inherited to false" with a concise
explanation of the invariant and rationale: state why next.toplevel must be set
to false (what property/state would break if it remained inherited), what
callers or code paths rely on this being false, and any assumptions about
downstream behavior or performance; apply the same explanatory replacement at
the other two occurrences that reference next.toplevel so future maintainers
understand the intent and not just the change.