Skip to content

perf(web): avoid scanning chat history for sidebar backgrounds - #11206

Merged
juliusmarminge merged 1 commit into
mainfrom
legend-perf/sidebar-style-scope
Sep 11, 2026
Merged

perf(web): avoid scanning chat history for sidebar backgrounds#11206
juliusmarminge merged 1 commit into
mainfrom
legend-perf/sidebar-style-scope

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 11, 2026

Copy link
Copy Markdown
Member

Every chat DOM update invalidated the outer sidebar wrapper's :has([data-variant=inset]) rule, even though the app uses a normal sidebar. Remove that unused query. Inset compositions can set bg-sidebar explicitly through the existing className prop; current app callers and their backgrounds are unchanged.

Six counterbalanced production-browser rounds against the previous stack tip, now merged as main:

Median Before After
Cached thread switch 163.975 ms 104.425 ms
Growing code update 104.85 ms 27.05 ms
Switches with sampled blank frames 0/48 0/48

Switching improves 36.3% and growing-code synchronous updates 74.2%, each improving in all six paired rounds. Prose-streaming differences were inconsistent; no reliable prose gain is claimed. These are client rendering measurements on this runner, not provider throughput or overall app speed.

Methods, rejected experiments and exact revisions · Raw samples and scripts

Before:

Before: conversation and sidebar

Before switching recording

After:

After: same conversation and sidebar appearance

After switching recording

Production build and targeted formatting/lint completed. Lint retains the unrelated existing Math.random-in-render warning. Browser checks covered light/dark, wide/narrow layouts, sidebar toggling, and interrupted navigation. Desktop shares this CSS; Electron was not run. Native mobile is unaffected.

GPT-6 / Codex, with agent-browser verification.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Sep 11, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This one-file change narrows the shared SidebarProvider's default background behavior for inset layouts, even though the current application caller supplies the background explicitly. Because it changes a product UI default, the change warrants human review.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 11, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 93598a39-f2ea-45da-8e32-a8d21a5658e9

📥 Commits

Reviewing files that changed from the base of the PR and between 211618f and 6a961ad.

📒 Files selected for processing (1)
  • apps/web/src/components/ui/sidebar.tsx

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The sidebar wrapper no longer applies bg-sidebar for inset variants. Inset layouts must provide their background class separately.

Changes

Sidebar background handling

Layer / File(s) Summary
Remove wrapper background class
apps/web/src/components/ui/sidebar.tsx
The wrapper keeps its base layout classes and caller-provided className. The conditional inset background class was removed.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix

Suggested reviewers: t3dotgg

Merge Risk: ⚪ Minimal · up to 6a961

The styling change preserves the inset sidebar background while avoiding the unnecessary wrapper behavior, so no merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the main change: preventing unnecessary chat-history scans caused by sidebar background styling.
Description check ✅ Passed The description clearly explains the change, motivation, measured results, validation scope, and UI evidence. It does not use the template headings or checklist, but it contains the required informati…
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch legend-perf/sidebar-style-scope

Comment @coderabbitai help to get the list of available commands.

@juliusmarminge juliusmarminge changed the title legend perf/sidebar style scope perf(web): avoid scanning chat history for sidebar backgrounds Sep 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 13.6 KiB 13.6 KiB −4 B (−0.0%) 15.1 KiB
Codex Thread snapshot wire 7.1 KiB 7.0 KiB −4 B (−0.1%) 7.3 KiB
Codex Live turn WebSocket wire 6.6 KiB 6.6 KiB 0 B (0.0%) 7.8 KiB
Codex Live turn WebSocket decoded 57.1 KiB 57.1 KiB 0 B (0.0%) 66.4 KiB
Codex Live turn messages 10 10 0 (0.0%) 21
Claude Total thread wire 13.6 KiB 13.5 KiB −14 B (−0.1%) 15.1 KiB
Claude Thread snapshot wire 7.1 KiB 7.1 KiB −7 B (−0.1%) 7.3 KiB
Claude Live turn WebSocket wire 6.5 KiB 6.5 KiB −7 B (−0.1%) 7.8 KiB
Claude Live turn WebSocket decoded 57.8 KiB 57.8 KiB 0 B (0.0%) 66.4 KiB
Claude Live turn messages 8 8 0 (0.0%) 21

Baseline: 211618f · PR result: 6a961ad · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 113.9 KiB
  • Claude decoded thread snapshot: 114.6 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@juliusmarminge
juliusmarminge added this pull request to stack #11212 September 11, 2026 07:59
@juliusmarminge
juliusmarminge merged commit 2b7d3a4 into main Sep 11, 2026
21 checks passed
@juliusmarminge
juliusmarminge deleted the legend-perf/sidebar-style-scope branch September 11, 2026 19:04
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 11, 2026
## What's Changed
* fix(mobile): prevent Hermes crashes when opening threads by @jakeleventhal in pingdotgg/t3code#11233
* feat(web): open Usage on the Limits tab by default by @juliusmarminge in pingdotgg/t3code#11261
* perf(web): avoid scanning chat history for sidebar backgrounds by @juliusmarminge in pingdotgg/t3code#11206
* perf(mobile): reuse completed code lines while streaming by @juliusmarminge in pingdotgg/t3code#11211
* perf(client): reduce remote request and message sync overhead by @Bil0000 in pingdotgg/t3code#11029
* fix(web): refresh usage limit countdowns without switching tabs by @t3-code[bot] in pingdotgg/t3code#11187
* fix(client-runtime): typecheck device hub ticket request on main by @juliusmarminge in pingdotgg/t3code#11304
* feat(settings): add per-project overrides for scopable server settings by @juliusmarminge in pingdotgg/t3code#11176
* feat(web): pick settings environment and project as two selects by @juliusmarminge in pingdotgg/t3code#10636
* feat(settings): edit any scopable setting as a project override by @juliusmarminge in pingdotgg/t3code#10639
* feat(web): float device streams over chat by @juliusmarminge in pingdotgg/t3code#11285
* fix(web): floating preview can use the margins beside the composer by @juliusmarminge in pingdotgg/t3code#11290
* perf(client-runtime): speed up message sync on desktop and mobile by @Bil0000 in pingdotgg/t3code#11302
* fix(web): use the configured panel shortcut on the PR page by @Bil0000 in pingdotgg/t3code#11292
* feat(web): add PR page selections to new draft threads by @Bil0000 in pingdotgg/t3code#11296


**Full Changelog**: pingdotgg/t3code@v0.0.41-nightly.20260911.1551...v0.0.41-nightly.20260911.1564

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.41-nightly.20260911.1564
aorwall added a commit to aorwall/t3code that referenced this pull request Sep 12, 2026
Merges `upstream/main` at `e81606494` into the fork, from merge base
`02297e3db` — 47 upstream commits.

The theme of this range is scopable settings: upstream made every server
setting addressable at a scope (global / environment / project) with
per-project overrides, which is why 11 of the 15 conflicts are settings
files. The rest is conversation rewind, floating device streams, and a
large batch of message-sync and markdown-streaming perf work.

## Merge stats

- Landed (`HEAD^1..HEAD`): 277 files, 17243+/4783−
- Upstream range (base..`HEAD^2`): 275 files, 17011+/4749−
- Fork delta (`HEAD^2..HEAD`): 756 files, 76559+/2096−

The two file lists reconcile: the 3 extra landed files are
`docs/fork/inventory.json`, `docs/fork/upstream-merge-log.md` and
`docs/fork/gaps.md`; the 1 file in the range that did not land is
`apps/web/src/routes/settings.integrations.tsx`, resolved `ours` per the
`moatless-admin-integrations-route` inventory entry (that route is a
Moatless admin page here, and upstream's embedded-surface settings live
at `/settings/browser`).

All 15 conflicts were resolved by the verdict `preflight.mjs` printed.
No `decide` conflict was left unresolved. Details, including the
owned-concern sweep (no keyword hits) and the unsupported-method
reconciliation (0 ADD, 0 DROP, 2 KEEP, 4 known exceptions), are in the
dated entry in `docs/fork/upstream-merge-log.md`.

Two findings worth naming here:

- **A silent auto-merge failure.** pingdotgg#11285 changed the mini-player target
from a tab id to a source union. Git updated upstream's own assertion in
`PreviewView.test.tsx` and left the fork-only "under the frame
capability" case next to it still asserting the old string. No conflict
marker, no `resolution-check.mjs` finding — only the fork's own test
suite caught it.
- **Stale inventory anchors.** Upstream moved the project Actions
section out of `ProjectSettingsPanel.tsx` into a new
`ProjectActionsSettings.tsx`, which is where `scriptsEditable` is now
derived and where upstream's new writing Reset button is gated. Four
inventory entries were re-pointed in this merge rather than silently
dropping their deltas.

## Usable as-is

Client work the fork can expose with no Moatless backend change:

- Scoped settings UI and the two-select scope picker (pingdotgg#10639, pingdotgg#10636) —
`SettingsScopeContext`, `ScopedSwitch`, `settingKeys`, the `mixed`
state. The reading half works against Moatless today.
- Float device streams over chat, as a source union rather than a tab id
(pingdotgg#11285); recording status on floating previews (pingdotgg#11312); floating
preview using composer margins (pingdotgg#11290).
- PR-page selections into new drafts (pingdotgg#11296);
projects-on-another-machine badge (pingdotgg#11323); Usage opening on Limits
(pingdotgg#11261).
- macOS permission onboarding (pingdotgg#11289); hold-to-quit fix (pingdotgg#11016);
preview keystrokes kept out of the composer (pingdotgg#11354).
- Message-sync and markdown-streaming perf: pingdotgg#11302, pingdotgg#11029, pingdotgg#11211,
pingdotgg#11198, pingdotgg#11196, pingdotgg#11193, pingdotgg#11181, pingdotgg#11206.
- Assorted web/mobile fixes: pingdotgg#11361, pingdotgg#10757, pingdotgg#11357, pingdotgg#10571, pingdotgg#11348,
pingdotgg#11349, pingdotgg#11281, pingdotgg#11188, pingdotgg#11283, pingdotgg#11292, pingdotgg#11187, pingdotgg#11228, pingdotgg#11103, pingdotgg#10612,
pingdotgg#11032, pingdotgg#11233, pingdotgg#11234, pingdotgg#11304, pingdotgg#11240.

## Unsupported in Moatless / needs implementation

- **Conversation rewind** — `thread.conversation.revert` (pingdotgg#11358). A new
member of `DispatchableClientOrchestrationCommand` in
`packages/contracts/src/orchestration.ts`, bringing the fork to 30
command types (28 upstream's, 2 fork-only). Moatless does not dispatch
it, and a client command cannot be refused per-type, so "Edit from here"
on `RevertUserMessageButton` is reachable whenever the turn is idle and
does nothing. Needs backend dispatch.
- **Per-project setting overrides** — the `projectSettingsOverrides`
capability and the 17-key `ProjectSettingsOverrides` record (pingdotgg#11176).
Two pieces are needed: the capability reported by
`/.well-known/t3/environment`, and `server.updateSettings` served at
project scope. Until both land, the capability filter in
`scopedSettings.ts:170` and `ProjectActionsSettings.tsx:72` drops the
write on the client — the control renders, the user toggles it, and
**the write never leaves the browser**. A silent no-op is worse than a
hidden control or an honest refusal; recorded in `docs/fork/gaps.md`.
- **Default thread permissions** — `defaultRuntimeMode` (pingdotgg#11346). Reads
fine, cannot be saved. Same `server.updateSettings` write path as above,
one level deeper, not a separate gap.

## Backend behavior to consider reproducing in Moatless

Upstream server-side work the fork cannot use directly, but that
Moatless would benefit from:

- **Queue messages during context compaction** (pingdotgg#11107,
`ProviderCommandReactor.ts`) — a message sent while compaction is in
flight is currently dropped rather than held.
- **Restore provider history and prompts when rewinding** (pingdotgg#11338,
`CheckpointReactor.ts`) — the counterpart to
`thread.conversation.revert` above; rewinding the thread without
rewinding provider state leaves the two out of sync.
- **Detect file renames in review diffs** (pingdotgg#8086,
`apps/server/src/vcs/GitVcsDriverCore.ts`) — a rename currently reads as
a whole-file delete plus a whole-file add.
- **Preserve qualified Codex model ids** (pingdotgg#9921, `ModelManifest.ts` +
`CodexTextGeneration.ts`).
- **Model defaults** astra-medium / fable-5.1-medium (pingdotgg#11347).

All five are recorded under the runtime-fixes entry in
`docs/fork/gaps.md`.

## Verification

`verify.mjs` (full pass): 7 of 8 checks green — `duplicate-adds`,
`tripwires`, `resolution-check`, `unsupported-methods`, `fmt:check`,
`lint`, `typecheck`.

`test` is red on **`@t3tools/desktop` only**, at
`scripts/browser-secret-native.test.mjs > bundled libsecret helper`:
`Command failed: pkg-config --cflags --libs libsecret-1`. This is the
standing sandbox gap, not a merge regression — the test file's last
commit is `498ab9c39` (pingdotgg#7261, before the merge base), `git diff
--name-only` against both merge parents is empty for it, and `pkg-config
--exists libsecret-1` fails in this environment. It is already an entry
in `docs/fork/gaps.md`. Every other package passes, including
`@t3tools/web` (5079 tests) after the `PreviewView.test.tsx` fix above.

Three typecheck failures the merge introduced were fixed in it:
`SETTINGS_CATEGORY_SCOPES` in `settingsSearch.ts` was missing all 9
fork-only settings paths, and two `filterAvailableSettingsSearchItems`
literals in `settingsSearch.test.ts` were missing the fork's
`forgejoEnabled` field.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---
Moatless task:
https://moatless.soaplabstest.com/tasks/e70b41b3-779d-43b8-8f34-7de516548e7c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant