Repository navigation
cmux NEXT Settings: a group title never repeats its page's title (Updates, Advanced) - #28665
Conversation
Settings > Updates draws "Updates" as its page title and again as its first group's title, then "Updates: Advanced". A DOM test over every category page and a Chromium gallery test (pages.settings, variant updates) that writes light and dark stills. Red on base. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-63b9964940c20354b639
Leo, 2026-10-30, on Settings > Updates: "look for wastage". The page
drew "Updates" twice (page title, then its first group's title) and
"Updates: Advanced" for the second group. On a category page a group
whose title is the page's title draws no title, and one that starts with
the page's title and a colon drops that prefix ("Advanced"). Search
results keep each group's whole title. Rows, groups and the schema are
unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-63b9964940c20354b639
|
Found 6 test failures on Blacksmith runners: Failures
|
…ettings-dup-title
|
Review (subagent opus-5.5 medium, from the nightly-land helper): SHIP at e229a25 Follow-ups:
That review is at e229a25. The branch then merged base cleanly (2ad6dd1), and an independent review runs at the new head. |
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
|
Review (codex-review-box/gpt-6-astra high, lane-place): SHIP at 2ad6dd1 SHIP at 2ad6dd1
No blocking code findings. All eight helper cases passed under Node. DOM/browser suites were not run: Bun and dependencies are unavailable. Follow-ups:
|
There was a problem hiding this comment.
4 issues found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="webviews/src/pages/settings/components/GroupList.tsx">
<violation number="1" location="webviews/src/pages/settings/components/GroupList.tsx:25">
P2: The prefix removal uses the lowercased page length as an offset into the original title. Case mappings such as `İ` can change string length, so valid localized `page: group` titles fail to lose their page prefix; keep the trimmed original page text for slicing and use a separate lowercased value only for comparison.</violation>
<violation number="2" location="webviews/src/pages/settings/components/GroupList.tsx:29">
P2: The prefix stripper misses the Khmer colon used by the shipped `updatesAdvanced` translation, so Khmer still renders `Updates`' equivalent twice. Include `៖` in the accepted separators.</violation>
</file>
<file name="webviews/src/pages/settings/groupTitles.test.tsx">
<violation number="1" location="webviews/src/pages/settings/groupTitles.test.tsx:57">
P3: This all-category check is vacuous when a route fails to render the requested category because a missing section becomes an empty title list. Assert the section exists before inspecting its group titles.</violation>
<violation number="2" location="webviews/src/pages/settings/groupTitles.test.tsx:69">
P2: This assertion does not verify that search matched an Updates row: `SearchResults` renders every group heading, including collapsed nonmatches. Assert the matching Updates row and its containing group title so a broken search cannot still pass this test.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| const page = pageTitle.trim().toLocaleLowerCase(); | ||
| const own = title.trim(); | ||
| if (own.toLocaleLowerCase() === page) return ""; | ||
| const head = own.slice(0, page.length).toLocaleLowerCase(); | ||
| const rest = own.slice(page.length).match(/^\s*[::]\s*(.+)$/u); |
There was a problem hiding this comment.
P2: The prefix removal uses the lowercased page length as an offset into the original title. Case mappings such as İ can change string length, so valid localized page: group titles fail to lose their page prefix; keep the trimmed original page text for slicing and use a separate lowercased value only for comparison.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At webviews/src/pages/settings/components/GroupList.tsx, line 25:
<comment>The prefix removal uses the lowercased page length as an offset into the original title. Case mappings such as `İ` can change string length, so valid localized `page: group` titles fail to lose their page prefix; keep the trimmed original page text for slicing and use a separate lowercased value only for comparison.</comment>
<file context>
@@ -14,23 +14,43 @@ const FIRST_FRAME_ROWS = 24;
+ */
+export function shownGroupTitle(title: string, pageTitle?: string): string {
+ if (!pageTitle) return title;
+ const page = pageTitle.trim().toLocaleLowerCase();
+ const own = title.trim();
+ if (own.toLocaleLowerCase() === page) return "";
</file context>
| const page = pageTitle.trim().toLocaleLowerCase(); | |
| const own = title.trim(); | |
| if (own.toLocaleLowerCase() === page) return ""; | |
| const head = own.slice(0, page.length).toLocaleLowerCase(); | |
| const rest = own.slice(page.length).match(/^\s*[::]\s*(.+)$/u); | |
| const pageText = pageTitle.trim(); | |
| const page = pageText.toLocaleLowerCase(); | |
| const own = title.trim(); | |
| if (own.toLocaleLowerCase() === page) return ""; | |
| const head = own.slice(0, pageText.length).toLocaleLowerCase(); | |
| const rest = own.slice(pageText.length).match(/^\s*[::]\s*(.+)$/u); |
| page = await renderPage({ path: "/settings/general" }); | ||
| await changeValue(page.container.querySelector<HTMLInputElement>("[data-settings-search]")!, "update channel"); | ||
| await settle(); | ||
| expect(groupTitles(page.container.querySelector("[data-search-results]"))).toContain("Updates"); |
There was a problem hiding this comment.
P2: This assertion does not verify that search matched an Updates row: SearchResults renders every group heading, including collapsed nonmatches. Assert the matching Updates row and its containing group title so a broken search cannot still pass this test.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At webviews/src/pages/settings/groupTitles.test.tsx, line 69:
<comment>This assertion does not verify that search matched an Updates row: `SearchResults` renders every group heading, including collapsed nonmatches. Assert the matching Updates row and its containing group title so a broken search cannot still pass this test.</comment>
<file context>
@@ -0,0 +1,70 @@
+ page = await renderPage({ path: "/settings/general" });
+ await changeValue(page.container.querySelector<HTMLInputElement>("[data-settings-search]")!, "update channel");
+ await settle();
+ expect(groupTitles(page.container.querySelector("[data-search-results]"))).toContain("Updates");
+});
</file context>
| expect(groupTitles(page.container.querySelector("[data-search-results]"))).toContain("Updates"); | |
| const updatesGroup = page.container.querySelector( | |
| '[data-search-results] [data-section="updates"] [data-group="settings.group.updates"]', | |
| ); | |
| expect(updatesGroup?.querySelector('[data-row-key="updates.channel"]')).not.toBeNull(); | |
| expect(updatesGroup?.querySelector(".group-title")?.textContent).toBe("Updates"); |
| const own = title.trim(); | ||
| if (own.toLocaleLowerCase() === page) return ""; | ||
| const head = own.slice(0, page.length).toLocaleLowerCase(); | ||
| const rest = own.slice(page.length).match(/^\s*[::]\s*(.+)$/u); |
There was a problem hiding this comment.
P2: The prefix stripper misses the Khmer colon used by the shipped updatesAdvanced translation, so Khmer still renders Updates' equivalent twice. Include ៖ in the accepted separators.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At webviews/src/pages/settings/components/GroupList.tsx, line 29:
<comment>The prefix stripper misses the Khmer colon used by the shipped `updatesAdvanced` translation, so Khmer still renders `Updates`' equivalent twice. Include `៖` in the accepted separators.</comment>
<file context>
@@ -14,23 +14,43 @@ const FIRST_FRAME_ROWS = 24;
+ const own = title.trim();
+ if (own.toLocaleLowerCase() === page) return "";
+ const head = own.slice(0, page.length).toLocaleLowerCase();
+ const rest = own.slice(page.length).match(/^\s*[::]\s*(.+)$/u);
+ return head === page && rest ? rest[1] : own;
+}
</file context>
| const rest = own.slice(page.length).match(/^\s*[::]\s*(.+)$/u); | |
| const rest = own.slice(page.length).match(/^\s*[::៖]\s*(.+)$/u); |
| page = await renderPage({ path: `/settings/${category.id}` }); | ||
| await settle(); | ||
| const title = text(category.title).toLocaleLowerCase(); | ||
| const titles = groupTitles(page.container.querySelector(`[data-section="${category.id}"]`)); |
There was a problem hiding this comment.
P3: This all-category check is vacuous when a route fails to render the requested category because a missing section becomes an empty title list. Assert the section exists before inspecting its group titles.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At webviews/src/pages/settings/groupTitles.test.tsx, line 57:
<comment>This all-category check is vacuous when a route fails to render the requested category because a missing section becomes an empty title list. Assert the section exists before inspecting its group titles.</comment>
<file context>
@@ -0,0 +1,70 @@
+ page = await renderPage({ path: `/settings/${category.id}` });
+ await settle();
+ const title = text(category.title).toLocaleLowerCase();
+ const titles = groupTitles(page.container.querySelector(`[data-section="${category.id}"]`));
+ expect({ page: category.id, repeats: titles.filter((t) => t?.toLocaleLowerCase() === title) }).toEqual({
+ page: category.id,
</file context>
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Gallery before/after for
States
Pick other states with a Passes: CI passes on CI passes on Written by red: cmux-next checks (god files, concurrency, crash safety, l10n); no finished base run on fe90490 to compare; do not merge until green; see lane-rules Shipped in:
|
|
Review (next-review patch-id; carried): SHIP at ad7da75 The independent SHIP at 2ad6dd1 carries to this head: the PR's own change ( |
|
Review (next-review patch-id; carried): SHIP at 604787a The independent SHIP at 2ad6dd1 carries to this head: the PR's own change ( |
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Leo, 2026-10-30, on Settings > Updates: "look for wastage".
Settings > Updates drew its title twice: "Updates" as the page title, then "Updates" again over its first group, then "Updates: Advanced" over the second. design/settings.md lists it under Not yet settled ("A group whose title repeats its page's title is a candidate for the same waste rule").
What changes
GroupListshownGroupTitle, the page title passed fromSectionView). Updates is the only page today whose group title repeats its page title. The DOM test checks every category page so a new one is caught.Tests (red at the first commit, green at the head)
webviews/src/pages/settings/groupTitles.test.tsx: the rule's cases; Updates draws ["Advanced"] only; no category page draws a group title equal to its own title; search keeps "Updates".webviews/test/settings-group-titles-browser.test.ts: Chromium on the gallery'spages.settingsvariantupdates. It checks the visible group titles and writes 2x light and dark stills.Stills
Gallery (
pages.settings,updates), Chromium, synthetic data, light and dark: to follow from the cmux-ci runs at the red commit (before) and the head (after).Observed / Not run
Changelog
Changed: Settings > Updates no longer repeats "Updates" over its groups; the advanced group reads "Advanced".
Design spec: leo-workspace design/settings.md (moves its first "Not yet settled" item into Rules in a follow-up).
Agent session: agent session
🤖 Generated with Claude Code