Skip to content

perf(web): settings search no longer re-normalizes every row on each keystroke - #28757

Open
only21mil wants to merge 3 commits into
pingdotgg:mainfrom
only21mil:fix/audit-m173
Open

only21mil wants to merge 3 commits into
pingdotgg:mainfrom
only21mil:fix/audit-m173

Conversation

@only21mil

@only21mil only21mil commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Settings search re-normalized every row on every search. searchSettings ran normalizeSearchText (NFKD, mark stripping, lowercasing, whitespace collapse) over each row's title, section label and search terms, and translated and normalized the title and section label again. The settings sidebar runs it on every keystroke, so each keystroke redid that work for every row, even though a row's text only changes with the UI language.

Change

searchSettings now reads each row's normalized fields from a module-level WeakMap keyed by the row object, stored with the translate function they were built with. A search with the same translate function reuses them. gt-react hands out a new translate function when the locale or its translations change, and a row's entry is rebuilt then.

  • Keyed by row, not by list. The available-settings filter and the platform filter return subsets of the same row objects, so changed availability still hits the cache. A caller that passes new row objects gets those rows normalized on first use, so a dynamically built list can't read stale text.
  • SettingsSearchItem fields are all readonly, and section labels are a module constant, so a cached entry can't drift from its row.
  • The fields and their order are the same as before, so query normalization, matching, rank, secondary-row ordering and platform filtering are unchanged. The default translate is a module constant, so calls without one share the cache.

Rejected alternatives: precomputing fields when the catalog is built would still need a fallback for the arbitrary items lists searchSettings accepts. Memoizing in the sidebar component would leave the per-row work in the shared function.

Scope and approval

No prior issue or discussion. This is a focused performance fix with no change to product behavior: the same rows appear for every query, in the same order. It touches searchSettings in settingsSearch.ts and its test file. Clients: web and desktop, which share this code. Mobile does not use this search.

Verification

Real browser on a dev server named "Test machine": Chrome 157 (headless), one revision at a time, main cd8fc91a57, the earlier head 892ef6f7b8 and this head 6a87912ff7. Each run typed 8 queries (theme, word wrap, font size, keybindings, worktree, notifications, git fetch, telemetry) into Settings search one key at a time, 73 keystrokes, with keys sent through the debugging port. Call counts come from Chrome's precise coverage and times from its sampling profiler at 20 µs: one count run and three timed runs per revision. It is a dev build, so absolute times run higher than production. The body runs long because it keeps the unedited per-run lines for all three revisions, folded below.

In these logs head2 is 892ef6f7b8 and offer is this head 6a87912ff7. Lines from the run logs, not altered:

main-count: [{"calls":{"searchSettings":1,"normalizeSearchText":1254},"keystrokes":73}]
head2-count: [{"calls":{"searchSettings":1,"normalizeSearchText":471},"keystrokes":72},{"calls":{"searchSettings":1,"normalizeSearchText":1254},"keystrokes":1}]
offer-count: [{"calls":{"searchSettings":1,"normalizeSearchText":1},"keystrokes":72},{"calls":{"searchSettings":1,"normalizeSearchText":1254},"keystrokes":1}]
Per-run times and result lists
main-cd8fc91a57-run1  searchSettings median 2.935 ms mean 3.113 ms | normalizeSearchText median 0.364 ms | main-thread busy median 13.506 ms
main-cd8fc91a57-run2  searchSettings median 2.874 ms mean 3.053 ms | normalizeSearchText median 0.366 ms | main-thread busy median 13.541 ms
main-cd8fc91a57-run3  searchSettings median 3.106 ms mean 3.387 ms | normalizeSearchText median 0.406 ms | main-thread busy median 15.014 ms
head2-892ef6f7b8-run1  searchSettings median 2.788 ms mean 3.096 ms | normalizeSearchText median 0.104 ms | main-thread busy median 14.017 ms
head2-892ef6f7b8-run2  searchSettings median 2.713 ms mean 2.867 ms | normalizeSearchText median 0.147 ms | main-thread busy median 12.604 ms
head2-892ef6f7b8-run3  searchSettings median 2.61 ms mean 2.794 ms | normalizeSearchText median 0.145 ms | main-thread busy median 13.414 ms
offer-6a87912ff7-run1  searchSettings median 0.219 ms mean 0.262 ms | normalizeSearchText median 0 ms | main-thread busy median 9.261 ms
offer-6a87912ff7-run2  searchSettings median 0.165 ms mean 0.232 ms | normalizeSearchText median 0 ms | main-thread busy median 8.615 ms
offer-6a87912ff7-run3  searchSettings median 0.22 ms mean 0.292 ms | normalizeSearchText median 0 ms | main-thread busy median 9.663 ms
main-time-1 {"theme":3,"word wrap":1,"font size":5,"keybindings":90,"worktree":9,"notifications":2,"git fetch":2,"telemetry":2} 48a622c73544b5fb
head2-time-1 {"theme":3,"word wrap":1,"font size":5,"keybindings":90,"worktree":9,"notifications":2,"git fetch":2,"telemetry":2} 48a622c73544b5fb
offer-time-1 {"theme":3,"word wrap":1,"font size":5,"keybindings":90,"worktree":9,"notifications":2,"git fetch":2,"telemetry":2} 48a622c73544b5fb

After the first keystroke fills the cache, a keystroke here normalizes only the query, and the time in searchSettings drops from about 3 ms to about 0.2 ms. All runs returned the same results for all 8 queries.

  • vp test run src/components/settings/settingsSearch.test.ts: 24 passed at 6a87912ff7. The new test translates each row once per language instead of on every keystroke fails at 892ef6f7b8 (translate called 10 times on the second search) and passes here. The English/Spanish case now also checks that going back to English drops the Spanish match.
  • vp run check:changed at 6a87912ff7: format, lint, knip, typecheck, tests and React Compiler checks pass.

UI: no screenshots. The same rows render in the same order.

Not checked:

  • The desktop app and slower hardware; the timings come from the web client in Chrome.
  • A production build.

Prior PRs

takeover verification

Merged main at 2bcbdae06a49, preserving translated search labels alongside the cache. Blacksmith vp run check:changed passed, including 23 settings-search tests. Chrome verified repeated queries, normalization, empty results, and navigation. Desktop shares this search; mobile has no equivalent consumer. Browser timing and the Electron shell were not measured.

settings search queries and result navigation

settings search returns word wrap

Generated with Claude Opus 5.5 and GPT-6.1 Sol in T3 Code Nightly

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Nov 1, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 9e8de2a

Macroscope's review found this PR approvable — Settings search now caches normalized row text instead of repeating that work on each keystroke. The change is limited to in-memory search processing, with no changes to product defaults, availability filtering, or persistent data.

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

@coderabbitai

coderabbitai Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

searchSettings caches normalized search fields for each settings item and translator. It uses a shared identity translator by default. Tests check matching with and without a translator and verify cache reuse.

Changes: Settings search

Layer / File(s) Summary
Cache normalized search fields
apps/web/src/components/settings/settingsSearch.ts, apps/web/src/components/settings/settingsSearch.test.ts
A WeakMap caches normalized titles, section labels, search terms, and translated labels per item and translator. Tests verify that translated queries require a translator and that repeated searches with the same translator reuse cached fields.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Possibly related PRs

  • pingdotgg/t3code#29576: Changes searchSettings to match terminology-formatted search fields and format result titles.

Suggested reviewers: t3dotgg

Merge Risk: Minimal · up to 6a879

This change caches normalized settings search fields to speed up repeated searches. No concrete defect was found, and results are expected to follow language changes because the cache rebuilds when the translator changes.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title names the main change: settings search no longer re-normalizes every row on each keystroke. That matches the WeakMap cache of normalized row fields in searchSettings, which is the core of th…
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.
Description check Passed The description covers the problem, the change, scope, and verification in detail, and it follows the repository template's section structure. It explains why the WeakMap cache keyed by row object is …
Full details: Title check

Explanation

The title names the main change: settings search no longer re-normalizes every row on each keystroke. That matches the WeakMap cache of normalized row fields in searchSettings, which is the core of the changeset. It follows the conventional commit form with a plain-language description and stays concise.

Full details: Description check

Explanation

The description covers the problem, the change, scope, and verification in detail, and it follows the repository template's section structure. It explains why the WeakMap cache keyed by row object is safe, lists the rejected alternatives, and reports benchmark numbers with run logs. Verification names the focused test file, the new test, the check:changed run, and the Chrome measurements. It also states what was not checked, which is the desktop app, slower hardware, and a production build. The template's Verification section asks for focused tests or manual checks. The author supplied those. Two points are weaker. The Scope section gives no linked issue, and it says no prior issue or discussion exists. The template asks for a reason when no issue is linked, and the author gave one: a focused performance fix. The description also has no 'Problem' link to a bug report, and the change is a performance improvement, not a bug fix. The template's Scope section says the project accepts bug fixes, not features. A performance fix is closer to a bug fix than a feature, so this is a minor concern for maintainers rather than a missing item. The description also contains a large folded log block and a takeover section that mixes in notes from a later reviewer. Those parts reduce readability but do not make the description incomplete. The UI section says no screenshots, while a later takeover section adds screenshots and a recording. The template asks for before/after screenshots for UI changes. This change does not alter the UI, so the omission is acceptable. The template's agent line is present at the end.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 892ef6f and 6a87912.

Files selected for processing: 2

  • apps/web/src/components/settings/settingsSearch.test.ts
  • apps/web/src/components/settings/settingsSearch.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

@maria-rcks

maria-rcks commented Nov 12, 2026 •

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

Merged current main without rebasing and resolved the settings-search conflict by keeping cached invariant fields alongside live translated labels. The diff against the merged base remains one file, 18 additions and 4 deletions.

Verified vp run check:changed through blacksmith: format, lint, knip, affected typechecks, 23 settings-search tests, and React Compiler coverage passed. Two independent code reviews found no actionable issues. Chrome verified repeated queries, whitespace/case normalization, no matches, and result navigation. No browser performance claim is made.

At 892ef6f7b81cfba2bc9bb7d652971f466031f200, all 32 reported checks passed and GitHub reports no conflicts. No review-bot findings remain.

settings search queries and result navigation

settings search returns word wrap

The per-row cache now also holds the two translated labels, reused while
the same translate function is passed. gt-react returns a new one when
the locale or its translations change, so a language switch rebuilds the
entry and results follow the new language.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@only21mil

Copy link
Copy Markdown
Contributor Author

Note

Written by Claude Opus 5.5, replying on behalf of Victor

I sent 73 keystrokes through the debugging port to Chrome on a dev server: after the first keystroke fills the cache, each one at 892ef6f7b8 still runs normalizeSearchText 471 times, with per-run medians of 2.6 to 2.8 ms in searchSettings (main: 1,254 calls, 2.9 to 3.1 ms).
fix/audit-m173-offer at 6a87912ff7, on the same fork as this PR, fast-forwards your head with one commit.
It caches the two translated labels with the rest of each row and rebuilds them when the UI language changes, so after the first keystroke a keystroke normalizes only the query: 1 call and per-run medians of 0.17 to 0.22 ms, with the same results for all 8 queries.

@only21mil

Copy link
Copy Markdown
Contributor Author

Note

🤖 Opus 5.5 replying on behalf of Victor.

I pushed 6a87912ff7 onto your head 892ef6f7b8 as a fast-forward: the commit from my earlier comment, unchanged.
Each row's cached fields now include the two translated labels, rebuilt when the translate function changes, plus one test and one assertion.
It merges cleanly with current main. Your section of the description is unchanged; the Verification above it describes this head.

@github-actions github-actions Bot added size:M 30-99 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Nov 12, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants