feat(coordination): unified worklist screen (Variant C)#173
Conversation
Add pure buildCoordinationWorklist: merges the reconciled notification model and the genuine (non-notification) threads into one agent-keyed, urgency-sorted stream. Each row carries a type (msg/note/task/review/handoff/conversation) and a bucket (needs-you/handled/unreachable) on a unified urgency scale so notifications and threads interleave sensibly (live needs-input > on-you thread > pending thread > offline unread > handled > stale > missing). Threads are first-class rows (the model's per-item thread annotation is ignored) so each appears exactly once. Dormant until the view is cut over. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the two-section Inbox/Threads Coordination screen with one agent-keyed, urgency-sorted worklist driven by buildCoordinationWorklist. Single selection index (coordinationIndex); Tab is now a filter (all/threads) rather than a section switch; per-row actions dispatch by item kind — notifications keep r/c/Enter (+ global R/C), threads keep s/A/c/b/o/x/P/J/E/Enter. refreshNotificationEntries now also builds threadEntries + the worklist so every mutation site keeps the screen coherent. Rows show a type chip (msg/note/task/review/handoff/conversation), reachability/stale tags, and bucket rules (Needs you / Handled / Unreachable); the detail pane dispatches by kind. openSelectedNotification → openCoordinationNotification (agent rollup). The reply overlay still reads threadEntries[threadIndex], synced from the selected worklist thread on reply. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Polish the unified worklist to match the Variant C mockup. The footer now
reflects the selected row's actions (notification verbs vs thread verbs). Bucket
boundaries render as dashboard-style titled rules ("Needs you ── N", accent for
the actionable bucket) instead of muted dividers. The detail pane is wrapped in
rounded card()s (Inbox/Thread + Body/Messages), matching the Dashboard. Pure
presentation; no model/handler changes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 2 hours, 33 minutes, and 58 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. 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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a ChangesUnified Coordination Worklist
Sequence Diagram(s)sequenceDiagram
participant User
participant CoordinationKeyHandler
participant refreshNotificationEntries
participant buildCoordinationWorklist
participant openCoordinationNotification
participant renderCoordinationScreen
User->>CoordinationKeyHandler: open coordination screen
CoordinationKeyHandler->>refreshNotificationEntries: rebuild entries
refreshNotificationEntries->>buildCoordinationWorklist: modelInput + threads
buildCoordinationWorklist-->>refreshNotificationEntries: merged WorklistItem[]
refreshNotificationEntries-->>CoordinationKeyHandler: host.coordinationWorklist set
CoordinationKeyHandler->>renderCoordinationScreen: draw with coordinationWorklist + coordinationIndex
User->>CoordinationKeyHandler: press Tab
CoordinationKeyHandler-->>CoordinationKeyHandler: flip coordinationFilter, reset coordinationIndex=0
CoordinationKeyHandler->>renderCoordinationScreen: redraw filtered view
User->>CoordinationKeyHandler: press Enter on selected item
CoordinationKeyHandler->>openCoordinationNotification: host + WorklistItem
openCoordinationNotification-->>CoordinationKeyHandler: activate session/service, mark read, settle
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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
🧹 Nitpick comments (4)
src/multiplexer/notifications.ts (1)
191-197: ⚡ Quick winDuplicate of
readNotificationItemin coordination.ts.This function is identical to
readNotificationItematsrc/multiplexer/coordination.ts:55-60(see context snippet 1). Consider exporting one version and reusing it to avoid divergence.🤖 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 `@src/multiplexer/notifications.ts` around lines 191 - 197, The function markCoordinationItemRead in notifications.ts is a duplicate of readNotificationItem from coordination.ts that performs identical logic. Remove the markCoordinationItemRead function definition and instead import readNotificationItem from coordination.ts, then update all call sites in notifications.ts to use the imported readNotificationItem function instead of the local markCoordinationItemRead function.src/coordination-model.ts (2)
281-286: ⚡ Quick winDuplicate model construction on every refresh.
buildCoordinationWorklistinternally callsbuildCoordinationModel(input)on line 285. However, inrefreshNotificationEntries(src/multiplexer/notifications.ts:101), the caller already builds the model forhost.coordinationModel. This results in the same model being constructed twice per refresh cycle.Consider accepting the pre-built model as an optional parameter or refactoring to avoid the redundant computation.
♻️ Suggested refactor
export function buildCoordinationWorklist( - input: BuildCoordinationModelInput & { currentParticipant?: string }, + input: BuildCoordinationModelInput & { currentParticipant?: string; model?: CoordinationModel }, ): CoordinationWorklist { const participant = input.currentParticipant ?? "user"; - const model = buildCoordinationModel(input); + const model = input.model ?? buildCoordinationModel(input); const items: WorklistItem[] = [];Then in
refreshNotificationEntries:const model = buildCoordinationModel(modelInput); host.coordinationModel = model; // ... - const worklist = buildCoordinationWorklist(modelInput); + const worklist = buildCoordinationWorklist({ ...modelInput, model });🤖 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 `@src/coordination-model.ts` around lines 281 - 286, The buildCoordinationWorklist function unnecessarily calls buildCoordinationModel internally on every invocation, even though refreshNotificationEntries in notifications.ts already builds this model separately and passes it to host.coordinationModel, causing duplicate model construction per refresh cycle. Refactor buildCoordinationWorklist to accept the pre-built coordination model as an optional parameter (e.g., coordinationModel?: CoordinationModel) in the input object, and use that provided model instead of calling buildCoordinationModel internally. Update the caller in refreshNotificationEntries to pass the already-constructed model to avoid redundant computation.
260-263: 💤 Low valueStale notifications may be incorrectly bucketed as "unreachable".
The
notificationBucketfunction places stale items in the"unreachable"bucket alongside missing-target items. However, stale notifications are still reachable (the agent exists and is live); they're just likely outdated. This conflates two different semantic states in the UI grouping.Consider whether stale items should fall into
"handled"instead, or introduce a separate bucket for display differentiation.🤖 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 `@src/coordination-model.ts` around lines 260 - 263, The notificationBucket function incorrectly groups stale items with missing-target items in the "unreachable" bucket, but stale notifications are still reachable and should be treated differently to reflect their distinct semantic state. Modify the conditional logic in notificationBucket to separate the handling of missing items (where reachability === "missing") from stale items, so that only truly unreachable items return "unreachable" and stale items are instead placed in the "handled" bucket or a new separate bucket for stale notifications depending on the desired UI behavior.src/multiplexer/subscreens.test.ts (1)
78-79: 💤 Low valueMissing
keyfield in test mock for consistency.Line 40's worklist item includes
key: "t:thread-1", but this mock is missing thekeyfield. While the current code doesn't appear to rely onkeyduring dispatch, adding it maintains consistency with theWorklistItemcontract and protects against future changes that might use it.Suggested fix
- coordinationWorklist: [{ kind: "thread", thread: buildThreadEntries()[0] }], + coordinationWorklist: [{ kind: "thread", key: "t:test-thread", thread: buildThreadEntries()[0] }],🤖 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 `@src/multiplexer/subscreens.test.ts` around lines 78 - 79, The worklist item in the coordinationWorklist array is missing the key field that exists in the similar mock at line 40. Add the key field to the worklist item object (the one with kind "thread" and thread property) to match the WorklistItem contract structure, using a consistent format similar to what appears on line 40 (e.g., key: "t:thread-1" or an appropriate identifier based on the buildThreadEntries() result).
🤖 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 `@src/multiplexer/notifications.ts`:
- Around line 116-118: The `refreshNotificationEntries` function clamps
`coordinationIndex` to 0 when the worklist is empty using Math.max(0,
host.coordinationWorklist.length - 1), but the `ensureNotificationState`
function at line 144 initializes `coordinationIndex` to -1 when the list is
empty. Fix this inconsistency by updating `ensureNotificationState` to
initialize `coordinationIndex` to 0 instead of -1 when the coordination worklist
is empty, ensuring both functions use the same convention for empty-list
handling.
---
Nitpick comments:
In `@src/coordination-model.ts`:
- Around line 281-286: The buildCoordinationWorklist function unnecessarily
calls buildCoordinationModel internally on every invocation, even though
refreshNotificationEntries in notifications.ts already builds this model
separately and passes it to host.coordinationModel, causing duplicate model
construction per refresh cycle. Refactor buildCoordinationWorklist to accept the
pre-built coordination model as an optional parameter (e.g., coordinationModel?:
CoordinationModel) in the input object, and use that provided model instead of
calling buildCoordinationModel internally. Update the caller in
refreshNotificationEntries to pass the already-constructed model to avoid
redundant computation.
- Around line 260-263: The notificationBucket function incorrectly groups stale
items with missing-target items in the "unreachable" bucket, but stale
notifications are still reachable and should be treated differently to reflect
their distinct semantic state. Modify the conditional logic in
notificationBucket to separate the handling of missing items (where reachability
=== "missing") from stale items, so that only truly unreachable items return
"unreachable" and stale items are instead placed in the "handled" bucket or a
new separate bucket for stale notifications depending on the desired UI
behavior.
In `@src/multiplexer/notifications.ts`:
- Around line 191-197: The function markCoordinationItemRead in notifications.ts
is a duplicate of readNotificationItem from coordination.ts that performs
identical logic. Remove the markCoordinationItemRead function definition and
instead import readNotificationItem from coordination.ts, then update all call
sites in notifications.ts to use the imported readNotificationItem function
instead of the local markCoordinationItemRead function.
In `@src/multiplexer/subscreens.test.ts`:
- Around line 78-79: The worklist item in the coordinationWorklist array is
missing the key field that exists in the similar mock at line 40. Add the key
field to the worklist item object (the one with kind "thread" and thread
property) to match the WorklistItem contract structure, using a consistent
format similar to what appears on line 40 (e.g., key: "t:thread-1" or an
appropriate identifier based on the buildThreadEntries() result).
🪄 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: 98e021ca-08b4-474b-b205-7636a2379fb1
📒 Files selected for processing (9)
src/coordination-model.test.tssrc/coordination-model.tssrc/multiplexer/coordination.tssrc/multiplexer/index.tssrc/multiplexer/notifications.test.tssrc/multiplexer/notifications.tssrc/multiplexer/subscreens.test.tssrc/multiplexer/subscreens.tssrc/tui/screens/subscreen-renderers.ts
… build Review fixes: - A stale notice belongs to a LIVE agent that moved on, so bucket it as "handled", not "unreachable" (which is for vanished targets). - buildCoordinationWorklist accepts a prebuilt model; refreshNotificationEntries passes the one it already built, so the model is built once per refresh, not twice. - Drop the byte-for-byte duplicates in coordination.ts (readNotificationItem, findCoordinationTarget) and reuse the exported markCoordinationItemRead / findNotificationSessionTarget from notifications.ts. - Empty worklist clamps coordinationIndex to -1 (matching ensureNotificationState's no-selection sentinel) instead of 0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
Summary
Replaces the two-section Inbox/Threads Coordination screen with one agent-keyed, urgency-sorted worklist (the "Variant C" mockup), now that the information layer is reconciled. Presentation + interaction restructure; the data model was already reconciled in #171/#172.
3b1862d): new purebuildCoordinationWorklistmerges the reconciled notification model and genuine (non-notification) threads into one stream. Each row carries atype(msg/note/task/review/handoff/conversation) and abucket(needs-you/handled/unreachable) on a unified urgency scale, so notifications and threads interleave sensibly (live needs-input > on-you thread > pending thread > offline unread > handled > stale > missing). Threads are first-class rows (each appears once).7a12809): one selection index;Tabis now a filter (all / threads) rather than a section switch; per-row actions dispatch by item kind — notifications keepr/c/Enter(+ globalR/C), threads keeps/A/c/b/o/x/P/J/E/Enter.refreshNotificationEntriesbuilds the worklist so every mutation site stays coherent; the reply overlay'sthreadIndexis synced from the selected thread. Direct cutover — the Inbox/Threads split is gone.a7b5dbf): footer reflects the selected row's verbs; bucket boundaries are dashboard-style titled rules (Needs you ── N, accent); the detail pane is roundedcard()s (Inbox/Thread + Body/Messages).Test plan
yarn typecheck,yarn lint, fullyarn vitest(1465 passing),yarn build— all clean.0.1.21-tui-iafor live verification.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes