Fix side chat activation latency - #1457
Merged
Merged
Conversation
createSideChat only reads the source thread's last conversation message (the reply-seed rule), but requested the timeline with no segmentLimit — the default 20 segments of fully nested rows, projected synchronously on the activation path. Ask for segmentLimit=1. Every timeline segment is anchored at a user message, so the newest segment always holds the newest conversation row; the seed answer is unchanged. Measured on a synthetic 40-turn x 30-item thread via buildThreadTimeline: 620 rows / 455 KB / 8.6 ms at the default limit vs 31 rows / 22.8 KB / 1.2 ms at segmentLimit=1, identical last-conversation-message result. Drops the measurement scaffolding the original change carried (panel and fork timing logs, the try/finally restructure, the panelHydrationStarts map — which also leaked an entry whenever a panel never mounted, since the launcher path's openPanel returns void and never hit the cleanup).
Collaborator
|
Commandeered this to reproduce the problem and verify the fix. The one-line Reproduced the cost. Measured Confirmed why it's safe. Every logical segment is anchored at a user message ( Removed the measurement scaffolding — the three debug timing logs, the One of those lines was also a small bug worth recording: Also merged current Net:
|
ymichael
added a commit
that referenced
this pull request
Aug 18, 2026
## What was wrong
The plugin SDK had four ways to ask the host to open a panel, and they
disagreed on how to report failure.
`PluginMessageActionContext.openPanel` and
`useBbNavigate().openThreadPanel` returned `boolean`;
`PluginThreadPanelActionContext.openPanel` and
`PluginNewThreadPanelActionContext.openPanel` returned `void`. Invalid
`params` compounded it — that threw out of `openPanel` on the
panel-action path but returned `false` on the message-action path, so
the same mistake surfaced two different ways.
Those four are really two operations, each existing twice for surface
reasons: *open the panel I already am* (no `actionId` — you are inside a
panel action's `run`), available on the thread panel and the New thread
screen; and *open a panel I am not* (`actionId` required), available as
a callback context and as a hook. Nothing about that split required the
return types to diverge — they diverged because each was declared
independently.
The inconsistency also leaked into plugin code.
`plugins/side-chat/app.tsx` shares one helper across two of those entry
points, so it typed the injected `openPanel` as returning `unknown` —
the only type that fits both a boolean and nothing — discarding a result
it had no way to describe.
This PR does **not** fix a user-facing bug. An earlier revision added
handling for a declined open in side chat; slopcop correctly pointed out
that path is unreachable, and it has been removed. `PluginThreadChat`
passes `includePluginMessageActions={false}`, and `ThreadTimelineRows`
swaps in an empty slot snapshot when that is false, so a plugin
`messageAction` never renders inside a plugin's own `ThreadChat`. Every
surface that does render them supplies `onOpenPluginPanel`. This is a
contract cleanup.
## What changed
`packages/plugin-sdk/src/app-contract.ts` — both panel-action
`openPanel`s now return `boolean`, matching the other two. The doc
comments state one rule: `true` accepted, `false` declined, never a
throw. The two operations now share types that name them:
`PluginPanelActionOpenOptions` (`{ title?, params? }`) for a panel
action opening its own tab, and `PluginTargetedPanelActionOpenOptions`,
which extends it with `actionId`, for a caller that is not itself a
panel action. `useBbNavigate().openThreadPanel` previously inlined a
shape identical to the latter and now uses it.
`apps/app/src/components/plugin/PluginPanelActions.tsx` — both
panel-action closures go through one `createPanelActionOpenPanel` helper
that catches non-JSON `params`, warns, and returns `false` rather than
throwing into `run`, where the host swallows it.
`apps/app/src/lib/plugin-message-actions.ts` — the no-side-panel branch
returned `false` silently and now warns, so all three decline paths are
diagnosable from the console.
`plugins/side-chat/app.tsx` — the injected `openPanel` is typed
`boolean` instead of `unknown`, removing the cast that existed only to
bridge the two old signatures. Nothing reads the result, and a comment
records why.
Generated files were regenerated by script, not hand-edited:
`bundled-types/` via `scripts/build-bundled-dts.mjs`, and
`@bb/templates`'s embedded copy of the SDK `.d.ts` via
`generate-templates.mjs`. That second one matters — its `--check` runs
inside `@bb/templates`'s `typecheck` and `test`, and would otherwise
fail CI. Docs updated in `packages/plugin-sdk/README.md`,
`docs/api_to_audit.md`, and the `bb-plugin-authoring` skill. No wire
changes, so `HOST_DAEMON_PROTOCOL_VERSION` is unchanged.
What this deliberately does not do is reduce the count or unify the
names — `openPanel` and `openThreadPanel` remain two names for one idea.
That is a rename across the public API and deserves its own decision.
### Breaking changes for plugin authors
**1.** `run` is declared `void | Promise<void>`, and `boolean` is not
assignable to that union — TypeScript's void-return exemption applies
only to a bare `void`. So the concise form `run: ({ openPanel }) =>
openPanel({ ... })` no longer typechecks and needs braces:
```ts
run: ({ openPanel }) => { openPanel({ ... }); }
```
Three in-repo call sites and both examples in the authoring skill are
fixed. We deliberately did not widen `run`'s return type to absorb this:
pre-1.0, a loud one-line compile error is an acceptable cost for keeping
the type honest.
**2.** `PluginMessageActionThreadPanelOptions` is renamed
`PluginTargetedPanelActionOpenOptions`, since it now describes both of
its callers rather than one.
## How you verified
- New: an accepted open reports `true` from both panel-action kinds
(`plugin-slot-mounts.test.tsx`).
- Extended: the existing non-JSON-`params` test now asserts `false` is
returned rather than thrown.
- Already covered: `ThreadTimelineRows.actions.test.tsx:1423` pins
`messageAction.openPanel` → `false` on a surface with no thread panel.
- `pnpm exec turbo run typecheck test lint --filter=@get-bb/plugin-sdk
--filter=@bb/templates --filter=bb-plugin-side-chat --filter=@bb/app` —
3,041 tests passed, typecheck and lint clean.
- `generate-templates.mjs --check` verified failing on this branch
before the regeneration and passing on `main`, confirming the staleness
was introduced here rather than pre-existing.
Rebased on `main` after #1851, which made the declaration bundles
deterministic; the generated-file conflicts were resolved by
regenerating from source rather than by hand.
No issue was filed for this; it was found while reviewing #1457.
> AGENT GENERATED: by Claude Opus 5
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was wrong
createSideChatreads exactly one thing from the source thread's timeline: its last conversation message, which the reply-seed rule compares against the anchor (resolveReplySeedText→lastConversationMessageText). It requested that timeline with nosegmentLimit, so it got the route default ofTHREAD_TIMELINE_DEFAULT_SEGMENT_LIMIT = 20segments of fully nested rows. On a long thread that is a large synchronous projection sitting directly on the side-chat activation path, before the fork can be persisted and the panel can open.No issue was filed for this; it was found while profiling side-chat activation.
What changed
plugins/side-chat/server.ts— the reply-seed timeline lookup now passessegmentLimit: "1".That is sound because of an invariant on the server side:
isTimelineSegmentAnchorRowanchors every logical segment at a user message, andpaginateTimelineRowstakessegments.slice(-segmentLimit)— the newest segments. So the newest segment always contains the thread's newest conversation row, user or assistant, and the seed answer is unchanged.segmentLimitalso bounds the underlying event read viaresolveTimelineWindowBoundsrather than only slicing the projected result, so the work is genuinely avoided and not merely discarded.No wire changes, no CLI or settings surface, no
HOST_DAEMON_PROTOCOL_VERSIONbump needed.How you verified
plugins/side-chat/server.test.tsgains a test pinning the request shape — it fails if thesegmentLimitargument is dropped or changed.To confirm the projection cost was real rather than assumed, I measured
buildThreadTimelinedirectly against an in-memory SQLite thread of 40 turns × 30 items:segmentLimitlastConversationMessageTextreturned the identical string in both cases, at that size and at 25 turns × 12 items. That benchmark was scaffolding for this investigation and is not part of the diff.pnpm exec turbo run test typecheck lint --filter=bb-plugin-side-chat— 29 tests passed, typecheck and lint cleanpnpm exec turbo run test --filter=@bb/server— 1725/1726 passed. The single failure,plugin-service.test.ts > reports one anonymous plugin_installed event per user install, is a 5s-timeout flake under full-suite load: it passes in isolation and touches nothing in this change.Fixes #1835
BB-Thread-ID: thr_6zqwdhihs2