Conversation
Every JSONL append from a live session made the renderer call get-projects twice (archived hidden and shown), each one reading the whole session cache, and then rebuild the full sidebar about once a second. get-projects now returns both views from one build, and the projects-changed reload skips the sidebar rebuild when nothing it displays has changed, refreshing only the time labels.
|
Reviewing |
devsuitup
left a comment
There was a problem hiding this comment.
Adversarial review at e313f7f.
I could not break the core of this one. The only get-projects consumer is loadProjects in public/app.js, both on this branch and on current main, so the new { projects, allProjects } shape breaks nothing. The archived view matches the old per-flag build, including an all-archived project, which survives through the empty-dir pass. The two views share session objects inside one structured-clone message, so the payload is not doubled. I traced the skip signature against the render code: busy, attention and response-ready come through the localPtyStates snapshot, remote host status through the project fields, and filters, search and the pty sets through the state blob. Local run of 10 related test files: 49/49 pass. CI has not run. Changes requested, mostly hygiene.
Must fix
- Rebase onto main.
merge-treereports three text conflicts:- the
main.jsdestructure at ~473, where main addsisIndexingFinished; keep it next tobuildProjectViewsFromCache; - the
get-indexing-statehandler that main adds just aboveget-projects(~1053); preload.js:12, where main addsgetIndexingState.
public/app.js,public/sidebar.jsandsession-cache.jsalso changed on main and auto-merge, so re-run the tests on the merged tree.
- the
- CHANGELOG or
no-changelog. One or the other is required. The PR states one visible side effect: the group-header time and the status-age text can lag by up to a minute while renders are skipped. I would give that a one-line entry. If you think it does not deserve one, say so and the maintainer will add the label, since your account cannot.
Should fix
- A skipped item is never repaired after a rename. While a
.session-rename-inputis open,onBeforeElUpdatedkeeps that item from being updated (sidebar.js~1046).renderProjectsstill storeslastSidebarRenderSignatureas if the item had been rendered (~1109).- Scenario: rename is open on session X, X's title or
modifiedchanges, then the user cancels. X shows the old label until the next real data change. Before this PR, the next render about a second later fixed it. - Fix: leave the signature
nullwhen an item was skipped for an open rename input.
- Scenario: rename is open on session X, X's title or
- The signature is a hand-maintained list of what
buildSessionItemandbuildSubagentItemread. It covers today's inputs. A future input that is not added there leaves the sidebar silently stale on aprojects-changedreload. Three PRs queued ahead each add sidebar state: #395 (remote attention), #400 (setSessionMcpState) and #374 (thebgbadge). Whichever lands second has to extend the signature. List the contract in.ai/contexts/session-cache.md. A test that fails when a builder reads state the signature ignores would be better still.
Nits
- Stale comments still describe the two-call behaviour:
session-cache.js~360 and ~632, andtest/build-projects-cold-scan-fallback.test.js~87. - The signature is computed twice on every render that runs, which adds cost rather than saving it. The 30–40 ms for ~5.4k sessions was measured in jsdom; a quick live check with
task test-prbefore merge would confirm it.annotateRemoteAttachableandmergePlaceholderSessionsstill run once per view: redundant, but harmless.
|
Closing for now: this change will be folded into a single perf PR with the other optimizations, opened once tested live. |
Why
Measured on a live instance with 6 Claude sessions: the renderer averaged ~33 % of a core and the main process ~16 %, with spikes to 110 %.
Every JSONL append from a live session sends
projects-changed(throttled to 1.5 s). On each one, the renderer:get-projectstwice, once with archived sessions hidden and once with them shown. Each call read the whole session cache (SELECT *over 5k+ rows), rebuilt the project list and cloned it across IPC;Change
get-projectsreturns{ projects, allProjects }from a single build:gatherProjectInputs()does every read once, andassembleProjects(inputs, showArchived)produces each view. The renderer can't derive the archived-hidden view on its own, because a project whose sessions are all archived is listed through the empty-directory pass.projects-changedreload skips the sidebar rebuild when a signature of everything the sidebar displays is unchanged, and refreshes only the time labels.modifiedis rounded to the minute, so a live session that only appends costs one render a minute instead of one a second.Known trade-offs
sidebarRenderSignatureby hand.The rationale is in
.ai/contexts/session-cache.md("Sidebar refresh cost"), andipc-bridge.mdis updated.Tests
test/sidebar-refresh-single-fetch.test.js(10 tests) checks:loadProjects;test/get-projects-cold-start-reconcile.test.jsis adapted to the new shape.task checkpasses.Not yet verified live with
task test-pr.