fix: off-by-one compact date formatting #434
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe internal logic of the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant formatDateCompact
participant SchemaValidator
participant DateParser
Caller->>formatDateCompact: Call with date string and optional timezone
formatDateCompact->>SchemaValidator: Validate if "YYYY-MM-DD" format
alt Matches "YYYY-MM-DD"
formatDateCompact->>DateParser: Append "T00:00:00" or "T00:00:00Z" based on timezone, then parse
else Not matched
formatDateCompact->>DateParser: Parse date string as is
end
formatDateCompact-->>Caller: Return formatted compact date string
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @travisjhicks, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
I've addressed an off-by-one error in the formatDateCompact function that occurred when processing YYYY-MM-DD formatted dates. The issue stemmed from JavaScript's Date constructor interpreting these strings as UTC, which caused incorrect date calculations for users in time zones with a negative offset from UTC. My change ensures these dates are consistently parsed as local time, resolving the discrepancy.
Highlights
- Date Parsing Fix: Modified formatDateCompact in src/data-loader.ts to correctly parse YYYY-MM-DD date strings. Previously, new Date() would interpret these as UTC, leading to an off-by-one day error for users in negative UTC timezones. The fix involves appending T00:00:00 to such date strings before creating a Date object, ensuring they are parsed as local time.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request correctly fixes an off-by-one error in date formatting by ensuring that YYYY-MM-DD date strings are parsed in the local timezone. The change is well-implemented and includes helpful comments. I have one suggestion to improve maintainability by reusing an existing schema for date validation, which will make the code more robust and easier to maintain.
src/data-loader.ts
Outdated
| const localDateStr = /^\d{4}-\d{2}-\d{2}$/.test(dateStr) | ||
| ? `${dateStr}T00:00:00` | ||
| : dateStr; |
There was a problem hiding this comment.
To improve maintainability and avoid duplicating the date-matching logic, you can reuse dailyDateSchema which is already imported in this file. This ensures that if the date format definition changes in the future, it only needs to be updated in one place.
const localDateStr = dailyDateSchema.safeParse(dateStr).success
? `${dateStr}T00:00:00`
: dateStr;![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/data-loader.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: Lint code using ESLint MCP server (available via Claude Code tools)
Format code with ESLint (writes changes) usingbun run format
No console.log allowed except where explicitly disabled with eslint-disable
Do not use console.log. Use logger.ts instead.
Files:
src/data-loader.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Type check with TypeScript using
bun typecheck
Files:
src/data-loader.ts
**/*.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.ts: File paths always use Node.js path utilities for cross-platform compatibility
Use.tsextensions for local file imports (e.g.,import { foo } from './utils.ts')
Prefer @praha/byethrow Result type over traditional try-catch for functional error handling
UseResult.try()for wrapping operations that may throw (JSON parsing, etc.)
UseResult.isFailure()for checking errors (more readable than!Result.isSuccess())
Use early return pattern (if (Result.isFailure(result)) continue;) instead of ternary operators
For async operations: create wrapper function withResult.try()then call it
Keep traditional try-catch only for: file I/O with complex error handling, legacy code that's hard to refactor
Always useResult.isFailure()andResult.isSuccess()type guards for better code clarity
Variables: start with lowercase (camelCase) - e.g.,usageDataSchema,modelBreakdownSchema
Types: start with uppercase (PascalCase) - e.g.,UsageData,ModelBreakdown
Constants: can use UPPER_SNAKE_CASE - e.g.,DEFAULT_CLAUDE_CODE_PATH
Only export constants, functions, and types that are actually used by other modules
Internal/private constants that are only used within the same file should NOT be exported
Always check if a constant is used elsewhere before making itexport constvs justconst
All test files must use current Claude 4 models, not outdated Claude 3 models
Test coverage should include both Sonnet and Opus models for comprehensive validation
Model names in tests must exactly match LiteLLM's pricing database entries
When adding new model tests, verify the model exists in LiteLLM before implementation
Tests depend on real pricing data from LiteLLM - failures may indicate model availability issues
Dynamic imports usingawait import()should only be used within test blocks to avoid tree-shaking issues
Mock data is created usingfs-fixturewithcreateFixture()for Claude data directory simulation
In-source testing pattern: Tests are written...
Files:
src/data-loader.ts
🧠 Learnings (1)
📚 Learning: applies to **/*.ts : mock data is created using `fs-fixture` with `createfixture()` for claude data ...
Learnt from: CR
PR: ryoppippi/ccusage#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-19T10:58:04.397Z
Learning: Applies to **/*.ts : Mock data is created using `fs-fixture` with `createFixture()` for Claude data directory simulation
Applied to files:
src/data-loader.ts
- Use dailyDateSchema parsing instead of regex - If time zone is specified, interpret date as UTC - Add unit tests
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/data-loader.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: Lint code using ESLint MCP server (available via Claude Code tools)
Format code with ESLint (writes changes) usingbun run format
No console.log allowed except where explicitly disabled with eslint-disable
Do not use console.log. Use logger.ts instead.
Files:
src/data-loader.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Type check with TypeScript using
bun typecheck
Files:
src/data-loader.ts
**/*.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.ts: File paths always use Node.js path utilities for cross-platform compatibility
Use.tsextensions for local file imports (e.g.,import { foo } from './utils.ts')
Prefer @praha/byethrow Result type over traditional try-catch for functional error handling
UseResult.try()for wrapping operations that may throw (JSON parsing, etc.)
UseResult.isFailure()for checking errors (more readable than!Result.isSuccess())
Use early return pattern (if (Result.isFailure(result)) continue;) instead of ternary operators
For async operations: create wrapper function withResult.try()then call it
Keep traditional try-catch only for: file I/O with complex error handling, legacy code that's hard to refactor
Always useResult.isFailure()andResult.isSuccess()type guards for better code clarity
Variables: start with lowercase (camelCase) - e.g.,usageDataSchema,modelBreakdownSchema
Types: start with uppercase (PascalCase) - e.g.,UsageData,ModelBreakdown
Constants: can use UPPER_SNAKE_CASE - e.g.,DEFAULT_CLAUDE_CODE_PATH
Only export constants, functions, and types that are actually used by other modules
Internal/private constants that are only used within the same file should NOT be exported
Always check if a constant is used elsewhere before making itexport constvs justconst
All test files must use current Claude 4 models, not outdated Claude 3 models
Test coverage should include both Sonnet and Opus models for comprehensive validation
Model names in tests must exactly match LiteLLM's pricing database entries
When adding new model tests, verify the model exists in LiteLLM before implementation
Tests depend on real pricing data from LiteLLM - failures may indicate model availability issues
Dynamic imports usingawait import()should only be used within test blocks to avoid tree-shaking issues
Mock data is created usingfs-fixturewithcreateFixture()for Claude data directory simulation
In-source testing pattern: Tests are written...
Files:
src/data-loader.ts
🧠 Learnings (3)
📚 Learning: applies to **/*.ts : mock data is created using `fs-fixture` with `createfixture()` for claude data ...
Learnt from: CR
PR: ryoppippi/ccusage#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-19T10:58:04.397Z
Learning: Applies to **/*.ts : Mock data is created using `fs-fixture` with `createFixture()` for Claude data directory simulation
Applied to files:
src/data-loader.ts
📚 Learning: applies to **/*.ts : variables: start with lowercase (camelcase) - e.g., `usagedataschema`, `modelbr...
Learnt from: CR
PR: ryoppippi/ccusage#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-19T10:58:04.397Z
Learning: Applies to **/*.ts : Variables: start with lowercase (camelCase) - e.g., `usageDataSchema`, `modelBreakdownSchema`
Applied to files:
src/data-loader.ts
📚 Learning: applies to **/*.ts : tests depend on real pricing data from litellm - failures may indicate model av...
Learnt from: CR
PR: ryoppippi/ccusage#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-19T10:58:04.397Z
Learning: Applies to **/*.ts : Tests depend on real pricing data from LiteLLM - failures may indicate model availability issues
Applied to files:
src/data-loader.ts
🪛 ESLint
src/data-loader.ts
[error] 572-572: Unexpected any value in conditional. An explicit comparison or type conversion is required.
(ts/strict-boolean-expressions)
[error] 572-572: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 572-572: Unsafe member access .safeParse on an error typed value.
(ts/no-unsafe-member-access)
[error] 572-572: Unsafe member access .success on an error typed value.
(ts/no-unsafe-member-access)
[error] 573-573: Unexpected nullable string value in conditional. Please handle the nullish/empty cases explicitly.
(ts/strict-boolean-expressions)
[error] 1443-1443: Trailing spaces not allowed.
(style/no-trailing-spaces)
[error] 1445-1445: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1445-1445: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1445-1445: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
[error] 1446-1446: Trailing spaces not allowed.
(style/no-trailing-spaces)
[error] 1448-1448: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1448-1448: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1448-1448: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
| // For YYYY-MM-DD format, append T00:00:00 to parse as local date | ||
| // Without this, new Date('YYYY-MM-DD') interprets as UTC midnight | ||
| const date = dailyDateSchema.safeParse(dateStr).success | ||
| ? timezone | ||
| ? new Date(`${dateStr}T00:00:00Z`) | ||
| : new Date(`${dateStr}T00:00:00`) | ||
| : new Date(dateStr); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix implemented correctly but consider timezone edge cases
The implementation successfully addresses the off-by-one error by using schema validation and appropriate time suffixes. However, there are a few concerns:
- Type safety issues: The ESLint errors indicate unsafe operations on the
safeParseresult - Timezone edge case: Per previous review feedback, this approach may still cause day shifts when the host timezone differs from the requested timezone for formatting
Consider the more robust approach suggested in previous reviews:
- const date = dailyDateSchema.safeParse(dateStr).success
- ? timezone
- ? new Date(`${dateStr}T00:00:00Z`)
- : new Date(`${dateStr}T00:00:00`)
- : new Date(dateStr);
+ const isPlainDate = dailyDateSchema.safeParse(dateStr).success;
+ const date = isPlainDate
+ ? new Date(`${dateStr}T00:00:00Z`) // Always use UTC for plain dates
+ : new Date(dateStr);This ensures consistent behavior regardless of the timezone parameter and avoids the type safety issues.
I’d like to inspect the actual implementation and accompanying tests to verify the type-safety and timezone logic. Let’s pull the relevant sections:
🏁 Script executed:
#!/bin/bash
# Show implementation around formatDateCompact
echo "=== src/data-loader.ts (lines 550–600) ==="
sed -n '550,600p' src/data-loader.ts
# Show the new test cases (lines 1440–1490)
echo
echo "=== src/data-loader.ts (lines 1440–1490) ==="
sed -n '1440,1490p' src/data-loader.tsLength of output: 4662
Approve formatDateCompact logic; address trailing whitespace
The updated implementation correctly distinguishes plain “YYYY-MM-DD” strings from full timestamps and respects the timezone parameter without unsafe type operations on safeParse. Tests confirm the intended behavior across UTC, local dates, and cross-day timezones.
• Remove any trailing spaces in the new test cases (e.g. the Asia/Tokyo line) to satisfy ESLint.
• Verify that all remaining ESLint type-safety complaints are resolved (there are no real unsafe operations on safeParse).
With those minor cleanups, this change is ready to merge.
🧰 Tools
🪛 ESLint
[error] 572-572: Unexpected any value in conditional. An explicit comparison or type conversion is required.
(ts/strict-boolean-expressions)
[error] 572-572: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 572-572: Unsafe member access .safeParse on an error typed value.
(ts/no-unsafe-member-access)
[error] 572-572: Unsafe member access .success on an error typed value.
(ts/no-unsafe-member-access)
[error] 573-573: Unexpected nullable string value in conditional. Please handle the nullish/empty cases explicitly.
(ts/strict-boolean-expressions)
🤖 Prompt for AI Agents
In src/data-loader.ts around lines 570 to 576, remove any trailing whitespace
characters from the new test cases, especially the Asia/Tokyo line, to comply
with ESLint rules. Also, review the usage of safeParse to ensure no ESLint
type-safety warnings remain by avoiding unsafe type assertions or operations.
These minor cleanups will finalize the changes for merging.
src/data-loader.ts
Outdated
|
|
||
| // Daily date defined as UTC is preserved | ||
| expect(formatDateCompact('2024-01-01', 'UTC', 'en-US')).toBe('2024\n01-01'); | ||
|
|
||
| // Daily date already in local time is preserved instead of being interpreted as UTC | ||
| expect(formatDateCompact('2024-01-01', undefined, 'en-US')).toBe('2024\n01-01'); |
There was a problem hiding this comment.
Fix trailing spaces and improve test assertions
The tests correctly verify the timezone behavior, but there are formatting issues that need to be addressed:
-
+
// Daily date defined as UTC is preserved
expect(formatDateCompact('2024-01-01', 'UTC', 'en-US')).toBe('2024\n01-01');
-
+
// Daily date already in local time is preserved instead of being interpreted as UTC
expect(formatDateCompact('2024-01-01', undefined, 'en-US')).toBe('2024\n01-01');The tests validate the core functionality well, covering both UTC and local timezone scenarios.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Daily date defined as UTC is preserved | |
| expect(formatDateCompact('2024-01-01', 'UTC', 'en-US')).toBe('2024\n01-01'); | |
| // Daily date already in local time is preserved instead of being interpreted as UTC | |
| expect(formatDateCompact('2024-01-01', undefined, 'en-US')).toBe('2024\n01-01'); | |
| // Daily date defined as UTC is preserved | |
| expect(formatDateCompact('2024-01-01', 'UTC', 'en-US')).toBe('2024\n01-01'); | |
| // Daily date already in local time is preserved instead of being interpreted as UTC | |
| expect(formatDateCompact('2024-01-01', undefined, 'en-US')).toBe('2024\n01-01'); |
🧰 Tools
🪛 ESLint
[error] 1443-1443: Trailing spaces not allowed.
(style/no-trailing-spaces)
[error] 1445-1445: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1445-1445: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1445-1445: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
[error] 1446-1446: Trailing spaces not allowed.
(style/no-trailing-spaces)
[error] 1448-1448: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1448-1448: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1448-1448: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
🤖 Prompt for AI Agents
In src/data-loader.ts around lines 1443 to 1448, fix the trailing spaces in the
test code to improve formatting and readability. Additionally, enhance the test
assertions by making them more explicit or descriptive if possible, ensuring
they clearly validate the expected behavior for both UTC and local timezone
inputs.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/data-loader.ts (1)
1443-1448: Fix formatting issues in test casesThe test logic correctly validates the timezone behavior, but there are formatting issues that need to be addressed:
Based on past review comments and ESLint errors, please clean up the formatting:
- + // Daily date defined as UTC is preserved expect(formatDateCompact('2024-01-01', 'UTC', 'en-US')).toBe('2024\n01-01'); - + // Daily date already in local time is preserved instead of being interpreted as UTC expect(formatDateCompact('2024-01-01', undefined, 'en-US')).toBe('2024\n01-01');The ESLint type safety errors on
.toBe()calls appear to be false positives similar to the schema validation issues above.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/data-loader.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: Lint code using ESLint MCP server (available via Claude Code tools)
Format code with ESLint (writes changes) usingbun run format
No console.log allowed except where explicitly disabled with eslint-disable
Do not use console.log. Use logger.ts instead.
Files:
src/data-loader.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Type check with TypeScript using
bun typecheck
Files:
src/data-loader.ts
**/*.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.ts: File paths always use Node.js path utilities for cross-platform compatibility
Use.tsextensions for local file imports (e.g.,import { foo } from './utils.ts')
Prefer @praha/byethrow Result type over traditional try-catch for functional error handling
UseResult.try()for wrapping operations that may throw (JSON parsing, etc.)
UseResult.isFailure()for checking errors (more readable than!Result.isSuccess())
Use early return pattern (if (Result.isFailure(result)) continue;) instead of ternary operators
For async operations: create wrapper function withResult.try()then call it
Keep traditional try-catch only for: file I/O with complex error handling, legacy code that's hard to refactor
Always useResult.isFailure()andResult.isSuccess()type guards for better code clarity
Variables: start with lowercase (camelCase) - e.g.,usageDataSchema,modelBreakdownSchema
Types: start with uppercase (PascalCase) - e.g.,UsageData,ModelBreakdown
Constants: can use UPPER_SNAKE_CASE - e.g.,DEFAULT_CLAUDE_CODE_PATH
Only export constants, functions, and types that are actually used by other modules
Internal/private constants that are only used within the same file should NOT be exported
Always check if a constant is used elsewhere before making itexport constvs justconst
All test files must use current Claude 4 models, not outdated Claude 3 models
Test coverage should include both Sonnet and Opus models for comprehensive validation
Model names in tests must exactly match LiteLLM's pricing database entries
When adding new model tests, verify the model exists in LiteLLM before implementation
Tests depend on real pricing data from LiteLLM - failures may indicate model availability issues
Dynamic imports usingawait import()should only be used within test blocks to avoid tree-shaking issues
Mock data is created usingfs-fixturewithcreateFixture()for Claude data directory simulation
In-source testing pattern: Tests are written...
Files:
src/data-loader.ts
🧠 Learnings (4)
📚 Learning: applies to **/*.ts : variables: start with lowercase (camelcase) - e.g., `usagedataschema`, `modelbr...
Learnt from: CR
PR: ryoppippi/ccusage#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-19T10:58:04.397Z
Learning: Applies to **/*.ts : Variables: start with lowercase (camelCase) - e.g., `usageDataSchema`, `modelBreakdownSchema`
Applied to files:
src/data-loader.ts
📚 Learning: applies to **/*.ts : mock data is created using `fs-fixture` with `createfixture()` for claude data ...
Learnt from: CR
PR: ryoppippi/ccusage#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-19T10:58:04.397Z
Learning: Applies to **/*.ts : Mock data is created using `fs-fixture` with `createFixture()` for Claude data directory simulation
Applied to files:
src/data-loader.ts
📚 Learning: when aspulse points out that variables are clearly type-inferred as strings from their construction ...
Learnt from: AsPulse
PR: ryoppippi/ccusage#328
File: src/_live-rendering.ts:133-167
Timestamp: 2025-07-20T23:46:21.740Z
Learning: When AsPulse points out that variables are clearly type-inferred as strings from their construction (using template literals, string methods like .toFixed(), .padStart(), etc.), avoid suggesting unnecessary type assertions that would reduce code readability. ESLint type safety errors about "error type" often indicate type definition issues with imported packages rather than actual type problems in the user's code.
Applied to files:
src/data-loader.ts
📚 Learning: applies to **/*.ts : dynamic imports using `await import()` should only be used within test blocks t...
Learnt from: CR
PR: ryoppippi/ccusage#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-19T10:58:04.397Z
Learning: Applies to **/*.ts : Dynamic imports using `await import()` should only be used within test blocks to avoid tree-shaking issues
Applied to files:
src/data-loader.ts
🪛 ESLint
src/data-loader.ts
[error] 572-572: Unexpected any value in conditional. An explicit comparison or type conversion is required.
(ts/strict-boolean-expressions)
[error] 572-572: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 572-572: Unsafe member access .safeParse on an error typed value.
(ts/no-unsafe-member-access)
[error] 572-572: Unsafe member access .success on an error typed value.
(ts/no-unsafe-member-access)
[error] 1445-1445: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1445-1445: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1445-1445: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
[error] 1448-1448: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1448-1448: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1448-1448: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/data-loader.ts (2)
570-577: Approve the off-by-one date formatting fix with ESLint cleanup neededThe implementation correctly addresses the off-by-one error by distinguishing plain "YYYY-MM-DD" strings from full timestamps and handling timezone appropriately. The logic properly ensures dates are interpreted as intended (UTC or local time) rather than defaulting to UTC midnight.
However, ESLint is flagging unsafe operations on the
safeParseresult. Extract the parse result to a variable to enable TypeScript's control-flow narrowing:- const date = dailyDateSchema.safeParse(dateStr).success - ? timezone != null - ? new Date(`${dateStr}T00:00:00Z`) - : new Date(`${dateStr}T00:00:00`) - : new Date(dateStr); + const parseResult = dailyDateSchema.safeParse(dateStr); + const date = parseResult.success + ? timezone != null + ? new Date(`${dateStr}T00:00:00Z`) + : new Date(`${dateStr}T00:00:00`) + : new Date(dateStr);
1444-1449: Fix trailing spaces in test casesThe tests correctly verify the timezone behavior and address the core issue, but there are formatting problems that need to be addressed:
- + // Daily date defined as UTC is preserved expect(formatDateCompact('2024-01-01', 'UTC', 'en-US')).toBe('2024\n01-01'); - + // Daily date already in local time is preserved instead of being interpreted as UTC expect(formatDateCompact('2024-01-01', undefined, 'en-US')).toBe('2024\n01-01');The test logic validates the intended behavior well, covering both UTC and local timezone scenarios.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/data-loader.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: Lint code using ESLint MCP server (available via Claude Code tools)
Format code with ESLint (writes changes) usingbun run format
No console.log allowed except where explicitly disabled with eslint-disable
Do not use console.log. Use logger.ts instead.
Files:
src/data-loader.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Type check with TypeScript using
bun typecheck
Files:
src/data-loader.ts
**/*.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.ts: File paths always use Node.js path utilities for cross-platform compatibility
Use.tsextensions for local file imports (e.g.,import { foo } from './utils.ts')
Prefer @praha/byethrow Result type over traditional try-catch for functional error handling
UseResult.try()for wrapping operations that may throw (JSON parsing, etc.)
UseResult.isFailure()for checking errors (more readable than!Result.isSuccess())
Use early return pattern (if (Result.isFailure(result)) continue;) instead of ternary operators
For async operations: create wrapper function withResult.try()then call it
Keep traditional try-catch only for: file I/O with complex error handling, legacy code that's hard to refactor
Always useResult.isFailure()andResult.isSuccess()type guards for better code clarity
Variables: start with lowercase (camelCase) - e.g.,usageDataSchema,modelBreakdownSchema
Types: start with uppercase (PascalCase) - e.g.,UsageData,ModelBreakdown
Constants: can use UPPER_SNAKE_CASE - e.g.,DEFAULT_CLAUDE_CODE_PATH
Only export constants, functions, and types that are actually used by other modules
Internal/private constants that are only used within the same file should NOT be exported
Always check if a constant is used elsewhere before making itexport constvs justconst
All test files must use current Claude 4 models, not outdated Claude 3 models
Test coverage should include both Sonnet and Opus models for comprehensive validation
Model names in tests must exactly match LiteLLM's pricing database entries
When adding new model tests, verify the model exists in LiteLLM before implementation
Tests depend on real pricing data from LiteLLM - failures may indicate model availability issues
Dynamic imports usingawait import()should only be used within test blocks to avoid tree-shaking issues
Mock data is created usingfs-fixturewithcreateFixture()for Claude data directory simulation
In-source testing pattern: Tests are written...
Files:
src/data-loader.ts
🧠 Learnings (4)
📚 Learning: applies to **/*.ts : variables: start with lowercase (camelcase) - e.g., `usagedataschema`, `modelbr...
Learnt from: CR
PR: ryoppippi/ccusage#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-19T10:58:04.397Z
Learning: Applies to **/*.ts : Variables: start with lowercase (camelCase) - e.g., `usageDataSchema`, `modelBreakdownSchema`
Applied to files:
src/data-loader.ts
📚 Learning: applies to **/*.ts : mock data is created using `fs-fixture` with `createfixture()` for claude data ...
Learnt from: CR
PR: ryoppippi/ccusage#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-19T10:58:04.397Z
Learning: Applies to **/*.ts : Mock data is created using `fs-fixture` with `createFixture()` for Claude data directory simulation
Applied to files:
src/data-loader.ts
📚 Learning: when aspulse points out that variables are clearly type-inferred as strings from their construction ...
Learnt from: AsPulse
PR: ryoppippi/ccusage#328
File: src/_live-rendering.ts:133-167
Timestamp: 2025-07-20T23:46:21.740Z
Learning: When AsPulse points out that variables are clearly type-inferred as strings from their construction (using template literals, string methods like .toFixed(), .padStart(), etc.), avoid suggesting unnecessary type assertions that would reduce code readability. ESLint type safety errors about "error type" often indicate type definition issues with imported packages rather than actual type problems in the user's code.
Applied to files:
src/data-loader.ts
📚 Learning: applies to **/*.ts : dynamic imports using `await import()` should only be used within test blocks t...
Learnt from: CR
PR: ryoppippi/ccusage#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-19T10:58:04.397Z
Learning: Applies to **/*.ts : Dynamic imports using `await import()` should only be used within test blocks to avoid tree-shaking issues
Applied to files:
src/data-loader.ts
🧬 Code Graph Analysis (1)
src/data-loader.ts (1)
src/_types.ts (1)
dailyDateSchema(30-32)
🪛 ESLint
src/data-loader.ts
[error] 572-572: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 572-572: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 572-572: Unsafe member access .safeParse on an error typed value.
(ts/no-unsafe-member-access)
[error] 573-573: Unexpected any value in conditional. An explicit comparison or type conversion is required.
(ts/strict-boolean-expressions)
[error] 573-573: Unsafe member access .success on an error typed value.
(ts/no-unsafe-member-access)
[error] 1446-1446: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1446-1446: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1446-1446: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
[error] 1449-1449: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1449-1449: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1449-1449: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
ryoppippi
changed the title
commit: |
2 participants
Suggested fix for issue #433.
formatDateCompact is being given a date in YYYY-MM-DD format that has already been converted to the local time zone. Constructing a new Date from that string interprets it as UTC, which causes an off-by-one error for users at a negative offset from UTC.
Summary by CodeRabbit