Using UTC year/month/day in data-loader #312
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. 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 @robKitson, 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!
This pull request standardizes date formatting within the data-loader by ensuring that all date components (year, month, and day) are extracted and formatted using Coordinated Universal Time (UTC) rather than the local time zone. This change affects two utility functions, formatDate and formatDateCompact, promoting consistency and preventing potential timezone-related discrepancies in date representation.
Highlights
- Date Formatting Consistency: The
formatDatefunction, responsible for formatting a date string into aYYYY-MM-DDformat, has been modified to use UTC-based methods (getUTCFullYear,getUTCMonth,getUTCDate) instead of local time methods (getFullYear,getMonth,getDate). - Compact Date Formatting: Similarly, the
formatDateCompactfunction, which formats a date string into aYYYY\nMM-DDformat, has also been updated to use UTC-based methods for extracting the year, month, and day.
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 is currently in preview and 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 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 updates the date formatting functions to use UTC methods, which will prevent timezone-related inconsistencies. The change is well-contained and improves the robustness of date handling. I've suggested a small refactoring to reduce code duplication between the two formatting functions for better maintainability.
src/data-loader.ts
Outdated
| const date = new Date(dateStr); | ||
| const year = date.getFullYear(); | ||
| const month = String(date.getMonth() + 1).padStart(2, '0'); | ||
| const day = String(date.getDate()).padStart(2, '0'); | ||
| const year = date.getUTCFullYear(); | ||
| const month = String(date.getUTCMonth() + 1).padStart(2, '0'); | ||
| const day = String(date.getUTCDate()).padStart(2, '0'); | ||
| return `${year}\n${month}-${day}`; |
There was a problem hiding this comment.
To improve maintainability and reduce code duplication, you can refactor formatDateCompact to reuse the formatDate function. The logic for parsing the date and extracting its UTC components is identical in both functions.
By calling formatDate and then transforming its output, you can make the code more concise and ensure that any future changes to the date formatting logic are applied consistently across both functions.
return formatDate(dateStr).replace('-', '\n');![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
**/*.{ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
**/*.ts
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
Refactored formatDate and formatDateCompact to use shared formatDateInternal method that properly detects timezone indicators (Z or ±HH:MM) and uses appropriate date getters (UTC vs local) based on the input format. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/data-loader.ts (4)
423-441: Well-implemented timezone-aware date formatting solutionThe new
formatDateInternalfunction effectively addresses the UTC vs local date parsing concerns raised in previous reviews. The regex pattern/Z|[+-]\d{2}:\d{2}/correctly detects timezone indicators, and the conditional logic appropriately uses UTC getters for timezone-aware strings and local getters for naive strings.However, there are trailing spaces that need to be cleaned up:
const date = new Date(dateStr); - + // Detect if the string includes UTC indicator (Z) or timezone offset (±HH:MM) const hasTimezone = /Z|[+-]\d{2}:\d{2}/.test(dateStr); - + // Use UTC getters if timezone is specified, otherwise use local getters const year = hasTimezone ? date.getUTCFullYear() : date.getFullYear(); const month = String(hasTimezone ? date.getUTCMonth() + 1 : date.getMonth() + 1).padStart(2, '0'); const day = String(hasTimezone ? date.getUTCDate() : date.getDate()).padStart(2, '0'); - + return twoLine ? `${year}\n${month}-${day}` : `${year}-${month}-${day}`;
1157-1181: Comprehensive test coverage with unsafe TypeScript callsThe test cases properly verify both UTC and local date handling scenarios, which is excellent coverage for the timezone-aware functionality. However, there are unsafe TypeScript calls that need to be addressed.
Fix the unsafe TypeScript calls in the test expectations:
it('formats UTC timestamps using UTC date', () => { // UTC timestamps should always use UTC date regardless of local timezone - expect(formatDate('2024-01-01T00:00:00Z')).toBe('2024-01-01'); - expect(formatDate('2024-12-31T23:59:59Z')).toBe('2024-12-31'); - expect(formatDate('2024-01-01T12:00:00.000Z')).toBe('2024-01-01'); + expect(formatDate('2024-01-01T00:00:00Z')).toBe('2024-01-01'); + expect(formatDate('2024-12-31T23:59:59Z')).toBe('2024-12-31'); + expect(formatDate('2024-01-01T12:00:00.000Z')).toBe('2024-01-01'); });Also remove the trailing space on line 1178:
const expected = `${expectedYear}-${expectedMonth}-${expectedDay}`; - + expect(formatDate('2024-01-01')).toBe(expected);
1190-1215: Comprehensive test coverage for compact formatThe test cases for
formatDateCompactmirror theformatDatetests appropriately, ensuring both functions handle timezone scenarios consistently.Remove the trailing space on line 1211:
const expected = `${expectedYear}\n${expectedMonth}-${expectedDay}`; - + expect(formatDateCompact('2024-01-01')).toBe(expected);
1157-1215: Address unsafe TypeScript calls throughout the test suiteThe static analysis correctly identifies numerous unsafe TypeScript calls in the test expectations. While these are just test assertions, they should be properly typed for consistency.
The unsafe calls appear to be due to TypeScript not properly inferring the return types of the
formatDateandformatDateCompactfunctions in the test context. This is likely a configuration issue rather than a code problem, but the trailing spaces should still be cleaned up as noted in the individual comments above.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/data-loader.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
**/*.{ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
**/*.ts
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: ryoppippi/ccusage#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-02T14:48:21.506Z
Learning: Applies to **/*.{ts,tsx} : Tests depend on real pricing data from LiteLLM - failures may indicate model availability issues
src/data-loader.ts (4)
<retrieved_learning>
Learnt from: CR
PR: ryoppippi/ccusage#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-02T14:48:21.506Z
Learning: Applies to **/*.{ts,tsx} : Tests depend on real pricing data from LiteLLM - failures may indicate model availability issues
</retrieved_learning>
<retrieved_learning>
Learnt from: CR
PR: ryoppippi/ccusage#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-02T14:48:21.506Z
Learning: Applies to **/*.{ts,tsx} : Test coverage should include both Sonnet and Opus models for comprehensive validation
</retrieved_learning>
<retrieved_learning>
Learnt from: CR
PR: ryoppippi/ccusage#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-02T14:48:21.506Z
Learning: Applies to **/*.{ts,tsx} : Tests are written directly in the same files as the source code, not in separate test files, using if (import.meta.vitest != null) blocks
</retrieved_learning>
<retrieved_learning>
Learnt from: CR
PR: ryoppippi/ccusage#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-02T14:48:21.506Z
Learning: Applies to **/*.{ts,tsx} : Vitest globals (describe, it, expect) are available automatically without imports
</retrieved_learning>
🪛 ESLint
src/data-loader.ts
[error] 431-431: Trailing spaces not allowed.
(style/no-trailing-spaces)
[error] 434-434: Trailing spaces not allowed.
(style/no-trailing-spaces)
[error] 439-439: Trailing spaces not allowed.
(style/no-trailing-spaces)
[error] 1157-1157: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1159-1159: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1159-1159: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1159-1159: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
[error] 1160-1160: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1160-1160: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1160-1160: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
[error] 1161-1161: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1161-1161: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1161-1161: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
[error] 1164-1164: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1166-1166: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1166-1166: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1166-1166: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
[error] 1167-1167: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1167-1167: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1167-1167: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
[error] 1168-1168: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1168-1168: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1168-1168: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
[error] 1171-1171: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1178-1178: Trailing spaces not allowed.
(style/no-trailing-spaces)
[error] 1179-1179: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1179-1179: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1179-1179: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
[error] 1180-1180: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1180-1180: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1180-1180: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
[error] 1190-1190: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1192-1192: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1192-1192: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1192-1192: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
[error] 1193-1193: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1193-1193: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1193-1193: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
[error] 1194-1194: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1194-1194: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1194-1194: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
[error] 1197-1197: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1199-1199: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1199-1199: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1199-1199: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
[error] 1200-1200: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1200-1200: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1200-1200: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
[error] 1201-1201: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1201-1201: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1201-1201: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
[error] 1204-1204: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1211-1211: Trailing spaces not allowed.
(style/no-trailing-spaces)
[error] 1212-1212: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1212-1212: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1212-1212: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
[error] 1213-1213: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1213-1213: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 1213-1213: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
🔇 Additional comments (4)
src/data-loader.ts (4)
448-450: Clean refactoring that eliminates code duplicationThe refactoring of
formatDateto delegate toformatDateInternalis clean and maintains backward compatibility while eliminating code duplication.
457-459: Consistent refactoring approachThe
formatDateCompactfunction follows the same clean delegation pattern, ensuring consistency across both formatting functions.
1164-1169: Good test coverage for timezone offset stringsThe test cases correctly verify that strings with timezone offsets are handled using UTC date parsing.
1171-1181: Proper local date handling verificationThe test correctly verifies that strings without timezone indicators use local date interpretation, which preserves backward compatibility.
commit: |
This reverts commit 6eaf257.
2 participants
#311
Summary by CodeRabbit