Settle divergence banners through islands instead of page morphs - #132
Conversation
A HEAD-divergence transition from the poller re-rendered the whole review page, re-hydrating every diff-file child (the 1+N cost the page avoids everywhere else) even when only banner chrome changed. The header marker and the missing-target bar now live in explicit islands that checkHeadDivergence refreshes under skipRender. The page still morphs when auto-follow rehydrates the file list, or when the list is empty: the divergence empty-state message lives outside the islands and that morph has no children to re-hydrate. Comment writes re-check divergence through a variant that leaves the render decision to the mutation's flags. That fixes two latent defects: a transition caught mid-write painted nothing (the poller then saw the state as already applied and stranded a stale banner), and the unchanged-divergence path latched skipRender over mutations that declared a render. The deleted/cleared/restored mutations now declare skipsRender: true, matching their tested and shipped behavior; their previous false flag was always overridden by that latch. Implements the one presently actionable proposal from #104. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G5BYb5cTR149b9ahJFYNTm
|
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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe divergence-transition flow in the review page trait now decides between a full page re-render and targeted "divergence island" updates. Comment mutation flags for delete/clear/restore switch to skip the parent render. Templates wrap divergence UI in Livewire islands, with documentation and tests updated accordingly. ChangesDivergence Islands and Render-Skip Flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ReviewPage
participant ReviewsBranchDivergence
participant DivergenceIslands
participant CommentMutation
CommentMutation->>ReviewPage: applyCommentMutation(mutation)
alt mutation.checksDivergence
ReviewPage->>ReviewsBranchDivergence: recheckDivergenceDuringCommentWrite()
ReviewsBranchDivergence->>ReviewsBranchDivergence: divergenceNeedsFullRender()
alt target branch changed or files empty
ReviewsBranchDivergence->>ReviewPage: forceRender()
else divergence-only change
ReviewsBranchDivergence->>DivergenceIslands: renderDivergenceIslands()
DivergenceIslands-->>ReviewPage: repaint divergence-marker, divergence-missing-bar
end
else
ReviewPage->>ReviewPage: apply mutation with skipsRender flag
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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: 39fe25b601
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The divergence marker and missing-target bar now live inside islands, where a wire:click scopes its render to that island. Their buttons (switch review, keep reviewing, dismiss) all settle more than the banner they were clicked in: switchReviewToHead rehydrates the file list, and keep/dismiss can swap the divergence empty-state message. The buttons now $dispatch bubbling rfa-* window events that the page-root Alpine forwards to $wire, matching the existing reviewed-state bridge pattern. Addresses the Codex review finding on PR #132. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G5BYb5cTR149b9ahJFYNTm
2 participants
Summary
Implements the one presently actionable proposal from #104: divergence chrome now settles through explicit Livewire islands instead of full-page morphs.
divergence-marker/divergence-missing-barislands thatcheckHeadDivergencerefreshes underskipRender(), mirroring the reviewed-state settle pattern.recheckDivergenceDuringCommentWrite(), which leaves the page-render decision to the mutation's flags. That fixes two latent defects:skipRendersuppressed it), and the poller then saw the state as already applied and never repainted, stranding a stale banner until an unrelated full render.skipRender()over mutations whose flag asked for a render, so the flag was never effective.ReviewCommentMutation::deleted()/cleared()/restored()now declareskipsRender: true, matching their tested and shipped behavior (theReviewPageTest"skips render" tests and the CLAUDE.md skipRender table); their previousfalseflag was always overridden by that latch, and no parent-rendered markup depends on comment state.Why the other five #104 proposals are not implemented
Each is conditional on a trigger that does not exist yet, or its concern is already covered:
applySelectionis already the server authority and rejects stalesnapshotKeys; expand-context/expand-gap have no batching or keyboard-repeat controls and Livewire serializes same-component calls; bulk copy-paths is already server-computed (copyVisiblePaths); the context-page comment bridge and the review-page comment burst queue are gated on comment controls moving into islands or burst-firing toolbars, which has not happened. Building those queues and coordinators now would add mechanisms nothing exercises.Validation
vendor/bin/pint --dirty --format agentcomposer test:types(PHPStan, clean)composer test— 1650/1653 pass; the 3 failures arePatchNativeServerTestvendored-NativePHP assertions that also fail on cleanmainin this environmentReviewPageBranchDivergenceTest: banner-only transition paints islands and skips the parent render, unchanged divergence paints nothing, auto-follow and empty-file-list transitions still morph the page, a transition caught during a comment write paints the islands, and comment deletion skips via its mutation flag🤖 Generated with Claude Code
https://claude.ai/code/session_01G5BYb5cTR149b9ahJFYNTm
Generated by Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation