perf(console): chat keystrokes stop repainting the view — in both modes - #108
Merged
Conversation
The context split exists so that typing does not repaint the tree, and it covered the toolbar search: `query` moved into its own context, and the sidebar and header stopped re-rendering for a character only the views care about. The composer was left sharing that context, and it is the one typing surface that sits on top of a view rather than beside it. So every character of a question re-rendered whichever view was mounted under the Ask slide-over. None of them read `chatInput` — Concepts, Conflicts, Files, Sources and Triage subscribe to `input` for `query` alone — but a context does not care what you destructure off it. Measured on the demo fixture: four characters typed into the composer, forty renders of the layer chips in the Concepts list behind it. The two surfaces have disjoint audiences, so they are now disjoint contexts. `input` is `query`, read by the five searchable views; `chat` is the composer, its transcript and its busy flag, read by ChatPanel and nothing else. Same delta as the original split, for the case it did not reach. The test is the pair, not the half: "the view did not re-render" is satisfiable by a composer that stopped updating, so it also asserts the composer holds what was typed. Views render no icons, which is what the existing suite counts, so this case counts LayerChip — rendered inline and unmemoized on every Concepts row. It fails at 40 against the code before this commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: John Siracusa <siracusa5@users.noreply.github.com>
…ips in Adversarial review of the commit before this one found that its headline number is unchanged in live mode, and it is right. `activity` is a dependency of the `data` memo, and it was `mode === 'demo' ? demoActivity : []`. The demo branch is a module constant. The live branch is a fresh array on every provider render — so in live mode `data` changed identity every time any provider state moved, `App` subscribes to `data`, and `App` owns every memoized child in the shell. One keystroke, one whole-tree repaint. Both context splits, the original and the chat one, were no-ops in the mode the Mac app runs in. The render tests could not see it. `createDataSource()` with no query string picks demo, which is precisely the mode that takes the stable branch, so the entire render budget was measured on the Web Demo and asserted of the product. Reproduced before fixing: same probe, same four composer keystrokes, 40 renders in live mode against 0 in demo. So `NO_ACTIVITY` is hoisted to module scope, and the budget is pinned in both modes. `render-hygiene.live.test.tsx` mounts the store with a source that reports `mode: 'live'` while still answering from the demo bundle — the mode flag is the whole variable under test, and a real LiveSource in jsdom would only measure the missing network. It fails at 40 without the hoist. It also counts a `useStoreData()` probe standing in for `App`, because that is the subscription this bug traveled through: the view is downstream of it, and a future dependency that is unstable in one mode should fail on the shell rather than on whichever view happened to be mounted. Corrections to the previous commit's own claims, from the same review: - The header does not stop re-rendering on a search keystroke and cannot: it owns the controlled field. The repo's own test asserts five renders for five characters. `query` has six consumers, not five — the Header and the five searchable views. - `useStore()` calls `useStoreChat()`, so ChatPanel is the only *component* that does, not the only caller. - `App.tsx` said "three narrow subscriptions" and makes two. Wrong when it was written, and worse beside a four-context store. The six store mocks that replace the whole module now declare `useStoreChat` too. Nothing under them reads chat today; the failure mode when something does is `useStoreChat is not a function`, and tsc cannot see into an untyped factory. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: John Siracusa <siracusa5@users.noreply.github.com>
…ave yet
Adversarial review of the test, this time, and it produced a working
counter-example rather than a worry: memoize the layer chips behind a prop that
carries the query, memoize ConceptDetail, reapply the chat regression in full —
and all 319 tests stay green while the view repaints on every keystroke.
The probe was the problem. It counted `LayerChip`, a leaf the view renders per
row, on the reasoning that one render of the view is one render of the chip.
That holds only until someone puts a `memo` between them, which
match-highlighting or list virtualization would do for ordinary reasons, on a
repo that talks about 3,000-concept vaults. The paired canary doesn't save it
either: once a memo key tracks the query, the canary is satisfied by that
dependency alone and stops proving that a context reached the view.
So the counter moves inside the memo boundary. `counted()` re-exports a view
module with the counter wrapping the view's own inner function, `React.memo`
unwrapped, so the view's hooks become the counter's hooks and every context it
subscribes to is observed directly. Nothing added inside the view can hide from
it. Counting a wrapper AROUND the view would not work — that counts the parent,
which a context-driven re-render never touches, and is why React.Profiler
reports zero here.
The chat cases are now table-driven over the same SEARCH_CASES table as the
search half, with the same completeness gate against SEARCHABLE_VIEWS. All five
views mount under the Ask panel; only Concepts was covered, so the other four
could each subscribe to `useStoreChat()` uncaught. That asymmetry was against
this file's own established rule.
The live file gains the case the stand-in probe structurally cannot make:
`DataConsumerProbe` mirrors App's data subscription and therefore cannot see a
NEW subscription added to the real App. So the real shell is mounted, the panel
opened by clicking Ask, and App's own renders counted.
Verified by mutation, each restored after:
- chat split reverted -> all 5 chat cases fail, delta 4-5 view renders
(the old probe reported 40 chip renders, which
is how a memo could swallow it)
- NO_ACTIVITY reverted -> both live cases fail
- `useStoreChat()` in App -> the shell case fails, and only that one
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: John Siracusa <siracusa5@users.noreply.github.com>
…ts on Neither is a defect and both are invisible at the call site, which is the problem — review found them by instrumenting the provider, not by reading. The measurement windows have to stay synchronous. Live mode genuinely arms the poll loop (one 5s timer per test, cleared on unmount), and a poll landing mid-window would move the counts honestly: `setLastRefreshAt` feeds `load`, and `load` is a dependency of the `data` memo. Nothing can interleave with straight-line code, so the counts are exact today. One `await` between a baseline and its assertion turns that into a five-second flake. The api Proxy resolves `this` only because DemoSource's fields are TypeScript `private` — a compile-time marker over a plain property, so `this.bundle` re-enters the trap. A real `#private` field would make every call through it throw, and it would arrive as the store's generic `refreshError` rather than as an obvious failure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: John Siracusa <siracusa5@users.noreply.github.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.
What
Three commits, replayed onto current
main. Each was caused by the adversarial review of the one before it.perf—chatBusy/chatInput/chatMessagesmove out ofStoreInputinto a fourth context,StoreChat, read only byChatPanel.StoreInputis now justquery.fix—NO_ACTIVITY. A one-line bug without which commit 1, and the context split it builds on (459ed96, already on main via Phase 5: desktop responsiveness + engine watchdog #105), is a no-op in live mode.test— the render probe in commit 1 could be blinded by amemothe views don't have yet. Rewritten to count inside the memo boundary, and made table-driven across all five searchable views.Why 1
The context split exists so a keystroke doesn't repaint the tree. It covered the toolbar search and left the chat composer sharing that context — and the composer is the one typing surface rendered on top of a view rather than beside it. All five searchable views read
queryfrom that context; none readchatInput. Every character of a question repainted whichever view was under the Ask slide-over.Why 2 — the finding with teeth
activityis a dependency of thedatamemo, and it wasmode === 'demo' ? demoActivity : []. The demo branch is a module constant; the live branch allocated a fresh array on every provider render. So in live modedatachanged identity whenever any provider state moved — andAppsubscribes todataand owns every memoized child. One keystroke, one whole-tree repaint.Both splits were no-ops in the mode the Mac app runs in, and the tests could not see it:
createDataSource()with no query string picks demo, exactly the mode that takes the stable branch. The whole render budget was measured on the Web Demo and asserted of the product.NO_ACTIVITYWhy 3
Review produced a working counter-example, not a worry: memoize the layer chips behind a query-carrying prop, memoize
ConceptDetail, reapply the chat regression in full — 319 tests green while the view repaints per keystroke. Counting a leaf the view renders is valid only until someone puts amemobetween them, and the paired canary stops helping once a memo key tracks the query.counted()now re-exports a view module with the counter wrapping the view's own inner function,React.memounwrapped: the view's hooks become the counter's hooks, so nothing added inside the view can hide from it. (A wrapper around the view counts the parent, which a context re-render never touches — the same reasonReact.Profilerreports zero here.)Tests
render-hygiene.test.tsx— chat cases table-driven over the sameSEARCH_CASEStable as the search half, with the same completeness gate againstSEARCHABLE_VIEWS. Each asserts both halves: the view must not repaint, and the composer must hold what was typed (else a dead composer satisfies it), and a search keystroke must still reach that view.render-hygiene.live.test.tsx— new. Mounts the store with a source reportingmode: 'live'while still answering from the demo bundle (the mode flag is the whole variable under test; a realLiveSourcein jsdom would only measure the missing network). Pins the same properties there, and mounts the realApp— a stand-in probe structurally cannot see a new subscription added to the real one.Verified by mutation, each restored after:
NO_ACTIVITYreverteduseStoreChat()added toAppAlso in here
Corrections found by the same review: the Header cannot stop re-rendering on a search keystroke (it owns the controlled field),
queryhas six consumers not five,useStore()is itself auseStoreChat()caller, and a pre-existingApp.tsxcomment claimed "three narrow subscriptions" where it makes two. The six whole-module store mocks now declareuseStoreChat, so the mocked surface matches the real one.Branch note
These commits also exist on
c/desktop-responsiveness, where they were reviewed and merged as #107. That branch predates #106 and its own contents are already on main via #105 (verified:git diff bcdfcb5 6f41b14is empty), so a PR from it would have shown 19 commits, 15 already merged, and a diff that appeared to delete the discrepancy center. This is the same three commits replayed onto current main instead.Gates
From
apps/console/:npm run typecheckclean ·npm test327 passed (27 files) ·npm run build·npm run build:live.🤖 Generated with Claude Code