CPLAT-12461: stop dropped commands stranding scratchpad collection - #171
Open
gavin-jeong wants to merge 1 commit into
Open
gavin-jeong wants to merge 1 commit into
gavin-jeong wants to merge 1 commit into
Conversation
dayScratchpadCmds marks a session in flight before returning its command,
and six call sites discarded that return value — the View path, which
structurally cannot dispatch a tea.Cmd, plus the tab-switch and query
handlers, which returned nil.
A discarded command left the session marked as "already walking" against
work that never ran: neither collected nor eligible for re-dispatch, so
its scratchpad was missing permanently. Switching tabs was enough to
trigger it, which is what the report showed.
refreshOutputsPreviewLayout already documents this exact failure for the
outputs digest. The new code did not follow it.
Fixed in three layers, because any one alone leaves a sharp edge:
- propagate: setDayOutputTabKind, cycleDayOutputTab, selectDayOutputTab,
applyDayOutputQuery and clearDayOutputSearch were void and now return
tea.Cmd, and every caller passes it on
- a non-dispatching form for render paths: refreshDayPreviewLayout and
refreshSessionPreviewLayout re-render without arming anything, the
same shape as refreshOutputsPreviewLayout
- a TTL on the in-flight mark, so a dropped command costs a retry after
10s rather than a permanent hole. Marking work in flight before it
reaches the runtime cannot be made safe by discipline alone
Verifying that turned up a second bug: the fan-out cap was applied per
call rather than over the total in flight. Every completion re-renders
the pane, so each message started a fresh batch of 8 and 250 sessions
reached 218 walks running at once. The budget now covers the running
set, with stale marks expired first. Measured after: 250 sessions
collect completely, in flight never exceeds 8, cursor movement is 12µs
against a 16ms frame budget.
TestDayScratchpadStaysBoundedAndCompletes dispatches repeatedly WITHOUT
completing anything, which is how the overrun actually accumulates — a
single-call check passes with the bug present, which the first version
of this test did.
TestDayCursorMoveCost builds sessions with no scratchpad, so it never
exercised this path; TestDayCursorMoveCostWithScratchpad covers it.
TestDayPaneScratchpadCollectionIsBounded asserted that a later render
dispatches the next batch. That is now wrong: re-rendering is not
evidence that anything finished. It asserts the real contract instead —
nothing new while the batch runs, progress once one completes.
|
| Commit | Scanned at | New | Resolved | Net |
|---|---|---|---|---|
db1e2b6 < |
2026-09-29 10:00 UTC | 0 | 0 | 0 |
Last scanned: db1e2b6 · 2026-09-29 10:00 UTC
|
| Commit | Scanned at | New | Resolved | Net |
|---|---|---|---|---|
db1e2b6 < |
2026-09-29 10:00 UTC | 0 | 0 | 0 |
Last scanned: db1e2b6 · 2026-09-29 10:00 UTC
Kairo-Kim
approved these changes
Sep 29, 2026
Kairo-Kim
added
the
auto-review/approved
9 tasks done
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
4 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.
JIRA: https://sendbird.atlassian.net/browse/CPLAT-12461
Follow-up to #170, reported as "scratchpad doesn't show in the day pane" — with a screenshot of the pane sitting on the
Planstab.The bug
dayScratchpadCmdsmarks a session in-flight before returning its command, and six call sites discarded that return value — the View path, which structurally cannot dispatch atea.Cmd, plus the tab-switch and query handlers, which returnednil.A discarded command left the session marked as "already walking" against work that never ran. It was then neither collected nor eligible for re-dispatch: its scratchpad was missing permanently. Switching tabs was enough to trigger it, which is exactly what the screenshot shows.
refreshOutputsPreviewLayoutalready documents this precise failure for the outputs digest. The new code did not follow that pattern — my miss in #170.Fix, in three layers
Any one alone leaves a sharp edge:
setDayOutputTabKind,cycleDayOutputTab,selectDayOutputTab,applyDayOutputQueryandclearDayOutputSearchwerevoidand now returntea.Cmd; every caller passes it on.refreshDayPreviewLayout/refreshSessionPreviewLayoutre-render without arming anything — the same shape asrefreshOutputsPreviewLayout.A second bug, found while verifying the first
The fan-out cap was applied per call, not over the total in flight. Every completion re-renders the pane, so each message started a fresh batch of 8 — 250 sessions reached 218 walks running at once. The budget now covers the running set, with stale marks expired first.
Measured after the fix: 250 sessions collect completely, in-flight never exceeds 8, cursor movement is 12µs against a 16ms frame budget.
Test plan
go build ./... && go vet ./... && go test ./...greenUpdateloop, before any changeTestDayScratchpadStaysBoundedAndCompletes— dispatches repeatedly without completing anything, which is how the overrun actually accumulates. My first version of this test checked a single call and passed with the bug present; it now fails on it (verified by reverting).TestDayCursorMoveCostWithScratchpad— the existingTestDayCursorMoveCostbuilds sessions with no scratchpad, so it never exercised this path at all.mitm-proxy, 16 rows) end to end.One existing test's assertion is now wrong
TestDayPaneScratchpadCollectionIsBoundedasserted that a later render dispatches the next batch. That was my own wording in #170 and it is incorrect: re-rendering is not evidence that anything finished, and dispatching there is the overrun. It now asserts the real contract — nothing new while the batch is running, progress once one completes.Security checklist
0.0.0.0/0inbound