You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(web): model settings no longer re-render every row on each star, toggle or move - #34964
Cause: ProviderModelRow (apps/web/src/components/settings/ProviderModelsSection.tsx) was a plain function component. Its handlers were already passed through useStableCallback and its other props are per-row primitives or reused model objects, but without memo every parent render re-rendered every row. The React Compiler caches JSX inside a component, not each row produced by a .map. Reproduced on a dev server (T3CODE_DEV_SEED=0 OPENROUTER_API_KEY=dummy) with OpenCode reporting 492 models: a temporary render counter showed 984 row renders per toggle or star (all rows, twice: optimistic update plus server echo), while a prop diff showed only the clicked row's isHidden and one neighbour's canMoveUp changing. Fix: wrap ProviderModelRow in React.memo, the same pattern AuthorizedClientsSettings uses for its list rows. After the fix, rows re-rendered per click went from 984 to 2-4. Toggles and moves produced no long tasks (before: ~60 ms each on this machine), and a 20-row filtered list showed no long tasks for stars. Not fully fixed: a star still costs about 54-100 ms with 492 rows, from React's keyed-list reordering (about 440 DOM insertions when a row moves up into Favorites), a separate cost listed under outOfScope. Nothing users see changes. No regexes or parsers added. No regression test: the defect is render cost and the existing tests cannot measure it.
Commit ff599f0 (origin/main). Ran T3CODE_DEV_SEED=0 OPENROUTER_API_KEY=dummy vp run dev (the dummy key makes OpenCode 1.19.5 report 496 models, close to the reporter's 404), paired a browser, opened Settings > Providers > Add provider > OpenCode > Next > Next > Add instance, then used the Models list. The list renders all 496 rows (2,497 tooltip triggers, about 12,600 DOM nodes). Unthrottled Chrome: one visibility toggle blocked the main thread about 250 ms and one star about 290 ms; typing in the filter and 'Disable all' stayed cheap. Headless Chrome at 1366x768 with CPU slowed 4x through CDP, measured with a longtask observer: one toggle, star or 'Move down' click blocked the renderer about 2.5-2.8 s in total (single tasks of 1.3-1.5 s); five toggles clicked 150 ms apart kept the renderer busy across about 5.9 s; a toggle inside a filtered list of about 15 rows cost about 190 ms, so the cost scales with rendered rows. WebSocket traffic per toggle was only about 64 KB and the server did no provider re-check, so the time is client rendering. Windows' 'Not Responding' dialog itself cannot appear on this Linux VM; the CPU slowdown approximates the reporter's slower Windows 10 machine.
Evidence
Being captured: before and after in the real app. This section is filled in before the pull request can merge.
Tests
vp test run src/components/settings/ProviderModelsSection.browser.test.tsx: 1 passed. vp run check:changed: format, lint, knip 1/2 and 2/2, typecheck (2 packages), test (1 file), react compiler all passed. Manual check in the dev app with 492 OpenCode models, before vs after: row renders per click 984 -> 2-4; toggle long tasks [60 ms] -> none; move -> none; star [57 ms] -> 0-100 ms (DOM-reorder cost, see outOfScope). CPU was not throttled, so the report's slow-machine numbers were not re-measured.
Not in this change
A star (or re-enabling a hidden model) moves a row earlier in the single keyed list, and React's reconciliation then moves every row between the old and new positions (~440 DOM insertions, ~54-100 ms per star with 492 rows on a fast machine). Splitting the list into one keyed container per group would avoid that, but the moved row would remount and lose keyboard focus, so it was not done here.
renderRow calls displayModels.indexOf(model) for each visible row, which is O(n^2) per section render; cheap at about 500 rows, so left unchanged.
The list is not virtualized; with rows memoized it no longer needs to be for the click cost, so it was not added.
AI-assisted; verified with the tests and tools listed above.
…toggle or move
ProviderModelRow already received stable handlers but was not memoized, so
every favorite, visibility or order change re-rendered all rows. With
OpenCode reporting ~500 models that blocked the renderer per click.
AI-assisted; verified with the tests and tools listed above.
Macroscope's review found this PR approvable — Memoizes model-settings rows so unchanged rows can skip re-rendering during preference updates. Existing actions, permission guards, and product defaults remain unchanged.
Notes:
All code in this push has already been reviewed. Approvability was decided on eligibility alone.
No code changes detected at 708e515. Prior analysis still applies.
You can add or adjust custom eligibility rules. Learn more.
This change reduces unnecessary re-renders of model rows in provider settings, and no behavior change is expected. No concrete merge-blocking risk was found. Maintainers may want to confirm that the failing web test job is unrelated to this change.
Pre-merge checks: 3 · 1
Failed checks: 1 inconclusive
Check name
Status
Explanation
Resolution
Linked Issues check
Issue #21281 reports that changing OpenCode models in Settings > Providers makes the app crash on Windows 10. The requested behavior is that users can enable, disable, star, and manage models without …
Provide measurements of the reviewed head under CPU throttling comparable to the reported slow machine, covering toggle, star, and move on a list of about 490 OpenCode models. Also provide evidence, or an explicit reason, that the reported …
Passed checks: 3 passed
Check name
Status
Explanation
Title check
The title names the main change: ProviderModelRow re-renders for every row on each star, toggle, or move in model settings. The raw summary confirms the change wraps ProviderModelRow in React memo, wh…
Description check
The description follows the repository template. It has Problem (Cause), Change (Fix), Scope (links #21281 and explains the bug fix), and Verification sections. The Verification sectio…
Out of Scope Changes check
The whole-PR diff (merge base 8024e50 to head 472d092) changes three lines in ProviderModelsSection.tsx. It adds the memo import, wraps ProviderModelRow in `memo(function ProviderModelRow ..…
Full details: Title check
Explanation
The title names the main change: ProviderModelRow re-renders for every row on each star, toggle, or move in model settings. The raw summary confirms the change wraps ProviderModelRow in React memo, which matches the title's fix. The title uses the conventional commit format and plain language, as the repository template requests. Its length (83 characters) is acceptable.
Full details: Description check
Explanation
The description follows the repository template. It has Problem (Cause), Change (Fix), Scope (links #21281 and explains the bug fix), and Verification sections. The Verification section gives specific measured results (984 to 2-4 row renders, long task changes) and states what was not checked (CPU throttling not re-measured, no regression test). It also includes the reproduction steps. The Evidence section is incomplete: before/after screenshots or a recording are marked as 'being captured' and the template requires them for UI changes. The model and harness line is present under the AI-assisted note.
Resolution
Complete the Evidence section with before/after screenshots or a short recording of the real app, as the template requires for UI changes, before merging. The PR itself states this section must be filled in before merge.
Full details: Out of Scope Changes check
Explanation
The whole-PR diff (merge base 8024e50 to head 472d092) changes three lines in ProviderModelsSection.tsx. It adds the memo import, wraps ProviderModelRow in memo(function ProviderModelRow ...), and closes the wrapper. No props, exports, or rendered output change. The change is directly tied to the render-cost defect described in the PR and the linked issue. The PR description lists the O(n^2) indexOf lookup, keyed-list reordering, and virtualization as deliberately excluded. The diff does not include any of them, so no unrelated change is present.
Full details: Linked Issues check
Explanation
Issue #21281 reports that changing OpenCode models in Settings > Providers makes the app crash on Windows 10. The requested behavior is that users can enable, disable, star, and manage models without failure. The reviewed head changes one component: ProviderModelRow (apps/web/src/components/settings/ProviderModelsSection.tsx) is wrapped in memo. The diff contains no other code change. The PR evidence shows that each click re-rendered all rows. The author reports row renders per click fell from 984 to 2-4, and that toggles and moves no longer produce long tasks on the test machine. Two gaps remain. First, the crash was not reproduced on Windows, and the reported blocking time was measured only on Linux with CPU throttling. Second, the post-fix CPU-throttled measurements were not repeated. A star still costs about 54-100 ms with 492 rows. The PR does not show that the crash condition is removed under the reporter's conditions.
Resolution
Provide measurements of the reviewed head under CPU throttling comparable to the reported slow machine, covering toggle, star, and move on a list of about 490 OpenCode models. Also provide evidence, or an explicit reason, that the reported crash path depends on the main-thread blocking that the memo change removes.
Finishing Touches
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
The automated pipeline stopped on this pull request and will not merge it:
the fix is written and reviewed, but proving it needs something no worker has:
Permanent blocker: Windows is required to exercise the reported Windows crash, but no Windows environment is available to this worker. The brief directs this step to stop immediately for an unavailable required operating system.
User-visible responsiveness remains unverified: no new paired browser recordings or performance traces were collected, so reduced row renders cannot be claimed to eliminate interface stalls.
The automated pipeline stopped on this pull request and will not merge it:
the capture step could not show the problem on the base commit, so there is nothing to prove fixed.
On both Linux commits: open Settings > Providers; Add provider > OpenCode > Next > Next > Add instance. Leave the model filter empty. Disable five individual models, star Qwen3.7 Plus well down the list, use Move down on Aion 3.5 Mini, then send five further visibility clicks with a requested 150 ms pause between CDP input sequences. Capture the interactions and collect renderer long-task timings. CDP backpressure means actual rapid-click intervals can exceed 150 ms. Inspect decoded recording frames and final stills; final stills required matching screenshots 500 ms apart.
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
size:XS0-9 changed lines (additions + deletions).takeover:blockedTakeover blocked; reason in the PR bodyvouch:trustedAuthor is trusted by repo permissions or the VOUCHED list.
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.
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
juliusmarminge
added
the
takeover:in-progress
Cause: ProviderModelRow (apps/web/src/components/settings/ProviderModelsSection.tsx) was a plain function component. Its handlers were already passed through useStableCallback and its other props are per-row primitives or reused model objects, but without memo every parent render re-rendered every row. The React Compiler caches JSX inside a component, not each row produced by a .map. Reproduced on a dev server (T3CODE_DEV_SEED=0 OPENROUTER_API_KEY=dummy) with OpenCode reporting 492 models: a temporary render counter showed 984 row renders per toggle or star (all rows, twice: optimistic update plus server echo), while a prop diff showed only the clicked row's isHidden and one neighbour's canMoveUp changing. Fix: wrap ProviderModelRow in React.memo, the same pattern AuthorizedClientsSettings uses for its list rows. After the fix, rows re-rendered per click went from 984 to 2-4. Toggles and moves produced no long tasks (before: ~60 ms each on this machine), and a 20-row filtered list showed no long tasks for stars. Not fully fixed: a star still costs about 54-100 ms with 492 rows, from React's keyed-list reordering (about 440 DOM insertions when a row moves up into Favorites), a separate cost listed under outOfScope. Nothing users see changes. No regexes or parsers added. No regression test: the defect is render cost and the existing tests cannot measure it.
Fixes #21281
Reproduction on main
Commit ff599f0 (origin/main). Ran
T3CODE_DEV_SEED=0 OPENROUTER_API_KEY=dummy vp run dev(the dummy key makes OpenCode 1.19.5 report 496 models, close to the reporter's 404), paired a browser, opened Settings > Providers > Add provider > OpenCode > Next > Next > Add instance, then used the Models list. The list renders all 496 rows (2,497 tooltip triggers, about 12,600 DOM nodes). Unthrottled Chrome: one visibility toggle blocked the main thread about 250 ms and one star about 290 ms; typing in the filter and 'Disable all' stayed cheap. Headless Chrome at 1366x768 with CPU slowed 4x through CDP, measured with a longtask observer: one toggle, star or 'Move down' click blocked the renderer about 2.5-2.8 s in total (single tasks of 1.3-1.5 s); five toggles clicked 150 ms apart kept the renderer busy across about 5.9 s; a toggle inside a filtered list of about 15 rows cost about 190 ms, so the cost scales with rendered rows. WebSocket traffic per toggle was only about 64 KB and the server did no provider re-check, so the time is client rendering. Windows' 'Not Responding' dialog itself cannot appear on this Linux VM; the CPU slowdown approximates the reporter's slower Windows 10 machine.Evidence
Being captured: before and after in the real app. This section is filled in before the pull request can merge.
Tests
vp test run src/components/settings/ProviderModelsSection.browser.test.tsx: 1 passed. vp run check:changed: format, lint, knip 1/2 and 2/2, typecheck (2 packages), test (1 file), react compiler all passed. Manual check in the dev app with 492 OpenCode models, before vs after: row renders per click 984 -> 2-4; toggle long tasks [60 ms] -> none; move -> none; star [57 ms] -> 0-100 ms (DOM-reorder cost, see outOfScope). CPU was not throttled, so the report's slow-machine numbers were not re-measured.
Not in this change
AI-assisted; verified with the tests and tools listed above.