[codex] Fix numeric sequence candidate selection

[Ahmed-Hindy]

Summary

• choose direct numeric frame tokens only from dot-delimited 3-5 digit groups
• preserve versioned filename support like shot_v001.0001.exr
• avoid treating fixed numeric suffixes like layer1, 2k, versions, or shot IDs as frame numbers

Root cause

Direct numeric detection used to grab the first digit group, while PR #113 tried always using the last digit group. Both are too broad for production filenames with shot IDs, versions, resolution tags, or layer tags.

This keeps the fallback intentionally narrow: direct numeric filenames are treated as frame sequences only when the candidate token looks like .<3-5 digits>., and the last such token is used as the frame number.

Verification

• uv --system-certs run --extra dev python -m pytest tests/test_sequence.py -q
• uv --system-certs run --extra dev ruff check src/renderkit/core/sequence.py tests/test_sequence.py
• uv --system-certs run --extra dev ruff format --check src/renderkit/core/sequence.py tests/test_sequence.py
• uv --system-certs run --extra dev python -m pytest -q

Fixes #94

[coderabbitai]

📝 Walkthrough

Walkthrough

Sequence detection's numeric fallback in detect_sequence() now scans all numeric groups in a filename via re.finditer and selects the best candidate using a new _find_best_numeric_sequence_candidate() helper, which scores by frame count then match position. Tests cover versioned filenames and trailing numeric suffixes.

Changes

Numeric sequence detection fix

Layer / File(s)	Summary
Multi-candidate numeric sequence detection src/renderkit/core/sequence.py, tests/test_sequence.py	detect_sequence() collects all numeric matches and delegates to new _find_best_numeric_sequence_candidate(), which picks the candidate with the most frames (tie-broken by earliest match position) instead of assuming the first numeric group is the frame token; new tests verify pattern, frame_numbers, and file path for versioned and multi-numeric-group filenames.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant SequenceDetector
    participant Helper as _find_best_numeric_sequence_candidate

    Caller->>SequenceDetector: detect_sequence(filename)
    SequenceDetector->>SequenceDetector: re.finditer(numeric pattern)
    SequenceDetector->>Helper: evaluate all numeric matches
    loop for each numeric match
        Helper->>Helper: _find_frames_by_numeric_pattern(match)
    end
    Helper-->>SequenceDetector: best candidate (match, frame_numbers)
    SequenceDetector-->>Caller: Sequence(pattern, frame_numbers)

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The code and tests address #94 by choosing among multiple numeric groups, preventing version digits from being used as the frame token.
Out of Scope Changes check	✅ Passed	The changes are limited to sequence detection logic and targeted regression tests, with no clear unrelated scope added.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and accurately summarizes the main change to numeric sequence candidate selection.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch dev/numeric-sequence-candidate-selection

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[coderabbitai]

🧹 Nitpick comments (1)

src/renderkit/core/sequence.py (1)

202-234: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Repeated full directory scans per numeric candidate.

_find_best_numeric_sequence_candidate calls _find_frames_by_numeric_pattern once per numeric group in the filename, and each call performs its own base_path.iterdir() scan (Line 260). For filenames with several numeric tokens (e.g. shot_010_v003.1001.exr) sitting in directories with many sibling frames, this results in multiple full directory listings where one would suffice.

Consider listing the directory once and reusing the sibling filenames across all candidate evaluations.

♻️ Suggested refactor

     `@staticmethod`
     def _find_best_numeric_sequence_candidate(
         base_path: Path, filename: str, numeric_matches: list[re.Match[str]]
     ) -> Optional[tuple[re.Match[str], list[int]]]:
         ...
+        sibling_names = (
+            [p.name for p in base_path.iterdir() if p.is_file()] if base_path.exists() else []
+        )
         candidates: list[tuple[re.Match[str], list[int]]] = []
         for numeric_match in numeric_matches:
             frame_numbers = SequenceDetector._find_frames_by_numeric_pattern(
-                base_path, filename, numeric_match
+                sibling_names, filename, numeric_match
             )
             if frame_numbers:
                 candidates.append((numeric_match, frame_numbers))

     `@staticmethod`
     def _find_frames_by_numeric_pattern(
-        base_path: Path, filename: str, numeric_match: re.Match[str]
+        sibling_names: list[str], filename: str, numeric_match: re.Match[str]
     ) -> list[int]:
         ...
         frame_numbers: list[int] = []
         start_pos, end_pos = numeric_match.span()
         prefix = filename[:start_pos]
         suffix = filename[end_pos:]

-        # Try to find files with same prefix/suffix but different numbers
-        if base_path.exists():
-            padding = end_pos - start_pos
-            pattern_regex = re.compile(
-                rf"^{re.escape(prefix)}(\d{{{padding}}}){re.escape(suffix)}$"
-            )
-            for file_path in base_path.iterdir():
-                if file_path.is_file():
-                    match = pattern_regex.match(file_path.name)
-                    if match:
-                        try:
-                            frame_num = int(match.group(1))
-                            frame_numbers.append(frame_num)
-                        except ValueError:
-                            continue
+        padding = end_pos - start_pos
+        pattern_regex = re.compile(rf"^{re.escape(prefix)}(\d{{{padding}}}){re.escape(suffix)}$")
+        for name in sibling_names:
+            match = pattern_regex.match(name)
+            if match:
+                try:
+                    frame_numbers.append(int(match.group(1)))
+                except ValueError:
+                    continue

         return sorted(frame_numbers)

Also applies to: 236-270

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/renderkit/core/sequence.py` around lines 202 - 234, _{}_Repeated
directory scans are happening in
SequenceDetector._find_best_numeric_sequence_candidate because each numeric
candidate re-invokes _find_frames_by_numeric_pattern, which walks base_path
repeatedly. Refactor the candidate evaluation to list the directory once, cache
the sibling filenames, and pass that shared data into the per-candidate frame
detection logic so each numeric_match can be scored without additional
base_path.iterdir() calls.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/renderkit/core/sequence.py`:
- Around line 202-234: _{}_Repeated directory scans are happening in
SequenceDetector._find_best_numeric_sequence_candidate because each numeric
candidate re-invokes _find_frames_by_numeric_pattern, which walks base_path
repeatedly. Refactor the candidate evaluation to list the directory once, cache
the sibling filenames, and pass that shared data into the per-candidate frame
detection logic so each numeric_match can be scored without additional
base_path.iterdir() calls.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8fa35a81-4f70-4070-9fdd-0c9489a37fef

📥 Commits

Reviewing files that changed from the base of the PR and between 945f9e3 and f69d3e3.

📒 Files selected for processing (2)

• src/renderkit/core/sequence.py
• tests/test_sequence.py

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud