Repository navigation
fix: reconcile after human message sends - #88
Conversation
|
CodeAnt AI is reviewing your PR. |
|
Warning Review limit reached
More reviews will be available in 4 minutes and 54 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (3)
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces tracking for human message send timestamps (lastHumanMessageSentAt) in the agent store and schedules message reconciliation when a human message is sent. It also adds corresponding unit tests. A review comment points out that asserting on the stringified source code of a React hook in the tests is brittle and recommends testing the hook's behavior instead.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| it('wires human message sends into the reconciliation hook', () => { | ||
| const source = hooks.useMessageReconciliation.toString() | ||
| expect(source).toContain('s.lastHumanMessageSentAt') | ||
| expect(source).toMatch(/scheduleHumanMessageSentReconciliation\([\s\S]*lastHumanMessageSentAt[\s\S]*reconciler/) | ||
| expect(source).toMatch(/\[lastHumanMessageSentAt,\s*reconciler\]/) | ||
| }) |
There was a problem hiding this comment.
Asserting on the stringified source code of a React hook (hooks.useMessageReconciliation.toString()) is highly brittle and considered an anti-pattern. This test will easily break if:
- The code is minified or bundled (where variable names like
sare mangled). - Code formatting changes (e.g., spacing, line breaks, or running Prettier).
- Internal variables or selectors are refactored (e.g., renaming
stostate).
Instead, you should test the hook's behavior. If @testing-library/react is available, you can use renderHook to verify that updating lastHumanMessageSentAt in the store triggers the reconciler's schedule function.
Review of #88 — Reconcile after human message sendsVerdict: APPROVE ✅ Tight, focused split-out of the R5 post-send trigger from PR #86. Diff is small (+60/-2, 3 files), wiring is clean, non-vacuity holds. What it doesCloses the "continuous typing in a focused window misses inbound messages" gap. PR #86's reconciliation triggers (focus/visibility/online/broker-status/event-stream-diagnostic) don't fire when the user is sitting in a focused Pear window typing message after message. With a half-open WS, inbound messages keep getting dropped. This PR adds a VerificationLocally on the PR branch (
Code qualityClean wiring
Edge cases I checked
Test 3 (
|
Summary
Follow-up to PR #86. PR #86 added bounded channel/DM reconciliation for missed broker events, but a continuously focused Pear window can still miss inbound messages while the user keeps typing because focus, visibility, online, and broker-status triggers do not fire.
This PR adds a post-send reconciliation trigger by tracking optimistic human sends in the renderer store and scheduling
human-message-sentreconciliation fromuseMessageReconciliation. It does not add IPC and does not touch ComposeBar.Non-vacuity
Removing the new
lastHumanMessageSentAthook effect makes the new hook wiring test go RED. The test also verifies the store timestamp advances whenaddHumanMessageperforms the optimistic insert.Validation
npx vitest run src/renderer/src/hooks/use-message-reconciliation.test.tsnpx vitest run src/main/broker.test.ts src/renderer/src/hooks/use-message-reconciliation.test.tsnpm testnpm run buildgit diff --check HEAD~1..HEAD