Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 10 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughTodoProvider now publishes canonical events for local todo mutations, persists changes asynchronously, and suppresses only exact origin echoes. Bun integration tests cover mutation, scoping, delivery, and registration behavior. ChangesTodo observation flow
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
the
maintainer:e2e:ok
WalkthroughThis PR changes 4 file(s).
Changes
Magnitude🎯 1 (S) RelatedNo related items found. Walkthrough generated by LLxprt PR Review. Planner issue: #2256 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
OpenCodeReview — automatic reviews suspendedAutomatic OCR reviews are suspended for this PR after 2 of 2 automatic reviews. To get more reviews you can:
|
![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
🤖 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 `@packages/cli/src/ui/contexts/TodoProvider.tsx`:
- Around line 52-56: Update the event publication logic around
originPublicationRef and emitTodoUpdated to save the current marker before
assigning eventData, then restore that saved value in finally instead of
clearing the ref unconditionally. Add a regression test with a prepended
listener that synchronously calls the originating provider’s updateTodos,
asserting the nested list remains the final persisted state.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fa7cfb8a-d7d5-4f90-9d8b-6f55cca4dca5
⛔ Files ignored due to path filters (1)
project-plans/issue3052/plan.mdis excluded by!project-plans/**
📒 Files selected for processing (3)
packages/cli/src/ui/contexts/TodoProvider.tsxpackages/cli/src/ui/contexts/__tests__/todoProvider.observation.bun.tsxscripts/bun-test-manifest.ts
|
Follow-up on the nested-publication finding: CI proved the marker-stack fix preserved React state but the outer and nested fire-and-forget TodoStore writes could still finish out of order. Commit 546a5a9 now serializes persistence per provider while keeping updateTodos synchronous and starting the first write before publication. The deterministic regression holds the outer real write, proves the nested write is queued, releases the outer write, and verifies the nested list is final on disk. The focused Bun suite passes 26/26 repeatedly; lint, typecheck, format, build, and the stepfun-37 smoke test pass. |
1 participant
TLDR
Publish every provider-originated todo mutation on the canonical todo event channel so JSP/jefe, Zed, and peer providers stop retaining stale lists after slash-command edits such as clear.
The originating provider skips only its exact publication object, preventing self-reentry without hiding matching peer or synchronously nested external events.
Dive Deeper
The TodoWrite tool already persisted and emitted TodoUpdateEvent, but TodoProvider.updateTodos only updated React state and started disk persistence. Every slash-command mutation funnels through that provider method, so the UI and TodoRead were correct while external observers retained the last tool-written list indefinitely.
This change keeps updateTodos synchronous and optimistic:
A per-provider ref records the exact event object during synchronous emission. The origin listener skips only that object; external observers, matching peer providers, later external events, and synchronously nested external events remain authoritative. The ref is cleared in finally so a throwing listener cannot leave suppression armed.
Mount/session refresh reads deliberately do not publish. TodoStore.readTodos maps parse and I/O failures to an empty list, so the refresh path cannot distinguish an authoritative empty list from a failed read.
The Bun-native behavioral suite renders the real TodoProvider, drives real slash-command operations, and observes through createTodoObservationSubscription. It covers clear, every mutation family, empty replacements, session/agent identity, auto-clear, external-event filtering, peer-provider delivery, exact origin suppression, and nested synchronous events.
Open Code Review found one medium test-isolation issue: the future-dated archive used by the load test was not removed. The test now registers cleanup for it.
Reviewer Test Plan
Run the focused behavioral suite:
Confirm all 25 tests pass.
In an observed CLI session, create todos via TodoWrite, run /todo clear, and confirm the JSP consumer receives an empty todos.replaced transition.
Exercise /todo add, set, unset, remove, undo, and load and confirm each mutation emits one complete replacement.
Confirm a matching peer TodoProvider receives the update while the originating provider does not re-enter its own listener.
Full local verification performed on macOS:
Testing Matrix
Linked issues / bugs
Fixes #3052
Summary by CodeRabbit
Bug Fixes
Tests