feat(companion): activity display#142
Merged
Merged
Conversation
- Update READMEs to reflect the new activity timeline feature in Companion - Add a new `activity_store.rs` module to handle reading and writing activity events - Implement `read_activity_events_default` to read activity events within a specified epoch range This introduces the activity timeline feature to the Workbranch Companion, allowing users to visualize their task sessions in a calendar view. The timeline is a navigation and reporting surface, with activity recording still driven by Companion refreshes and CLI state changes.
- Introduce `MIN_DISPLAY_LANE_SECONDS` to ensure a minimum visual length for sessions - Refactor `assignLanes` to accept a `minimumLaneSeconds` parameter - Create `assignDisplayLanes` as a wrapper for `assignLanes` with the new minimum - Remove `maybeRenderPlanTitle` and related logic for displaying plan titles - Simplify session rendering in `ActivityCalendarView` to only show task and time - Update `assignLanes` to use `laneEnd` for calculating cluster end times, respecting the minimum lane duration - Add a test case for `assignLanes` to verify correct lane separation with `minimumLaneSeconds` - Adjust existing `CalendarTimeline` tests to reflect the updated display logic This change improves the readability of the activity calendar by ensuring that even very short sessions are visually distinct and occupy a reasonable amount of space. It also simplifies the display logic by removing the conditional rendering of plan titles, focusing on the core task and time information. The `assignLanes` function is now more flexible, allowing for different minimum display durations based on context.
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a Companion Activity view: a Rust Tauri command reads append-only activity log events within an epoch range, a new TS calendar domain module derives sessions/lanes/color indices, and a React ChangesCompanion Activity Calendar Timeline
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant View as ActivityCalendarView
participant Client as tauriClient
participant Command as read_activity_events (Rust)
participant Store as activity_store.rs
participant Domain as calendar.ts
View->>Client: readActivityEvents(fromEpoch, toEpoch)
Client->>Command: invoke("read_activity_events", {fromEpoch, toEpoch})
Command->>Store: read_activity_events_default(from, to)
Store->>Store: read_activity_events_in_range(path, from, to)
Store-->>Command: Vec<serde_json::Value>
Command-->>Client: Vec<Value>
Client->>Domain: calendarEventFromUnknown(each item)
Domain-->>Client: CalendarEventInput[] (filtered)
Client-->>View: readonly CalendarEventInput[]
View->>Domain: sessionsFromEvents / clipToRange / assignLanes / assignDisplayLanes
Domain-->>View: LaneSession[]
View->>View: render CalendarTimeline
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 032350901d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
apps/companion/src-tauri/src/activity_store.rs (1)
96-124: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftFull linear scan of the append-only log on every read.
read_activity_events_in_rangereads and JSON-parses every line inactivity.jsonlregardless of the requested range, on every invocation from the UI (viaspawn_blocking). As the log grows unbounded over long-term usage, this becomes an O(n) scan per calendar view/period-navigation request. Consider maintaining a lightweight index (e.g., periodic offsets by day) or log rotation/compaction so lookups scale with the requested range rather than total log size.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/companion/src-tauri/src/activity_store.rs` around lines 96 - 124, `read_activity_events_in_range` currently does a full line-by-line JSON scan of `activity.jsonl` on every call, which will not scale as the log grows. Update the `read_activity_events_in_range` path (and any related append-only log handling in `activity_store.rs`) to use a lightweight lookup strategy such as a periodic index by time bucket or log rotation/compaction, so range reads only touch the relevant portion of the file instead of all events.apps/companion/tests/activity-calendar.test.tsx (1)
387-399: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a three-day test with cross-day sessions.
The "renders three day columns" test only passes
sessions={[]}. Consider adding a case with sessions on day 2/3 to cover thehourRangecomputation for non-empty multi-day sets — this would have caught thehourRangeday-reference bug flagged incalendar.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/companion/tests/activity-calendar.test.tsx` around lines 387 - 399, Add a three-day CalendarTimeline test that includes sessions spanning day 2 and day 3 instead of only sessions={[]}, so the hourRange path is exercised with a non-empty multi-day set. Extend the existing renders three day columns in three-day mode case in activity-calendar.test.tsx using CalendarTimeline and the day/session fixtures to verify the computed range still renders correctly across multiple days and catches the calendar.ts day-reference issue.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/companion/src-tauri/src/activity_store.rs`:
- Around line 109-122: The `read_activity_events_in_range` loop currently uses
`line?`, which aborts the entire read on invalid UTF-8 instead of skipping the
bad entry like the existing malformed JSON and missing `observedAt` cases.
Update the `BufReader::new(file).lines()` handling in
`read_activity_events_in_range` to treat per-line read/decoding failures as
skippable and continue scanning subsequent lines. Keep the same filtering
behavior for valid `serde_json::Value` records and only push events that pass
the `observedAt` range check.
In `@apps/companion/src/activity/calendar.ts`:
- Around line 267-291: The hour bounds in hourRange are being calculated from a
single dayStart even when sessions span multiple visible days, so day 2/3
sessions get clamped to 24 and force the full range. Update hourRange in
calendar.ts to normalize each CalendarSession against its own containing day
before computing sessionStartHour/sessionEndHour, and keep the clampHour
behavior only as a final safety net. Use the hourRange and clampHour symbols to
locate the fix, and add a regression test covering multi-day sessions in
three-day CalendarTimeline usage.
In `@README.ko.md`:
- Line 14: The README.ko.md wording still conflates the CLI state source with
the activity timeline source; update the affected intro text and the
Companion/Activity description so it clearly says the CLI reads
workspace/project state while the Activity view is powered by the local
append-only log. Use the existing README sections around the shared `workbranch`
description and the Companion feature summary to separate these contracts
without implying `workbranch list --global --json` feeds the activity timeline.
In `@README.md`:
- Line 14: Update the README wording to separate the CLI state from the
activity-log source: the CLI state should be described as coming from the shared
workbranch project state, while the Activity view should be described as reading
from the local append-only log. Adjust the copy in the main overview and any
matching text near the later reference so terms like “same workbranch project
state” no longer imply `workbranch list --global --json` feeds the activity
timeline; use the existing README section text to locate and revise the affected
descriptions.
---
Nitpick comments:
In `@apps/companion/src-tauri/src/activity_store.rs`:
- Around line 96-124: `read_activity_events_in_range` currently does a full
line-by-line JSON scan of `activity.jsonl` on every call, which will not scale
as the log grows. Update the `read_activity_events_in_range` path (and any
related append-only log handling in `activity_store.rs`) to use a lightweight
lookup strategy such as a periodic index by time bucket or log
rotation/compaction, so range reads only touch the relevant portion of the file
instead of all events.
In `@apps/companion/tests/activity-calendar.test.tsx`:
- Around line 387-399: Add a three-day CalendarTimeline test that includes
sessions spanning day 2 and day 3 instead of only sessions={[]}, so the
hourRange path is exercised with a non-empty multi-day set. Extend the existing
renders three day columns in three-day mode case in activity-calendar.test.tsx
using CalendarTimeline and the day/session fixtures to verify the computed range
still renders correctly across multiple days and catches the calendar.ts
day-reference issue.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c25feff8-a878-4e97-82ab-0758a5fbba84
📒 Files selected for processing (17)
README.ko.mdREADME.mdapps/companion/README.mdapps/companion/src-tauri/src/activity_store.rsapps/companion/src-tauri/src/lib.rsapps/companion/src/App.tsxapps/companion/src/activity/ActivityCalendarView.tsxapps/companion/src/activity/activity-calendar.cssapps/companion/src/activity/calendar.tsapps/companion/src/application/activity.tsapps/companion/src/infrastructure/tauriClient.tsapps/companion/src/style.cssapps/companion/src/styles/themes.cssapps/companion/tests/activity-calendar.test.tsxapps/companion/tests/tauri-client.test.tsdocs/plans/0041-companion-activity-calendar-timeline.mdpackage.json
- Update READMEs (English and Korean) to distinguish activity view data source - Explain that activity view reads from a local append-only log - Clarify that companion itself shows workspace/project state The previous documentation might have implied that the activity timeline was also derived from the shared `workbranch list --global --json` output. This change clarifies that the activity view uses a separate local log for its timeline, providing a more accurate description of the system's architecture. Fixes: #123
- Extract `assignDisplayLanes` into a new `displaySessions.ts` module - Implement `coalesceDisplaySessions` to merge close, non-overlapping sessions for the same task into a single display session - Update `assignDisplayLanes` to use the new coalescing logic - Add tests for `assignDisplayLanes` to verify coalescing behavior This change improves the visual representation of activity sessions in the calendar view by reducing clutter and making it easier to understand continuous work on a single task, even if it's broken into multiple short sessions.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/companion/tests/activity-display-sessions.test.ts (1)
17-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a regression test for sessions exceeding the 90-minute display-lane threshold.
Both cases here use sub-90-minute sessions. Consider adding a case where one same-task session's own duration already exceeds
MIN_DISPLAY_LANE_SECONDS(90 min) followed by a short-gap continuation — this is the scenario affected by theshouldCoalesce/displayLaneEndanchoring issue flagged indisplaySessions.ts(lines 13-41), and would guard against regressions once fixed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/companion/tests/activity-display-sessions.test.ts` around lines 17 - 51, Add a regression test in assignDisplayLanes to cover same-task sessions where the first session already exceeds MIN_DISPLAY_LANE_SECONDS and is followed by a short-gap continuation, since the current cases only cover sub-90-minute sessions. Use sessionsFromEvents with event entries to create that over-threshold scenario and assert the resulting lane is coalesced correctly, with the expected laneCount, start/end, and single-lane behavior. Reference assignDisplayLanes and the threshold/anchoring behavior from displaySessions.ts to ensure the test protects the shouldCoalesce and displayLaneEnd fix.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/companion/src/activity/displaySessions.ts`:
- Around line 13-41: The coalescing logic in shouldCoalesce/displayLaneEnd is
using a start-anchored window that collapses once a session exceeds
MIN_DISPLAY_LANE_SECONDS, so long sessions can no longer merge even with tiny
gaps. Update shouldCoalesce to use a fixed gap tolerance for the next session
start, and adjust mergeDisplaySession so the merged session’s start is updated
appropriately when combining CalendarSession blocks. Keep the changes localized
to displayLaneEnd, shouldCoalesce, and mergeDisplaySession.
---
Nitpick comments:
In `@apps/companion/tests/activity-display-sessions.test.ts`:
- Around line 17-51: Add a regression test in assignDisplayLanes to cover
same-task sessions where the first session already exceeds
MIN_DISPLAY_LANE_SECONDS and is followed by a short-gap continuation, since the
current cases only cover sub-90-minute sessions. Use sessionsFromEvents with
event entries to create that over-threshold scenario and assert the resulting
lane is coalesced correctly, with the expected laneCount, start/end, and
single-lane behavior. Reference assignDisplayLanes and the threshold/anchoring
behavior from displaySessions.ts to ensure the test protects the shouldCoalesce
and displayLaneEnd fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6446f943-e431-45b3-8acc-3d42d3134e1b
📒 Files selected for processing (3)
apps/companion/src/activity/ActivityCalendarView.tsxapps/companion/src/activity/displaySessions.tsapps/companion/tests/activity-display-sessions.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/companion/src/activity/ActivityCalendarView.tsx
- Adjust `displayLaneEnd` calculation to always extend the end time - Modify `shouldCoalesce` to use inclusive comparison for `next.start` - Add `start: Math.min(current.start, next.start)` to `mergeDisplaySession` The previous logic for `displayLaneEnd` could result in a shorter display lane than intended if `session.end` was already greater than `session.start + MIN_DISPLAY_LANE_SECONDS`. This change ensures the display lane always extends by `MIN_DISPLAY_LANE_SECONDS` from the session's end. The `shouldCoalesce` function was updated to use an inclusive comparison for `next.start <= displayLaneEnd(current)`. This allows sessions that start exactly at the calculated display lane end to be coalesced, which was previously missed. Additionally, `mergeDisplaySession` now correctly sets the start of the merged session to the minimum of the current and next session's start times, ensuring that the merged session spans the entire duration of both original sessions. These changes fix an issue where closely spaced sessions, especially longer ones with small gaps, were not being correctly coalesced into a single display session, leading to fragmented display in the activity view.
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
1 participant
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 by CodeRabbit