-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor: remove unnecessary sorting and consolidate load functions #86
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
- Remove event count sorting from loadSessionsInTimeRange and loadAllSessions - Sorting is handled later by sortTimelines function, making this sort redundant - Improves performance by avoiding duplicate sorting operations 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughThe changes remove the default fallback for the CLI Changes
Sequence Diagram(s)sequenceDiagram
participant CLI
participant App
participant Parser
CLI->>App: Pass options (color directly, no fallback)
App->>Parser: loadSessionsInTimeRange(startTime?, endTime?, projectNames?, progressTracker?)
Parser->>Parser: Traverse all project directories, collect .jsonl files
Parser->>Parser: Parse files concurrently with Promise.allSettled
Parser->>Parser: Aggregate fulfilled events using Array.flat()
Parser-->>App: Return session timelines
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (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 (
|
- Make loadSessionsInTimeRange handle both time-ranged and all-time queries - Convert loadAllSessions to a simple wrapper function - Reduce code duplication and improve maintainability - All existing tests pass without modification 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…n loadEventsFromProjects
9f2082f to
b2e3691
Compare
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/core/git/index.ts (1)
8-8: Remove debug logging before production deployment.This debug log should be removed or replaced with a proper logging framework that supports log levels, as console.log statements are typically not desired in production code.
- console.log(gitPath);src/core/parser/index.ts (1)
26-26: Remove debug logging before production deployment.This debug log should be removed or replaced with a proper logging framework that supports log levels, as console.log statements are typically not desired in production code.
- console.log({ repoName });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/core/git/index.ts(1 hunks)src/core/parser/index.ts(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/core/parser/index.ts (2)
src/utils/progressTracker.ts (1)
ProgressTracker(8-55)src/models/events.ts (1)
SessionTimeline(24-33)
🔇 Additional comments (5)
src/core/parser/index.ts (5)
75-89: Function signature change looks correct for consolidation.The changes make
startTimeandendTimeoptional and adjust the filtering logic appropriately. This enables the function to handle both time-ranged and all-time queries as intended by the PR objectives.
94-94: Good removal of redundant sorting.Correctly removes the redundant sorting by event count since the PR objectives state that sorting is handled later by the
sortTimelinesfunction. This eliminates unnecessary processing.
101-101: Excellent consolidation -loadAllSessionsis now a simple wrapper.This change successfully eliminates code duplication by making
loadAllSessionsa simple wrapper that callsloadSessionsInTimeRangewith undefined time parameters, which aligns perfectly with the PR objectives.
130-160: Restructured file loading logic improves error handling.The restructuring adds proper error handling for inaccessible subdirectories and moves project filtering earlier in the process for better efficiency. The separation of directory existence checks and file discovery is cleaner.
175-175: UsingArray.flat()is more efficient than manual concatenation.The change from manual array concatenation to
Array.flat()is a good optimization that makes the code more concise and readable while maintaining the same functionality.
b76f3ab to
c399399
Compare
- Replace try/catch with explicit directory existence checks using stat() - Optimize array concatenation using Array.flat() instead of manual indexing - Add decodeClaudeProjectPath to properly decode encoded directory names - Remove debug console.log statements for cleaner output 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
c399399 to
bb3af53
Compare
…events in active duration calculation
|
🎉 This PR is included in version 2.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary
sortTimelineslater)loadAllSessionsandloadSessionsInTimeRangeto reduce code duplicationloadSessionsInTimeRangehandle both time-ranged and all-time queriesloadAllSessionsto a simple wrapper functionChanges
sort((a, b) => b.eventCount - a.eventCount)callsstartTimeandendTimeoptional inloadSessionsInTimeRangeTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit