pick the earliest-occurring level keyword in detect_log_level#23
pick the earliest-occurring level keyword in detect_log_level#23HrachShah wants to merge 1 commit into
Conversation
detect_log_level walked a hardcoded list of (regex, level) tuples in CRITICAL > ERROR > WARNING > INFO > DEBUG > TRACE order and returned the first patternprecedence — whichever keyword appears first in the line is the intent of the line. Without this, '2024-01-15 10:23:45 WARNING cannot connect to CRITICAL service' was classified as CRITICAL even though the line is a WARNING. The fix runs every level regex, finds the leftmost match across all of them, and returns the level that owns that match. A new tests/test_utils.py pins the new contract: the leftmost keyword wins, embedded levels in later words do not override, and words like 'ERROR_RATE' or 'CRITICAL_ERROR' (which contain the level as a prefix of a longer identifier) are correctly ignored because the pattern uses \b boundaries.
Reviewer's GuideUpdate log level detection to choose the earliest-occurring level keyword in a line instead of a fixed severity order, and add unit tests covering the new behavior and existing edge cases. Flow diagram for updated detect_log_level logicflowchart TD
A[detect_log_level line] --> B[Convert line to uppercase line_upper]
B --> C[Initialize earliest_level = None]
C --> D[Initialize earliest_index = infinity]
D --> E[Iterate level_patterns]
E --> F[re.finditer pattern line_upper]
F --> G{Any matches?}
G -->|No| H{More patterns?}
H -->|Yes| E
H -->|No| I{earliest_level is not None?}
G -->|Yes| J[Take first_match.start]
J --> K{first_match.start < earliest_index?}
K -->|Yes| L[Update earliest_index and earliest_level]
K -->|No| H
L --> H
I -->|Yes| M[Return earliest_level]
I -->|No| N[Return UNKNOWN]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- You don't need to materialize all matches with
list(re.finditer(...)); usingnext(re.finditer(...), None)and checking that single result would avoid unnecessary allocations while still letting you compare positions. - Consider avoiding
float('inf')forearliest_indexand instead initializing it toNoneand adjusting the comparison logic, which can make the intent clearer and removes reliance on magic sentinel values.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- You don't need to materialize all matches with `list(re.finditer(...))`; using `next(re.finditer(...), None)` and checking that single result would avoid unnecessary allocations while still letting you compare positions.
- Consider avoiding `float('inf')` for `earliest_index` and instead initializing it to `None` and adjusting the comparison logic, which can make the intent clearer and removes reliance on magic sentinel values.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Warning Review limit reached
More reviews will be available in 37 minutes and 3 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
What
utils.detect_log_levelreturns the first regex match in a hardcodedCRITICAL > ERROR > WARNING > INFO > DEBUG > TRACE order, regardless of
where the keyword actually appears in the line.
Repro
Summary by Sourcery
Update log level detection to choose the earliest-occurring level keyword in a log line and add unit tests to cover the new behavior and edge cases.
Bug Fixes:
Tests: