Conversation
Reviewer's GuideRefactors create_windows by removing debug prints and simplifying the start position update to correctly handle zero overlap without premature loop termination. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey @strickvl - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Reintroduce clamp or break to prevent invalid or duplicate windows (link)
Here's what I looked at during the review
- 🔴 General issues: 1 blocking issue, 1 other issue
- 🟢 Testing: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if start <= 0 or start >= end: | ||
| break | ||
| # Move start position for next window | ||
| start = end - overlap_size |
There was a problem hiding this comment.
suggestion (bug_risk): Reintroduce clamp or break to prevent invalid or duplicate windows
Clamp start (e.g. start = max(0, end - overlap_size)) and reintroduce a break if start <= prev_start or start <= 0 to avoid invalid windows or infinite loops.
| if not text: | ||
| return [] |
There was a problem hiding this comment.
suggestion (bug_risk): Validate overlap_size against window_size
When overlap_size >= window_size, the loop won’t advance or produce windows. Consider raising an error or normalizing overlap_size up front, e.g.: overlap_size = min(overlap_size, window_size - 1).
| if not text: | |
| return [] | |
| if overlap_size >= window_size: | |
| overlap_size = min(overlap_size, window_size - 1) | |
| if not text: | |
| return [] |
Summary
create_windowsso overlap size of zero doesn't exit loop earlyTesting
python -m py_compile $(git ls-files '*.py')pytest -q(fails: command not found)Summary by Sourcery
Remove debug logging and streamline the sliding-window logic in create_windows to prevent premature termination when overlap_size is zero
Bug Fixes:
Enhancements: