-
-
Notifications
You must be signed in to change notification settings - Fork 317
feat: add configurable session length to blocks command #92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add DEFAULT_SESSION_DURATION_HOURS constant (5 hours) - Update identifySessionBlocks to accept optional sessionDurationHours parameter - Update createBlock and createGapBlock to use configurable duration - Add sessionDurationHours to LoadOptions type in data-loader - Pass session duration through loadSessionBlockData to identifySessionBlocks This change maintains backward compatibility by using 5 hours as the default session duration when no custom value is provided.
- Test custom session durations (1h, 2h, 2.5h, 3h, 24h, 0.5h) - Test gap block creation with custom durations - Test edge cases (exactly equal durations, fractional hours) - Test backward compatibility (default 5h behavior) - All tests verify correct block creation, gap detection, and timing Covers various scenarios to ensure robust session block identification with configurable durations across different time ranges.
|
Caution Review failedThe pull request is closed. WalkthroughA configurable session duration parameter was introduced, allowing the session block duration to be specified via a command-line argument. This parameter propagates through the command, data loading, and session block identification logic, replacing the previously hardcoded 5-hour duration. Comprehensive tests were added to verify correct behavior with various durations. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI (blocksCommand)
participant DataLoader
participant SessionBlocks
User->>CLI (blocksCommand): Run with --sessionLength/-l argument
CLI (blocksCommand)->>DataLoader: loadSessionBlockData({ sessionDurationHours })
DataLoader->>SessionBlocks: identifySessionBlocks(entries, sessionDurationHours)
SessionBlocks-->>DataLoader: SessionBlock[]
DataLoader-->>CLI (blocksCommand): SessionBlock[]
CLI (blocksCommand)-->>User: Output session block data
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
✨ 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 (
|
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @ryoppippi, 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 introduces a significant enhancement to the blocks command by making the session block duration configurable. Previously fixed at 5 hours, users can now define custom session lengths via a new CLI option, providing greater flexibility for analyzing usage data. This change impacts how usage entries are grouped into sessions and how idle periods are identified as gaps, while maintaining full backward compatibility with the default 5-hour duration.
Highlights
- Configurable Session Length: Added a new
--session-length(or-l) CLI option to theblockscommand, allowing users to specify the duration of a session block in hours. This option supports any positive number, including decimals, with a default of 5 hours for backward compatibility. - Input Validation: Implemented robust input validation for the new
sessionLengthoption, ensuring that only positive numerical values are accepted, and providing an error message if an invalid value is provided. - Core Logic Adaptation: Updated the underlying
identifySessionBlocksfunction and its helpers (createBlock,createGapBlock) to dynamically use the configured session duration. This change impacts how usage entries are grouped into sessions, how block end times are calculated, and how idle periods are identified as gaps. - Comprehensive Test Coverage: Expanded the test suite with 9 new comprehensive tests specifically for the configurable session duration. These tests cover various scenarios, including different custom durations (e.g., 1h, 2.5h, 24h), fractional hours, and edge cases like exact duration matches and default behavior, ensuring the feature works as expected.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request adds a configurable session length to the blocks command. The changes are well-implemented, and the addition of comprehensive unit tests is commendable. A few minor areas for potential code clarification were identified.
| if (ctx.values.sessionLength <= 0) { | ||
| logger.error('Session length must be a positive number'); | ||
| process.exit(1); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| function createBlock(startTime: Date, entries: LoadedUsageEntry[], now: Date, sessionDurationMs: number): SessionBlock { | ||
| const endTime = new Date(startTime.getTime() + sessionDurationMs); | ||
| const lastEntry = entries[entries.length - 1]; | ||
| const actualEndTime = lastEntry != null ? lastEntry.timestamp : startTime; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fallback to startTime in const actualEndTime = lastEntry != null ? lastEntry.timestamp : startTime; seems unnecessary, as lastEntry should never be null here. Simplify this to const actualEndTime = lastEntry!.timestamp;.
| const actualEndTime = lastEntry != null ? lastEntry.timestamp : startTime; | |
| const actualEndTime = lastEntry!.timestamp; // entries array is guaranteed to be non-empty here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a configurable session duration for the blocks command, allowing users to override the default 5-hour window.
- Updated core logic in
identifySessionBlocksand related helpers to accept asessionDurationHoursparameter and compute durations in milliseconds. - Propagated the new
sessionDurationHoursoption through the data loader and CLI (--session-length), including validation. - Added extensive tests covering custom durations (including fractional hours), gap detection, and backward compatibility.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/session-blocks.internal.ts | Refactored identifySessionBlocks, createBlock, and createGapBlock to accept and use a configurable duration parameter. |
| src/session-blocks.internal.test.ts | Added a new test suite verifying behavior for various custom session lengths and edge cases. |
| src/data-loader.ts | Extended LoadOptions with sessionDurationHours and passed it into identifySessionBlocks. |
| src/commands/blocks.ts | Introduced the --session-length (-l) CLI flag, defaulting to 5h, with validation. |
Comments suppressed due to low confidence (2)
src/commands/blocks.ts:115
- [nitpick] The CLI option is named
sessionLength, but the internal API parameter issessionDurationHours. Consider renaming the flag and context variables to match (sessionDurationHours) for clearer alignment between layers.
sessionLength: {
src/commands/blocks.ts:130
- [nitpick] This error message could be more user-friendly by naming the specific flag, e.g.,
Invalid value for --session-length: must be a positive number, so users know exactly which input failed.
logger.error('Session length must be a positive number');
| export function identifySessionBlocks( | ||
| entries: LoadedUsageEntry[], | ||
| sessionDurationHours = DEFAULT_SESSION_DURATION_HOURS, | ||
| ): SessionBlock[] { |
Copilot
AI
Jun 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a guard in identifySessionBlocks to validate that sessionDurationHours is positive (e.g., throw or early return) to avoid unexpected behavior if called directly with invalid values.
| ): SessionBlock[] { | |
| ): SessionBlock[] { | |
| if (sessionDurationHours <= 0) { | |
| throw new Error("sessionDurationHours must be a positive number."); | |
| } |
feat: add configurable session length to blocks command
Summary
Changes Made
Core Implementation
Testing
Validation
Usage Examples
Default 5-hour sessions (backward compatible):
bun run start blocks
Custom 3-hour sessions:
bun run start blocks --session-length 3
Short 1-hour sessions for fine-grained tracking:
bun run start blocks -l 1
Fractional hours:
bun run start blocks --session-length 2.5
Works with all existing options:
bun run start blocks --session-length 8 --recent --json
Test Plan
Implementation Notes
The feature correctly handles:
Full backward compatibility maintained while providing flexibility for different use cases.
Summary by CodeRabbit