fix: parse reset time when percentage shares same line#174
Merged
hanrw merged 1 commit intotddworks:mainfrom Apr 15, 2026
Merged
fix: parse reset time when percentage shares same line#174hanrw merged 1 commit intotddworks:mainfrom
hanrw merged 1 commit intotddworks:mainfrom
Conversation
Newer Claude CLI versions (v2.1.109+) render the reset text and percentage on the same line instead of separate lines: Resets 3pm (Europe/Amsterdam) 27% used The trailing 'NN% used/left' suffix prevented timezone stripping and date parsing, causing resetsAt to be nil and the pace triangle marker to disappear. Strip the suffix before date parsing begins.
Contributor
📝 WalkthroughWalkthroughAdded regex normalization to the date parsing function in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
Tests/InfrastructureTests/Claude/ClaudeUsageProbeParsingTests.swift (1)
324-336: Strengthen this test to verify quota-specific reset mapping, not just non-nil.Given the fixture uses different weekly and sonnet reset dates, asserting
resetTextcontent would prevent false positives where both quotas accidentally share one parsed reset.Suggested assertion additions
`@Test` func `parses resetsAt when reset and percent share same line`() throws { // When let snapshot = try ClaudeUsageProbe.parse(Self.resetOnSameLineOutput) // Then — all quotas should have resetsAt populated (enables pace triangle) `#expect`(snapshot.sessionQuota?.resetsAt != nil, "Session resetsAt should be populated for 'Resets 3pm (TZ) ... 27% used' format") `#expect`(snapshot.weeklyQuota?.resetsAt != nil, "Weekly resetsAt should be populated for 'Resets Apr 16 at 4:59pm (TZ) ... 40% used' format") `#expect`(snapshot.quota(for: .modelSpecific("sonnet"))?.resetsAt != nil, "Sonnet resetsAt should be populated for 'Resets Apr 17 at 11:59am (TZ) ... 0% used' format") + `#expect`(snapshot.weeklyQuota?.resetText?.contains("Apr 16") == true) + `#expect`(snapshot.quota(for: .modelSpecific("sonnet"))?.resetText?.contains("Apr 17") == true) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Tests/InfrastructureTests/Claude/ClaudeUsageProbeParsingTests.swift` around lines 324 - 336, Update the test `parses resetsAt when reset and percent share same line` to assert the specific reset mapping instead of only non-nil: after calling ClaudeUsageProbe.parse(Self.resetOnSameLineOutput) compare snapshot.sessionQuota?.resetsAt, snapshot.weeklyQuota?.resetsAt, and snapshot.quota(for: .modelSpecific("sonnet"))?.resetsAt against the expected values derived from the fixture (or assert their resetText strings match the distinct expected reset strings) so the test verifies weekly vs sonnet reset dates are parsed differently and not accidentally shared.Sources/Infrastructure/Claude/ClaudeUsageProbe.swift (1)
708-712: Make percent-suffix stripping resilient to no-space formatting.Current regex requires whitespace before
NN% used|left. If CLI output ever collapses spacing (e.g.,)...27% used), stripping won’t happen and date parsing can regress.Suggested patch
- cleaned = cleaned - .replacingOccurrences(of: #"\s+\d{1,3}%\s*(?:used|left)\s*$"#, with: "", options: .regularExpression) + cleaned = cleaned + .replacingOccurrences(of: #"\s*\d{1,3}%\s*(?:used|left)\s*$"#, with: "", options: .regularExpression)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Infrastructure/Claude/ClaudeUsageProbe.swift` around lines 708 - 712, The regex that strips trailing "NN% used/left" in ClaudeUsageProbe.swift requires a whitespace before the percent block, so cases like "...27% used" (no extra space) won't match; update the replacingOccurrences call that operates on the variable `cleaned` to use a regex that allows zero or more spaces before the percent (e.g., make the whitespace before `\d{1,3}%` optional) so both " 27% used" and "27% used" are removed reliably, leaving the rest of the parsing unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Sources/Infrastructure/Claude/ClaudeUsageProbe.swift`:
- Around line 708-712: The regex that strips trailing "NN% used/left" in
ClaudeUsageProbe.swift requires a whitespace before the percent block, so cases
like "...27% used" (no extra space) won't match; update the replacingOccurrences
call that operates on the variable `cleaned` to use a regex that allows zero or
more spaces before the percent (e.g., make the whitespace before `\d{1,3}%`
optional) so both " 27% used" and "27% used" are removed reliably, leaving the
rest of the parsing unchanged.
In `@Tests/InfrastructureTests/Claude/ClaudeUsageProbeParsingTests.swift`:
- Around line 324-336: Update the test `parses resetsAt when reset and percent
share same line` to assert the specific reset mapping instead of only non-nil:
after calling ClaudeUsageProbe.parse(Self.resetOnSameLineOutput) compare
snapshot.sessionQuota?.resetsAt, snapshot.weeklyQuota?.resetsAt, and
snapshot.quota(for: .modelSpecific("sonnet"))?.resetsAt against the expected
values derived from the fixture (or assert their resetText strings match the
distinct expected reset strings) so the test verifies weekly vs sonnet reset
dates are parsed differently and not accidentally shared.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f5eeb6f9-f330-4881-97f7-4613755bdc26
📒 Files selected for processing (2)
Sources/Infrastructure/Claude/ClaudeUsageProbe.swiftTests/InfrastructureTests/Claude/ClaudeUsageProbeParsingTests.swift
Contributor
Author
CI failure is pre-existing (not related to this PR)Both checks fail with the same This reproduces on Possible fixes:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
NN% used/leftsuffix prevented timezone stripping and date parsing inparseAbsoluteDate, causingresetsAtto benil— which made the pace triangle marker disappear from the progress bar.% used/% leftsuffix before date parsing begins.Changes
ClaudeUsageProbe.swift: Strip\d{1,3}% (used|left)suffix at the start ofparseAbsoluteDateClaudeUsageProbeParsingTests.swift: Two new tests with real CLI output from v2.1.109Test Plan
parses percentages when reset and percent share same line— verifies 27%, 40%, 0% are parsed correctlyparses resetsAt when reset and percent share same line— verifiesresetsAtis populated (enables pace triangle)Summary by CodeRabbit