fix(desktop): batch pi streaming and reuse transcript derivation - #98932
Conversation
|
React Doctor found 1 issue in 1 file · 1 warning. 1 warning
Reviewed by React Doctor for commit |
🤖 CI report✅ Trunk lane — non-backend lane (
|
e07e20c to
78a4127
Compare
🦔 PostHog Review reviewed this pull requestFound 0 must fix, 0 should fix, 4 consider. Published 4 findings (view the review). |
|
@greptileai review this draft |
Prompt To Fix All With AI### Issue 1
products/desktop/packages/ui/src/features/sessions/components/incrementalConversationItems.ts:63
**Missing explicit return type**
The new exported `createIncrementalAgentConversationBuilder` function relies on an inferred, nontrivial return type. This violates the repository directive that TypeScript functions use explicit return types. Please declare the builder contract before merging so this public API remains clear and stable.
```suggestion
export function createIncrementalAgentConversationBuilder(): ReturnType<
typeof createIncrementalConversationBuilder
> {
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix(desktop): batch pi streaming and reu..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
PostHog Review alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
f79f388 to
ea31a46
Compare
4256b22 to
4806be4
Compare
There was a problem hiding this comment.
Approved.
Internal performance refactor to the desktop app's Pi chat streaming/transcript-derivation logic — not risky territory (no auth, billing, data model, public API, or CI surface). Author is on the owning team with STRONG familiarity (76% of touched lines, 10/12 files previously modified, 28 merged PRs in these paths), the change ships with extensive new tests covering the batching, cancellation, and reconnect edge cases it introduces, and the only bot review turned up non-blocking "consider" items with a positive reaction, no unresolved substantive concerns.
- Author wrote 76% of the modified lines and has 28 merged PRs in these paths (familiarity STRONG).
- 👍 on the PR from greptile-apps[bot].
- 12 of the 12 changed files are governed by
products/desktop/AGENT_APPROVALS.md.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 352L, 8F substantive, 672L/12F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (672L, 12F, single-area, fix) |
| stamphog 2.0.0b4 | .stamphog/policy.yml @ ea31a46 · reviewed head ea31a46 |
4806be4 to
946640b
Compare
ea31a46 to
476ed1c
Compare
946640b to
3d4ae4b
Compare
476ed1c to
1344966
Compare
Both factories returned an inferred shape. Naming the interface gives the exported API an explicit return type. Generated-By: PostHog Desktop Task-Id: 3c052294-7a72-4a70-8963-8dd85db130bb
A source id only reaches session.events at flush, so batching text chunks opened a window where a redelivered chunk passed the dedup guard and ran applyTurnEvent again. If an abort landed between the two deliveries, that second pass overwrote the cancelled stop reason and re-set isStreaming, so a cancelled turn could notify as a normal completion. Track the source ids already buffered in the pending batch and reject a redelivery before it mutates turn state. Generated-By: PostHog Desktop Task-Id: 3c052294-7a72-4a70-8963-8dd85db130bb
3d4ae4b to
78e26fd
Compare
1344966 to
628c927
Compare
The PR was retargeted to a different base branch, so the approved diff is no longer what was reviewed. Stamphog re-reviews automatically.
|
😎 Merged successfully - details. |
There was a problem hiding this comment.
Approved.
Contained desktop streaming/UI change with no touch on auth, billing, migrations, public APIs, CI, or dependencies; author is on the owning team with STRONG familiarity (79% of touched lines, 29 merged PRs in these paths, last touch today), and the change ships substantial new tests covering batching, ordering, and cancellation. The only bot review comment is a minor style "consider" item, not a blocking concern.
- Author wrote 79% of the modified lines and has 29 merged PRs in these paths (familiarity STRONG).
- 👍 on the PR from greptile-apps[bot].
- 12 of the 12 changed files are governed by
products/desktop/AGENT_APPROVALS.md.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 339L, 8F substantive, 658L/12F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (658L, 12F, single-area, fix) |
| stamphog 2.0.0b4 | .stamphog/policy.yml @ 628c927 · reviewed head 628c927 |
HostHog preview —
|
Generated-By: PostHog Desktop Task-Id: 3c052294-7a72-4a70-8963-8dd85db130bb
1 participant
Problem
Long Pi chats repeatedly copy and process old messages as text arrives.
Changes
Batch streamed text and reuse completed message processing. Preserve event order and cancel buffered updates on disconnect. Layout unchanged.
Related: #76788 overlaps footer reuse; #92041 covers copy-text caching.
How did you test this code?
The full desktop suite and type checks pass on this PR alone.
Controller, builder, and hook tests cover batching, event order, and corrected messages. Production streaming latency remains unmeasured.
Automatic notifications
Docs update
Desktop performance guidance