Repository navigation
perf(staged): speed up web session load by shrinking, windowing and deferring transcript payloads - #974
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd73ab133e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const [s, msgsResult, acpMsgs, queued] = await Promise.all([ | ||
| getSession(sessionId), | ||
| getSessionMessages(sessionId), | ||
| getLatestSessionMessages(sessionId, pageRequestLimit(TRANSCRIPT_PAGE_ROWS)), |
There was a problem hiding this comment.
For sessions with more than 300 visible rows, this loads only the newest page, but performSearch still searches only the rendered messagesEl and does not fetch older pages. A query whose matches are exclusively in older history therefore reports zero results and cannot navigate to them until the user manually scrolls back far enough to load those rows; load the remaining history when search begins or add a server-side transcript search.
Useful? React with 👍 / 👎.
Comment on lines
+46
to
+48
| export function transcriptPageFromRows(rows: SessionMessage[], pageRows: number): TranscriptPage { | ||
| if (rows.length > pageRows) { | ||
| return { messages: rows.slice(rows.length - pageRows), hasMore: true }; |
There was a problem hiding this comment.
When the row-count boundary falls between a tool_call and its matching tool_result, this slice drops the call while retaining the result. buildAcpTranscriptGroups then has no matching tool assembly and renders the result as a synthetic result-only tool card at the top of the window, with an incorrect header/details until the preceding page loads—and permanently if that request fails. Extend or align the page boundary so paired tool rows are delivered together.
Useful? React with 👍 / 👎.
Opening a session in the browser downloaded the whole transcript twice, uncompressed, over HTTP/1.1 (26 MB for the largest local session). This lands the five server-only quick wins from the investigation; no UI change. 1. CompressionLayer (brotli + gzip) on the composed Axum router. The router is added here as `app_router` so the mobile-web startup restore can serve it as-is; `start()` stays stubbed on this branch. tower-http's brotli default is already q4, which took ~35 ms for a 13.5 MB body in the offline measurement (10x on text, 3x on base64 images). The default predicate skips images and sub-32-byte bodies, so WebSocket upgrades and /api/media are untouched. 2. `get_session_acp_metadata_messages` no longer returns rows that `get_session_messages` already ships. The client merges the two and dedups by id (acpTranscript.ts `sortedUniqueMessages`), so this is safe. Visible tool rows that are still in flight are deliberately kept: the writer mutates them in place and the poll builds its refetch ids from `acpMetadataMessages`, so dropping them would have frozen tool statuses at in_progress. The refetch-id half of the `_since` query bypasses the filter so a settled row is delivered once and then stops being requested. 3. Hidden rows are restricted to the kinds the client reads: tool_call, tool_call_update, plan_update, available_commands_update, config_options_update, pikchr_session_started. agent_message_chunk, user_message_chunk, usage_update, prompt_response, response_result, session_info_update, session_mode_state, current_mode_update and initialize (served by get_session_acp_initialization) stay in the table for replay but are not fetched. A new client consumer of a hidden kind must be added to `TRANSCRIPT_METADATA_FILTER`. 4. ALPN ["h2", "http/1.1"] on the rustls ServerConfig, plus axum's `http2` feature, which is not on by default and without which hyper would refuse the negotiated protocol. 5. The session, messages, ACP metadata and queued-message arms of the invoke handler run their store read and JSON serialization under spawn_blocking instead of on the tokio worker that also services /api/events. Measured with the new filter against the two profiled local sessions (payload bytes, read-only SQL): - image-heavy 82598983: ACP response 1,453 rows / 12.5 MB -> 5 rows / 0.08 MB; bytes on open 24.9 MB -> 12.4 MB before compression. - text-heavy 43d5c5b3: ACP response 5,491 rows / 2.5 MB -> 4 rows / 0.04 MB; bytes on open 4.6 MB -> 2.2 MB before compression. With brotli on top the image-heavy session should land near 4 MB. Rust-side tests that assert on the replay archive (agent_message_chunk, session_info_update rows) use a new `#[cfg(test)]` `get_session_acp_metadata_archive` reader; production callers (config lookup in session_commands, pikchr announcements) only need allowlisted kinds and are unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
… base64
Even after the previous round of payload cuts, opening the image-heavy
local session still shipped ~11 MB of base64 screenshots on every load:
each `read` of an image file stores the bytes twice, once in
`acp_raw_output` (Claude's `{"type":"image","source":{"type":"base64",
"data":…}}`) and once in `acp_content` (ACP's `{"type":"content",
"content":{"type":"image","data":…,"mimeType":…}}`), and the client only
looks at either once a tool card is expanded. This moves the bytes out of
the transcript payload and fetches them on demand. Read-side projection
only: no schema or ingest change, the table keeps the base64.
Ref format. The four transcript queries (`get_session_messages`, its
`_since` variant, `get_session_acp_metadata_messages` and its `_since`
variant) now run rows through `transcript_message_from_row`, which
rewrites every base64 image block in both columns into
{"type":"image","ref":"<message_id>/<output|content>/<index>",
"mimeType":"image/png"}
keeping the block's other non-payload keys (e.g. `annotations`) and the
ACP `content` envelope. `<field>` names the column and `<index>` the
block's position in the result's block list (the value itself, or its
`content` array for MCP results), mirroring the traversal the client's
`toolCallImageContent` already does so exactly the blocks it would have
rendered inline are projected. The logic lives in the new
`store/tool_images.rs`. `get_session_replay_messages`,
`get_session_acp_metadata_archive` and `get_session_acp_initialization`
keep the raw row, so resume/replay and the existing writer tests are
unaffected; `Store::get_tool_image(message_id, field, index)` decodes a
single block for the serving paths.
Route. `GET /api/tool-images/{message_id}/{field}/{index}` joins the
authenticated media router next to `/api/media`, runs the store read under
`spawn_blocking`, and answers with the block's `Content-Type`,
`Cache-Control: private, max-age=31536000, immutable` (a settled tool
result never changes and id/index is a stable identity) and `nosniff`.
Anything that does not resolve to a base64 image block — unknown row, bad
field, index out of range, a text block, an unparseable id — is a 404.
tower-http's `DefaultPredicate` includes `NotForContentType::IMAGES`, so
the compression layer leaves these bodies alone.
Tauri. The desktop build has no HTTP server, so a `get_tool_image`
command (and a matching `/api/invoke` arm) returns the same block as a
data URL, following `get_image_data`.
Client. `toolCallImages.ts` gains a `ref` image kind and
`parseToolImageRef`; the inline base64 branch stays so payloads cached in
IndexedDB before this change, legacy `tool_result` JSON and any producer
that still embeds bytes render as before. `toolImageSource.ts` resolves a
`ref` to `/api/tool-images/<ref>` on the web, handed straight to
`<img loading="lazy">` so the browser decides when to download, and to the
`get_tool_image` command under Tauri. `ToolResultImage.svelte` only mounts
when a card is expanded (unchanged), and now gates IPC-backed loads (Tauri
refs, `read_image_file` paths) behind an IntersectionObserver with a
one-viewport margin so expanding a card with twenty screenshots does not
decode all twenty at once.
Measured on the image-heavy local session 82598983 with the same
read-only SQL approach as before (sum of the shipped columns over the rows
the visible + metadata queries select, plus per-row envelope): 436 rows,
12.63 MB before this change (the previous commit estimated 12.4 MB),
1.44 MB after. The 38 projected blocks (19 rows × 2 columns) accounted
for 11.20 MB.
Verification: `cargo check --tests`, `cargo clippy -- -D warnings` and
`cargo fmt --check` clean; `cargo test --lib --bins --tests` 1039 passed;
`pnpm run check` 0 errors / 0 warnings over 3416 files; `vitest run`
1280 passed across 90 files (including the new store projection, route and
client ref tests); prettier clean.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
…roll-up
Opening a session fetched every visible row of the transcript. After the
previous two commits that was still 1.3 MB (image-heavy local session,
431 rows) and 1.9 MB (text-heavy, 771 rows) of JSON before compression,
all parsed and grouped before the first paint. The pane now opens with
the newest page only and fetches older pages as the user scrolls towards
the top. Desktop goes through the same Tauri command with the same
optional arguments.
Server. `Store::get_session_messages_window(session_id,
TranscriptWindow { limit, before_id })` is the one query;
`get_session_messages` is now `TranscriptWindow::ALL`, and with neither
parameter set the SQL and result are what they were (pinned by test).
With `limit` the newest rows are read `ORDER BY id DESC LIMIT ?` and
reversed in Rust: a subquery with an outer `ORDER BY id ASC` plans a
temp B-tree, while the index already yields rowid order either way.
`before_id` adds `AND id < ?`. EXPLAIN QUERY PLAN reports `SEARCH
session_messages USING INDEX idx_session_messages_session (session_id=?
AND rowid<?)` and no temp B-tree for every combination;
`test_session_messages_window_query_plan` asserts that against the
statement the store actually runs. The `get_session_messages` Tauri
command and the web invoke arm take `limit` / `beforeId` as optionals;
the web arm still reads and serializes under `blocking_store_json`
(`spawn_blocking`), and a source-level test keeps it that way. No schema
change.
Page size: 300 visible rows (`TRANSCRIPT_PAGE_ROWS`), requested as 301
so the client learns whether older rows exist from the count and drops
the extra row. A group count cannot be the server parameter and would
not bound the payload anyway: the text-heavy session's 771 rows are 8
rendered groups, because consecutive tool calls collapse into one. On
the image-heavy session (~7 rows per group) the last 50 groups span 294
rows, so 300 is "about 50 groups" where the transcript alternates prose
and tool runs, and keeps the opening response under 0.8 MB.
Client. `getLatestSessionMessages(sessionId, limit)` goes through the
IndexedDB SWR cache under a key that includes `limit`, so a window is
never served as the full transcript or vice versa (`getSessionMessages`
is unchanged for `liveSessionHints`); the completion invalidation now
matches on `sessionId` alone (`invalidateCacheByArgs`) so both keys are
dropped. `getOlderSessionMessages(sessionId, limit, beforeId)` is
uncached: history is immutable and a page is requested once per open.
The list handling lives in the new `transcriptWindow.ts`, unit-tested:
`transcriptPageFromRows` (over-fetch -> `hasMore`),
`prependOlderMessages` (dedup against the loaded start),
`applyLatestWindow` (a revalidated window replaces the cached one but
keeps older pages already loaded below it and rows the poll appended
above it) and `metadataRowsForWindow` (below). In the pane,
`handleScroll` asks for the next page within 600 px of the top;
`loadOlderMessages` runs one request at a time, bails if the session or
the window changed under it, prepends with intro transitions
suppressed, and restores the scroll position by the first rendered
group's offset (falling back to the container's growth when the keyed
each re-created that group), with `overflow-anchor: none` so the
browser does not correct it a second time. A fixed-height slot above
the transcript shows the spinner so its appearance does not shift the
rows. A first page shorter than the viewport tops itself up until it
overflows. The 500 ms poll's `_since` path is untouched: it keys off the
newest loaded row, which windowing does not change. Its
empty-transcript branch now bypasses the SWR cache, which otherwise
handed back the empty list the pane had just stored for the whole TTL.
Metadata rows at the boundary. `get_session_acp_metadata_messages` is
still fetched whole (4-5 rows on the profiled sessions) and is not
windowed. Rows older than the first loaded visible row would group as
orphan tool cards at the top of the window - a hidden `tool_call` or an
in-flight visible tool row whose call is above the window, stuck "in
progress" - or, for `pikchr_session_started`, pair with the wrong tool.
`metadataRowsForWindow` drops them for the grouping while more history
exists, except tool rows whose call is loaded (a hidden row that raced
ahead of its visible row; 41 of the 45 hidden tool rows in the local
database are of that kind), which still carry that tool's input and
status. `latestPlan`, `latestAvailableCommands` and
`latestAcpConfigDiscoveryFromMetadata` keep reading the unfiltered rows.
Deriveds that read `messages`, audited against a windowed list:
- `grouped` / `lastGroupIndex`: windowed by design; "last group" is the
newest row, always loaded.
- `openAssistantRefetchIds(messages)` (poll): newest rows only. (a)
- `noteFollowupLabel` -> `shouldAskForNoteUpdate`:
`latestAssistantMessage` is the newest assistant row (loaded);
`hasNoteFollowupBeenSent` looks for a user row newer than the note's
`updatedAt`, normally near the tail. Only a follow-up sent more than a
page ago that the agent never folded into the note would be missed,
re-offering the CTA. (c)
- `assistantMarkdownContent` / `sessionHasPikchr` /
`pikchrRendererLoadKey`: only decide whether to load the renderer for
rows being rendered; an older pikchr fence loads it when its page
arrives. (c)
- `noteBearingMessageIds[0]` ("Note created" vs "Note updated"): the
oldest loaded note message may read "created" while an earlier one is
unloaded; corrects itself when that page loads. (c)
- message image preload effect: loaded rows only, runs per page. (a)
- `grouped.length === 0` empty states: the newest page is empty only if
the transcript is. (a)
- transcript search (`highlightMatches`): DOM-based, so loaded rows
only, as a windowed list implies. (c)
No signal needed moving to the server.
Measured by serializing the store's actual output against a read-only
`.backup` of the local database (the same rows the invoke arm ships,
tool images already projected to refs):
- image-heavy 82598983: full 431 rows / 1,291,210 B; opening page
300 rows / 771,363 B (301-row request 774,077 B). 200 rows would be
457,574 B, 400 rows 1,154,633 B. Metadata response 5 rows / 80,232 B.
- text-heavy 43d5c5b3: full 771 rows / 1,907,477 B; opening page
300 rows / 689,426 B (request 690,686 B). 200 rows 430,292 B,
400 rows 898,099 B. Metadata 4 rows / 44,165 B.
Verification: `cargo check --tests`, `cargo clippy -- -D warnings` and
`cargo fmt --check` clean; `cargo test --lib -- store:: web_server::
session_commands::` 334 passed (5 new store tests, 1 new web_server
test); `pnpm run check` 0 errors / 0 warnings over 3418 files; `vitest
run` 1292 passed across 91 files (12 new in
`transcriptWindow.test.ts`); prettier clean on the changed files.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Item 8 of the slow-web-session-load note. A settled tool row carries up to three JSON bodies -- acp_raw_input, acp_raw_output and acp_content -- that the UI only renders once the card is expanded, yet across a long session they are most of what the transcript queries ship. The store now replaces each body over 2 KB with a marker and the client fetches the row's bodies when the card opens, following the read-side pattern of 0266a8d and 585425f: no schema or ingest change, a dedicated web route plus a matching Tauri command, and a client that still renders legacy full-body payloads (IndexedDB-cached sessions) unchanged. Store - New store/tool_bodies.rs. transcript_message_from_row runs project_tool_images and then project_tool_bodies, so the size judged is the size that would ship (a 4 KB base64 image row becomes a small ref and stays inline). Replay, archive and initialization readers still see raw rows. - A body is deferred only when the row's acp_tool_status is terminal (the same list TRANSCRIPT_METADATA_FILTER uses) and its serialized size exceeds DEFERRED_TOOL_BODY_MIN_BYTES = 2048. In-flight rows ship whole, so the writer's in-place mutation, the poll's _since reads and the metadata refetch are untouched. Deferred => settled => immutable, which is what lets the body route be cached as such. - Marker: {"$deferred":{"messageId","field":"input|output|content", "bytes", "innerSessionId"?, "pikchr"?}}. The two optional hints are the only facts the *collapsed* card reads out of a body: the child session id of a generate_pikchr result (searched exactly as innerSessionIdFromValue does) and the source of a render_pikchr input. Verb, detail, status tone and category come from the row's other columns; hasDetails only needs to know a body exists. - Store::get_tool_call_body(id) -> Option<ToolCallBody> returns the three bodies with tool images projected to refs, so a fetched body is byte-for-byte what the transcript would have carried pre-deferral. Fetch path - GET /api/tool-bodies/{message_id} on the authenticated media router: JSON, compressed, read under spawn_blocking, Cache-Control: private, max-age=31536000, immutable like /api/tool-images; 404 for unknown or non-numeric ids; 401 without the session cookie. - Tauri command get_tool_call_body(messageId) and the matching web invoke arm (blocking_store_json), sharing session_commands:: tool_call_body. Client - toolCallBodies.ts recognises markers (deferredToolBody, deferredToolBodyIds) and hydrateToolItem swaps them for fetched fields, leaving unfetched markers in place and the item identity unchanged when nothing applies. - toolBodySource.ts fetches through the command under Tauri and plain fetch (cookie auth) on the web, memoised per message id in a module-level promise cache (cap 200); a failed load is evicted so Retry refetches. - ToolCallCard loads the missing bodies in an $effect the first time it is expanded, builds the view model from the hydrated item, and shows "Loading details..." / "Couldn't load the tool details." + Retry in place of the details. Collapse and re-expand never refetch. The collapsed card is unchanged across all variants: acpTranscript reads the pikchr hints off a marker, and hasDetails treats a marker as a present body. Threshold Sweep over the two profiled local sessions (serialized bytes of the store output, after image projection; counts are deferred bodies/rows): session / read inline 8 KB 4 KB 2 KB (chosen) 1 KB 512 B image-heavy page 301 774,077 650,276 616,387 509,718 (55/31) 435,794 379,840 image-heavy full (431) 1,291,210 1,004,378 905,250 701,600 (113/60) 584,892 517,937 text-heavy page 301 690,686 658,825 548,060 459,481 (54/53) 408,055 382,007 text-heavy full (771) 1,907,477 1,615,341 1,281,877 1,025,201 (174/173) 925,949 877,609 Each extra deferred body saves ~4.6 KB going 8->4 KB, ~2.6 KB going 4->2 KB and ~1.3 KB going 2->1 KB, while costing a round trip on expand; below ~2 KB a body fits in about one compressed segment, so 2048 is where shipping inline becomes cheaper than fetching. Metadata (_since refetch) responses are unchanged: 80,232 / 44,165 bytes. Measured on read-only .backup copies of ~/.staged/data.db, sessions 82598983-75c7-49a1-bf8f-62c0befc46cb (image-heavy) and 43d5c5b3-99ed-4b98-90a2-07a9e0dbc2fe (text-heavy). Per row, acp_content is the heaviest column (text-heavy: 1.05 MB content, raw_output all NULL; image-heavy: 484 KB content / 314 KB output / 144 KB input), so all three columns are deferred. Legacy tool_result rows have NULL status and no ACP bodies, so they are never touched. Follow-up (not in this change): the visible tool_call.content column is {"name","input":<raw_input>} and duplicates acp_raw_input on every row (205 KB / 286 KB in the two sessions); dropping the copy at read time would be a further ~15% on top of this. Verification: cargo check --tests, cargo clippy -- -D warnings, cargo fmt --check, cargo test --lib --bins --tests (1051 passed), pnpm run check (0 errors / 0 warnings), vitest (1304 passed, 12 new), prettier. New tests cover the projection (settled-only, size after image projection, pikchr hints, replay/archive untouched), the route (200/401/404 with exact JSON and headers), the command path, and the client marker/hydration/cache/retry behaviour. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
…sion call
While a session runs, SessionChatPane polled every 500 ms with four
requests, three of them serialised: get_session, then
list_queued_session_messages, then get_session_messages_since and
get_session_acp_metadata_messages_since in parallel. That is 8 requests/s
per open pane, three round trips per tick, and four spawn_blocking hops and
four store-lock acquisitions, nearly all returning empty arrays during a
long tool call. This is phase 1 of the poll plan note: one request per
tick, no new events, no schema change, and the client merge logic is
unchanged.
Store. The new store/session_poll.rs adds `Store::poll_session(session_id,
&SessionPollRequest { since_id, refetch_ids, metadata_since_id,
metadata_refetch_ids })`, which returns `SessionPoll { session,
queued_messages, messages, acp_metadata_messages }`. The four existing
readers are split into `*_with_conn(&Connection, ...)` bodies that their
public methods lock and delegate to, because std's Mutex is not
re-entrant. poll_session takes the lock once and runs all four inside one
read transaction. The lock already shuts out this process's writers, and
the transaction extends the snapshot to any other process on the file.
Both message readers still go through transcript_message_from_row, so tool
images become refs and large bodies become deferred markers exactly as in
the standalone commands. The runner writes an assistant row's final text
before it flips status, so a poll that reports a terminal status now
always carries that text. finishPolling() stays for the event path.
Transports. There is a `poll_session` Tauri command, registered in
generate_handler!, and a matching web invoke arm that reads `sessionId`
and `request` and runs under blocking_store_json. A source-level test pins
the spawn_blocking path. The four standalone commands and arms stay
because liveSessionHints, BranchCard and others still call them.
Client. `SessionPollRequest` / `SessionPoll` types and a never-cached
`pollSession` command. `sessionPollRequest(messages, acpMetadataMessages)`
in messagePoll.ts now computes the cursors that used to be inline in the
pane: the tail id, openAssistantRefetchIds, the metadata tail id and the
unsettled tool-row ids. poll() becomes one await. It applies the session
under the same statusEventDuringFetch guard, the queue under the same
JSON compare as refreshQueuedMessages (still used by the status event and
the queue actions), and the metadata and visible rows through
applyAcpMetadataUpdates / mergeSinceMessages as before. The stop condition
is unchanged.
The empty-transcript branch is folded in. Polling from since_id 0 returns
every visible row, and mergeSinceMessages([], rows, 0) yields them with
appended = true (pinned in messagePoll.test.ts), so hasOlderMessages
correctly stays false. That branch's windowed getLatestSessionMessages
call with bypassCache existed only to dodge the SWR cache's stored empty
list, and the since path never touches the cache. One trade-off: in the
rare case where a stale cached empty window is still revalidating when the
first tick fires, that tick reads the whole visible transcript instead of
the newest page. That is a one-off, matches the behaviour before
windowing, and the revalidated window merges over it through
applyLatestWindow.
Effect: 8 -> 2 requests/s per running pane on the web, and 8 -> 2 IPC
calls/s on desktop, with one round trip per tick instead of three serial
ones.
Verification: cargo fmt --check, cargo clippy -- -D warnings, cargo test
--lib --bins --tests (1054 passed), including two new store tests (poll
output equals the four standalone reads for the same cursors with queue,
refetched interrupted assistant row, in-flight tool row and a projected
image ref; a zero cursor returns every visible row; an unknown session is
an empty poll), the new web arm test and web_dispatch_covers_tauri_commands;
pnpm run check 0 errors / 0 warnings; vitest 1308 passed (4 new); prettier
clean. Not yet manually checked in a browser (Network tab one POST per
tick, streaming, tool settle, cross-tab queue).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
…d rows Review finding on bd73ab1: `project_tool_images` was status-agnostic while `project_tool_bodies` gates on `is_settled_tool_status`, yet `/api/tool-images/{id}/{field}/{index}` answers with `Cache-Control: private, max-age=31536000, immutable`. A ref's identity is positional, so if an in-flight `tool_call_update` already carried an image block and a later update replaced or reordered the block list before the row settled, a browser that had fetched `…/content/0` would keep the old bytes for a year. The "settled => immutable" argument the bodies module makes explicitly was never enforced for images. Both halves now hold: - `project_tool_images` returns early unless the row's `acp_tool_status` is terminal (same predicate as `tool_bodies.rs`, imported from there). In-flight rows ship their base64 inline from every transcript read, including the poll's `_since` and refetch paths; the client's inline branch in `toolCallImages.ts` renders those as before, so no client change is needed. Once the row settles the refetch delivers it with refs, exactly as it already delivered the row with deferred bodies. - `Store::get_tool_image` also selects `acp_tool_status` and returns `None` for an unsettled row, so the web route 404s and the Tauri command reports not-found rather than caching bytes the row may still replace. The transcript never emits such a ref, so this only matters for a stale ref held by an old client payload. Tests: `test_transcript_reads_project_tool_images_to_refs_and_keep_raw_bytes` now walks the row through in_progress (inline everywhere, lookup refuses) to completed (refs everywhere, lookup resolves); the poll test checks the in-flight tool row comes back inline and, after settling, projected and equal to the standalone refetch; the route test adds an in-flight row whose image URL is a 404. Verification: cargo fmt --check, cargo clippy -- -D warnings, cargo test --lib -- store:: web_server:: session_commands:: (343 passed), full cargo test --lib --bins --tests 1053 passed. The one failure in the full run, `test_poll_session_matches_the_four_standalone_reads` asserting queued-message order, is a pre-existing flake unrelated to this change: queued ids are UUID strings, so two messages inserted in the same `created_at` tick sort randomly under `ORDER BY created_at ASC, id ASC` (5 of 12 reruns fail, on the assertion that precedes the image checks). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
… copy Review finding on bd73ab1. `extractToolCallMetadata` built `input` as `recordOrNull(item.rawInput) ?? parsedArgs`, and a deferred-body marker (`{"$deferred":{...}}`, 0b775f8) is a record, so for a settled row whose input the store deferred, `input` was the marker and `parsedArgs` -- the `tool_call.content` `{"name","input":<raw_input>}` copy of the very same input -- was never consulted. The collapsed header reads `item.detail` and was unaffected, but `classifyToolCall` falls through to `categoryFromParsedCommands` and `hasNetworkMetadata(metadata.input)`, so a card that depends on those (a shell `curl`, a tool whose input carries `url`) could change icon the moment it expanded and `hydrateToolItem` swapped the marker for the fetched input. `inputSource` had the same shape, so had anything read `inputText` before hydration it would have pretty-printed the marker. `extractToolCallMetadata` now treats a deferred `rawInput` as absent for both `input` and `inputSource`, so `parsedArgs` fills in and the collapsed card's metadata, `parsedCommands`, category and `inputText` are identical before and after the body is fetched. The existing marker test keeps pinning the no-copy case (`input` null, `hasInputText` false, no pretty-printed marker) and a new test compares a deferred card against its hydrated twin field by field. Verification: vitest `src/lib/features/sessions` 310 passed (1 new); `pnpm run check` 0 errors / 0 warnings over 3422 files; prettier clean. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
…erred Review finding on bd73ab1. While a tool row is in flight the poll ships its bodies inline (0b775f8 defers settled rows only), so the pane holds the full acpRawInput / acpRawOutput / acpContent of every tool the user is watching. The tick that sees the row settle gets it back with each body over 2 KB replaced by a `$deferred` marker, and both merge points -- applyAcpMetadataUpdates for the metadata copy (which wins the id dedup in acpTranscript) and mergeSinceMessages for the visible tail refetch -- replaced the row wholesale. An expanded ToolCallCard then saw a new deferred id, dropped to "Loading details...", and fetched the bytes it had just been rendering. Most visible with Write/Edit calls, whose large input is present from the first tick. The merge is now per field. `retainInlineToolBodies(current, update)` in toolCallBodies.ts swaps each marker in the update back for the held value when it evidently is the body the marker stands in for, and returns the update itself otherwise so identity-based change detection is unaffected. Both applyAcpMetadataUpdates and mergeSinceMessages (refetched rows and tail rows alike, so the outcome does not hinge on which copy the dedup keeps) run returned rows through it before comparing. The guard: the review suggested keeping the held value whenever the update's field is a marker for the same row, but the held copy is not necessarily current. Streamed tool output accumulates in acp_content across in-flight tool_call_update rows, and ACP lets an update replace rawInput, so keeping blindly could pin stale partial output on a settled card. The marker already carries `bytes`, the store's `serialized_len` of the body it replaced (pinned to `serde_json::to_vec(..).len()`, i.e. UTF-8 bytes, which key order cannot change), so a held body is kept only when the UTF-8 length of its JSON.stringify equals that. Any mismatch -- a partial stream, a changed input, an in-flight copy whose images are still inline base64 where the settled row's are refs (2e9664a5), or a serde/JS formatting difference such as `1.0` vs `1` -- leaves the marker in place and the card fetches exactly as before. The check only runs when a marker meets a held inline value, so it costs one stringify per settling body, not per tick. Tests: retainInlineToolBodies keeps matching bodies (including a non-ASCII one, so the UTF-8 rather than UTF-16 length is what is compared), leaves a marker whose held body is a different size, and returns the update untouched for a missing/marker held value, a marker for another row or field, a different row id, and an update with no markers. mergeSinceMessages gets a case where the in-flight tail tool row returns settled and deferred alongside a new assistant row: the input stays the held object, the status and new content come from the update, and the append is still reported. Verification: vitest src/lib/features/sessions 314 passed (4 new); pnpm run check 0 errors / 0 warnings over 3422 files; prettier clean. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
matt2e
force-pushed
the
slow-session-load-on-web
branch
from
October 14, 2026 02:17
bd73ab1 to
e889400
Compare
… flake test_poll_session_matches_the_four_standalone_reads asserts the queued messages come back in insertion order, but queued ids are UUID strings and created_at has millisecond resolution, so two follow-ups queued in the same tick sort randomly under `ORDER BY created_at ASC, id ASC`. It failed CI on PR #974 and roughly 5 of 12 local reruns. Sleep 2 ms between the two add_queued_session_message calls, the same idiom the other order-sensitive store tests in this file use, so the rows get distinct created_at values and the assertion stays as is. Verification: the test passed 15 of 15 consecutive runs; cargo fmt --check clean. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Review finding on e889400. `Store::get_tool_call_body` served a row whatever its status, while `/api/tool-bodies/{id}` answers with `Cache-Control: private, max-age=31536000, immutable`. The transcript only emits a `$deferred` marker for a settled row (0b775f8), so a normal client never asks for an in-flight id, but nothing stopped the request: a stale payload or a hand-typed URL would have had the browser pin a partial `acp_content` stream or a since-replaced `acp_raw_input` for a year. 8fc2219 closed the same gap for images by having `get_tool_image` return `None` for an unsettled row; this applies the same rule to bodies so the two routes agree and the cache header is honest in both. `get_tool_call_body` now also selects `acp_tool_status` and returns `None` unless `is_settled_tool_status` holds, so the web route 404s and the Tauri `get_tool_call_body` command reports not-found. Once the row settles the same URL resolves, which is the first time the transcript would have pointed at it anyway. Tests: the store deferral test asserts the lookup refuses the row while it is `in_progress` and resolves after `completed`; the route test adds an in-flight row whose `/api/tool-bodies/{id}` is a 404 alongside the unknown-id and non-numeric cases. Verification: cargo fmt --check and cargo clippy -- -D warnings clean; cargo test --lib -- store:: web_server:: session_commands:: 343 passed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Review finding on e889400. `is_image_mime_type` admits `image/svg+xml`, so a tool result carrying an SVG is served from the app origin with that `Content-Type` by `/api/tool-images/{id}/{field}/{index}`. Inside the `<img>` the client uses, the SVG is inert (no script, no external loads), but opened as a top-level document -- right-click "Open image in new tab", or any link, since the `staged_session` cookie is `SameSite=Lax` and the ids are sequential -- it runs its script on the app origin with the session cookie in scope. Agents routinely `read` files from untrusted repos, so this is a plausible prompt-injection -> XSS chain. `nosniff` does not help: the declared type is already a scriptable one. Nothing in the local database has such a block today (0 SVG image blocks across ~1.4 GB), so this is hardening against a hostile producer rather than a bug users hit, but the route is live on this branch and would be mounted as-is by the mobile-web restore. Every `serve_tool_image` and `serve_media` response now carries Content-Security-Policy: default-src 'none'; style-src 'unsafe-inline'; sandbox (`UNTRUSTED_MEDIA_CSP`), the policy raw.githubusercontent.com uses for the same problem. `sandbox` puts a top-level view in an opaque origin with script disabled, `default-src 'none'` blocks subresource loads (tracking pixels, exfiltration via `<image href>`), and `style-src 'unsafe-inline'` keeps the SVG's own `<style>` working when viewed directly. WebKit, Chromium and Firefox all honour `sandbox` from a response header (only `<meta>` ignores it). A response's CSP is not applied when it loads as an `<img>`, so rendering in the card is unchanged, and the header applies to any mime type, so it does not depend on listing dangerous subtypes -- which is why this was chosen over refusing `svg+xml` in `is_image_mime_type`, a deny-list that also controls projection and would have changed payload shape. `/api/media` cannot serve SVG today (`mime_for_extension` has no entry), but the one-line insert stops a future entry from silently reopening the hole. `/api/tool-bodies` is `application/json` + `nosniff`, which browsers will not run as a document, and is left alone. No client, store or Tauri-command change: the desktop path returns a `data:` URL that only ever lands in an `<img>`. Tests: the image route test's fixture gains a third block, an SVG whose body is `<script>alert(1)</script>`; `/output/2` must be served as `image/svg+xml` with the CSP header (pinning that SVG is still served, only ever sandboxed), the PNG case asserts the header too, and the past-the-end 404 case moves to `/output/3`. The media route test asserts `nosniff` and the CSP header on its 200 response. Stale caches: the route sends `immutable` for a year, so an SVG fetched before this change would stay cached without the header. The route is not on main yet and there are no SVG blocks locally, so there is nothing to migrate. Verification: cargo fmt --check and cargo clippy -- -D warnings clean under the hermit toolchain; cargo test --lib -- web_server::media_tests 3 passed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Review finding on e889400. After the opening page rendered, and again after each prepend, the pane called `loadOlderMessagesIfNearTop`, which reused the scroll handler's `LOAD_OLDER_THRESHOLD_PX` (600 px) check on `scrollTop`. The pane opens scrolled to the bottom, so for any session whose first page overflows the viewport by less than 600 px -- a transcript just over one screen, which is common -- `scrollTop` was already under the threshold and a second 300-row page was fetched immediately, roughly doubling the opening payload the windowing exists to cut. The intent of that call was only to make an under-height first page scrollable, since the scroll handler can never fire on a pane with nothing to scroll. The helper is now `loadOlderMessagesIfUnscrollable` and asks for the next page only while `scrollHeight <= clientHeight`. That is the one case where no scroll event can arrive; once the transcript overflows at all, `handleScroll` applies the 600 px threshold to real scrolling as before. Both call sites (post-load and post-prepend) use it, so a sequence of short pages still tops up until the pane overflows and then stops. `loadOlderMessages` already bails on `hasOlderMessages === false`, so an exhausted transcript does not loop. Verification: `pnpm run check` 0 errors / 0 warnings over 3422 files; prettier clean on the changed file. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Review finding on e889400. When `blocking_store_json` was inserted above `get_store` in web_server.rs, the `/// Helper to get the Store Arc from the shared mutex slot.` line stayed where it was, so rustdoc attached it to `blocking_store_json` (prepended to that function's own doc) and left `get_store` undocumented. Move the line back down to `fn get_store`. Comment-only change. Verification: cargo fmt --check clean under the hermit toolchain. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.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
1 participant
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.
Opening a long session in the web UI was slow: the browser downloaded the whole transcript twice, uncompressed, including every base64 screenshot (about 26 MB for the largest local session), and a running pane sent 8 requests/s. This branch cuts that down with five read-side changes. There's no schema or ingest change, and the client still renders legacy full-body payloads that are already cached in IndexedDB.
Changes
0266a8da)CompressionLayeron the Axum router, plus HTTP/2 via ALPN and axum'shttp2feature.get_session_acp_metadata_messagesno longer resends rows thatget_session_messagesalready ships. Hidden rows are limited to the kinds the client reads (TRANSCRIPT_METADATA_FILTER). Visible tool rows that are still in flight are kept, so status refetch keeps working.spawn_blocking.585425fa)acp_raw_outputandacp_contentare rewritten to{"type":"image","ref":"<id>/<field>/<index>"}.GET /api/tool-images/...(immutable cache, auth-gated) or from theget_tool_imagecommand on desktop.d28fb726)Store::get_session_messages_windowuses the existing index; the plan is asserted by a test.0b775f88)$deferredmarker.GET /api/tool-bodies/{id}or theget_tool_call_bodycommand, and shows loading and retry states.bd73ab13)Store::poll_sessionreturns the session, queue, new messages and metadata in one read transaction.Measured (serialized store output, read-only DB backup)
Both "before" numbers are uncompressed; brotli shrinks the "now" numbers further on the wire.
Testing
cargo fmt --check,cargo clippy -D warnings,cargo test(1054 passed) andpnpm run checkare clean; vitest: 1308 passed.🤖 Generated with Claude Code