Conversation
`addToClineMessages` called `flushPostStateToWebviewThrottled()` for every
message with `partial === true`. With lodash debounce configured
`{ leading: true, trailing: true }`, a flush issued right after a leading-edge
invocation has no pending trailing invocation to run, so it only cancels the
trailing timer. The next call then hits the leading edge again and posts
immediately, so the debounce never coalesced anything on the streaming path.
Measured against lodash.debounce 4.0.8 with the provider's own settings
(500ms wait, 1000ms maxWait), 20 messages arriving 100ms apart produced:
with the interleaved flush: 20 full-state posts
without it: 3 full-state posts
Because `requiresImmediateState` matched every partial first chunk, throttling
was effectively inert for tool-heavy tasks and the gray-screen OOM that Zoo-Code-Org#1078
set out to fix remained reproducible at high message counts.
Drop `partial` from the condition. Partial `say` messages are safe on the
throttled path: the trailing/maxWait post carries the message's current text,
so a `messageUpdated` dropped for a `ts` the webview does not know yet is
superseded rather than lost. Partial asks are unaffected — `Task#ask` adds them
without `isAnswered`, so they keep flushing through the ask clause and retain
the ordering guarantee unanswered asks depend on.
The existing suite could not catch this: every Task-level test mocks the
provider, so it asserted that flush was *called* rather than that state posts
were actually coalesced. Retarget the partial-message test at the new contract
and add a characterization test in ClineProvider.spec.ts that pins the
debounce semantics against the real timer.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: Zoo-Code-Org/Zoo-Code/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: Zoo-Code-Org/Zoo-Code/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📜 Recent review details🧰 Additional context used📓 Path-based instructions (6)Check persistence and lifecycle invariants: awaited atomic writes, rollback or explicit partial-failure behavior, cross-window state consistency, stale listeners/watchers, cancellation, idempotency, and safe restart/resume without lost or d...⚙️ CodeRabbit configuration file Files:
For persisted settings, verify the complete schema/storage/runtime/webview round trip, shared default semantics, and focused true plus false/unset tests.⚙️ CodeRabbit configuration file Files:
Require regression coverage at the lowest valid harness with behavior-focused assertions, including relevant negative, error, false/unset, and boundary cases.⚙️ CodeRabbit configuration file Files:
Check strict typing and exhaustive behavior across normal, boundary, error, cancellation, retry, and compatibility paths.⚙️ CodeRabbit configuration file Files:
Verify extension/webview contracts, cancellation and error propagation, VS Code lifecycle correctness, and behavior under retries and partial failure.⚙️ CodeRabbit configuration file Files:
Act as an adversarial second-opinion reviewer.⚙️ CodeRabbit configuration file Files:
🔇 Additional comments (3)
📝 SummarySummary by CodeRabbit
WalkthroughPartial messages now use throttled state posting instead of triggering an immediate flush. Unanswered asks still trigger an immediate flush. Tests check partial-message updates and how flushes affect throttled posts. ChangesPartial message state throttling
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to Partial-message state updates are throttled while unanswered asks retain immediate flushing. No concrete merge-blocking regression is established; the change appears ready to merge subject to normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to Streaming messages now use coalesced interface updates, while unanswered prompts still update immediately. No new access path or weakened security control was identified. Delivery after a failed interface post remains uncertain. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 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 |
Review statusThanks for contributing. This comment tracks the review sequence and the next action. Current step: Wait for GitHub to finish calculating mergeability. Review-state labels are managed by this workflow; do not edit them manually. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
github-actions
Bot
added
coderabbit-review-active
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
github-actions
Bot
added
awaiting-maintainer
github-actions
Bot
added
awaiting-maintainer
2 participants
Related GitHub Issue
Closes: #1851
Description
The webview state debounce (led by #1078) was effectively inert on the streaming path:
addToClineMessagescalledflushPostStateToWebviewThrottled()for every partial (partial === true) message. With lodash debounce configured as{ leading: true, trailing: true }, a flush issued right after a leading-edge invocation has no pending trailing invocation to run, so it only cancels the trailing timer — which makes the next call hit the leading edge and post immediately. Result: one full-state post per message, defeating the debounce entirely.This PR drops
partialfrom the immediate-state condition. Partialsaymessages are safe on the throttled path: the trailing/maxWait post carries the message’s current text, so amessageUpdateddropped for atsthe webview does not know yet is superseded rather than lost. Partial asks are unaffected —Task#askadds them withoutisAnswered, so they keep flushing through the ask clause and retain the ordering guarantee unanswered asks depend on.Against lodash.debounce 4.0.8 with the provider’s own settings (500ms wait, 1000ms maxWait), 20 messages arriving 100ms apart produced 20 full-state posts with the interleaved flush vs 3 without it.
The existing Task-level suite mocked the provider, so it asserted flush was called rather than that posts were actually coalesced. Retargeted the partial-message test at the new contract and added a characterization test in
ClineProvider.spec.tsthat pins the debounce semantics against the real timer.Test Procedure
pnpm --dir src exec tsc --noEmit(pre-push hook runs fullturbo check-types).cd src && npx vitest run core/task/__tests__/Task.spec.tscd src && npx vitest run core/webview/__tests__/ClineProvider.spec.tsPre-Submission Checklist
Visual Snapshots
N/A (no user-visible rendered state change; behavioral bug fix).
Videos (interaction / animation only)
N/A.
Documentation Updates
Additional Notes
None.
Get in Touch
hehegwk_23849