Repository navigation
Diff viewer: Group Files by Kind is a setting, so the toggle sticks - #23653
Conversation
The toolbar's "Group files by kind" reset itself on every reopen. The page persists a display option by sending `diff.<key>` to the host, the host refuses a `diff.*` key it has no schema row for (DiffPrefKey.accepts), and saveViewerPrefs swallows that refusal on purpose -- so the one option with no row silently never persisted. It was kept session-local and off by default for exactly that reason (2026-10-20 review of #22951). This gives it the row: `diff.groupByKind`, a toggle in Settings > Appearance > Diff Viewer, agent-settable with the rest of the diff page's looks (it is in DiffViewerSettingsSchema, which SettingsSchema+AgentPolicy unions whole), in DiffPrefKey.booleans so the host accepts the write, and titled in all 21 locales. On the page, `groupByKind` moves into `defaultDiffViewerOptions` with the other schema-backed defaults, is sanitized and persisted like them, and `setOption` no longer skips it. It stays off by default. The familiar order is the order the diff arrived in, and now that the toggle lasts past the tab, turning it on is the reader's to make; whether it becomes the default is a separate, reviewable change. Red then green: `bun test test/viewer-prefs-host-keys.test.ts` fails its new case against the old host list ("expect(accepted.has(\"groupByKind\")).toBe(true)" receives false) and passes with it. `test/diff-option-defaults.test.ts` holds every key of `defaultDiffViewerOptions` to the exported schema, so it stays red until the Mac settings export regenerates schemas/settings/settings-schema.json for the new row; that commit follows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15cc0-fc9d-7382-af90-51f8726825bd&host=big-red
|
Owned by orphan-sweep on air-blue · session |
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
Passes: CI passes on CI passes on Written by red: cmux-next checks (god files, concurrency, crash safety, l10n); base red: the same failure on the base 5363032; does not block
Gallery before/after for
States
Pick other states with a |
|
Bead |
|
|
|
Found 11 test failures on Blacksmith runners: Failures
...and 1 more test failures. View all on Blacksmith
|
…f-groupbykind-setting
A new setting is two things: the descriptor in `DiffViewerSettingsSchema`, and the exports every other consumer reads from it -- the MDM manifests, the portable `settings-schema.json`, the defaults snapshot, and the source stamp that proves the exports match the sources that produced them. Only the descriptor was in this branch, so `cmux-next generated files` and the defaults guard were red on a tree whose exports did not name `diff.groupByKind`. These six files come from the sanctioned producer, not from an editor: the PR autofix run of this head (39660516674, `pr-autofix-patch`), which runs `scripts/cmux-next/regenerate-settings-exports.sh` on a Mac and then rewrites the defaults snapshot from the exports it produced. That workflow cannot push to a pull request branch -- its push job is gated `github.event_name != 'pull_request'` (cmux-next-pr-autofix.yml:559) while its only trigger is `pull_request` -- so the artifact is the whole of its output and the lane commits it. The base merge in this branch touches no `CmuxNextSettings` source, so the stamp the Mac computed still matches the tree it lands on. The diff is the one key and nothing else: the row in the Diff Viewer group, its plist and JSON manifest entries, its line in the managed-preferences table, its `false` in the defaults snapshot, and the two source hashes that changed (`DiffViewerSettingsSchema.swift` and `CmuxNextSettings/Localizable.xcstrings`). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15cc0-fc9d-7382-af90-51f8726825bd&host=big-red
Default changesThis pull request changes 1 default at
Every change is a new setting that defaults to off, the behavior from before it existed. Regenerate the snapshot with |
|
Check classification on
So the only thing between this and a merge is the defaults call. |
Settings export stamp: base's stamp, with the merged digests for the two sources this branch changes (DiffViewerSettingsSchema.swift and the CmuxNextSettings catalog). The sources base left stale stay stale as on base, so the base autofix still regenerates them after the merge. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15cfc-663f-7650-b9f7-64d198c6b88b&host=Air-Blue
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. |
teamleaderleo
left a comment
There was a problem hiding this comment.
Review (subagent, correctness first): SHIP at 38eab2d
Independent subagent review (fresh context) of the merged head. Nothing blocking.
- Swift:
diff.groupByKindrow matches its siblings (toggle, default false,.appearance);DiffPrefKey.booleansgains the key, which is allaccepts/sanitized/setPrefneed; agent policy picks it up viaDiffViewerSettingsSchema.keys. - Webview round trip:
defaultDiffViewerOptionscarriesgroupByKind: false,sanitizeViewerPrefsaccepts it only as a boolean,App.tsxnow persists every key exceptcollapsed; coherent with base'sApp.tsx/Toolbar.tsxafter the merge. - Exports vs base: only
diff.groupByKindlines plus two stamp lines (DiffViewerSettingsSchema.swift and the CmuxNextSettings catalog, digests verified against HEAD); no base lines reverted. xcstrings: +131/-0, valid JSON, 21 locales like siblings;check-l10n.sh0 errors; localization ratchet exit 0. defaults_guard.py checkexit 0 (a new setting that defaults to off);check-stamp --baseexit 0 (only base-stale sources warned).- Not superseded: base still keeps
groupByKindsession-local. - Fleet compile:
cmux-ci compile-check --ref 38eab2da27c7ac6f1f2f8ec639c3b55f752f6108 --apppassed (job 0ab41537ae982378dc6b3212).
Non-blocking follow-ups: skills/cmux-customization/references/viewer-types.md:14 does not list diff.groupByKind (already omits two other diff keys); palette-eval / palette-ranker corpus fixtures have no entry for the new setting (nothing checks them against the schema).
Settings export stamp: base's stamp, with the merged digests for the two sources this branch changes (DiffViewerSettingsSchema.swift and the CmuxNextSettings catalog). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15cfc-663f-7650-b9f7-64d198c6b88b&host=Air-Blue
teamleaderleo
left a comment
There was a problem hiding this comment.
Review (subagent, correctness first): SHIP at 2dadcf8
Independent subagent review (fresh context) of the head after the second base merge. No findings.
git diff origin/feat-cmux-next HEAD(base 5363032): only this PR's 13 files; outside the stamp, the change lines are identical to the earlier SHIP head 38eab2d. No base lines reverted; xcstrings +131/-0, valid JSON, 21 locales likesettings.diff.expandUnchanged; exports parse (plutil -lintOK) and are purely additive.- Round trip:
DiffPrefKey.booleansgainsgroupByKind(feedsaccepts/sanitized;setPreffinds the new schema row); webview sanitizes, seeds and persists it;defaultDiffViewerOptionsdefault false matches the schema. - Stamp: the two changed entries equal sha256 of those files at HEAD; nothing else differs from base.
defaults_guard.py check-fresh,check-stamp --base,checkall exit 0 (new setting off by default). - Not superseded: base keeps
groupByKindsession-local. - Fleet compile:
cmux-ci compile-check --ref 2dadcf85a297c8fd75235beb9a9cf46353868ff4 --apppassed.
|
Shipped in:
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
What
The toolbar's Group files by kind reset itself on every reopen. The page persists a display
option by sending
diff.<key>to the host; the host refuses adiff.*key it has no schema rowfor (
DiffPrefKey.accepts), andsaveViewerPrefsswallows that refusal on purpose — so the oneoption with no row silently never persisted. It was kept session-local and off by default for
exactly that reason (2026-10-20 review of #22951).
This gives it the row, through the whole new-setting checklist:
diff.groupByKindinDiffViewerSettingsSchema— a toggle in Settings → General → DiffViewer, with keywords (
group,kind,tests,generated,order,sort).SettingsSchema+AgentPolicyunionsDiffViewerSettingsSchema.keyswhole, so the row is covered by that decision rather than by anew one.
DiffPrefKey.booleans, so the host accepts the write at all.CmuxNextSettings/Localizable.xcstrings(entranslated, the restneeds_review, as every sibling row in that table is).groupByKindmoves intodefaultDiffViewerOptionswith the other schema-backeddefaults, is sanitized and persisted like them, and
setOptionno longer skips it.It stays off by default
The familiar order is the order the diff arrived in. Now that the toggle lasts past the tab,
turning the grouping on is the reader's to make; whether it becomes the default is a separate
change with its own argument, not a side effect of giving it a row.
Red then green
bun test test/viewer-prefs-host-keys.test.ts— the new case fails against the old host list(
expect(accepted.has("groupByKind")).toBe(true)receivesfalse) and passes with the change.It asserts both halves, because either one alone is the silent bug again: the host accepts the
key, and the page actually persists it.
test/diff-option-defaults.test.tsholds every key ofdefaultDiffViewerOptionsto theexported schema, so it reads
{ key: "groupByKind", value: undefined }untilschemas/settings/settings-schema.jsonhas the new row. That export is a Macswift test(
regenerate-settings-exports.sh); no lane regenerates it by hand, so the cmux-next PR autofixgenerates it and pushes it to this branch, and that test turns green on the autofix head. If the
autofix does not run, the fleet export step produces the same commit.
Observed / Not run
Ran and saw, on this head:
bun test test/viewer-prefs-host-keys.test.ts test/diff-option-defaults.test.ts— 6 pass, andthe one expected red above (the missing exported row).
webviews/bun run check— exit 0.scripts/web-lint.sh— passed.scripts/cmux-next/prepush-gate.py— no new violations.scripts/cmux-next/check-l10n.sh— 135 tables, 8,030 keys, 21 languages, 0 errors.Only read, not run here: Swift compilation (Linux host) — the fleet compile receipt covers it; and
the settings export, which needs a Mac and is the autofix's job.
The exports, and who has to say yes
A new setting is the descriptor plus everything derived from it. The second commit adds those six
files —
docs/mdm/com.manaflow.cmux.json,.plist,managed-preferences.md,schemas/settings/settings-schema.json,schemas/defaults/cmux-next.jsonand the source stampschemas/settings-export/settings-schema.sources.sha256— and they come from the sanctionedproducer, not from an editor: the PR autofix run of the previous head
(39660516674, artifact
pr-autofix-patch), which runsscripts/cmux-next/regenerate-settings-exports.shon a Mac andrewrites the defaults snapshot from what it produced. That workflow cannot push here: its push job
is gated
github.event_name != 'pull_request'(cmux-next-pr-autofix.yml:559) while its onlytrigger is
pull_request, so the artifact is all of its output and the lane commits it. The basemerge in this branch touches no
CmuxNextSettingssource (git diffover that directory andschemas/,docs/mdmis empty), so the stamp the Mac computed matches the tree it lands on.This PR needs a human. Adding a key adds a line to
schemas/defaults/cmux-next.json, anddefaults_guard.pycounts an added default as a default change, socmux-next-defaults-guard.ymlholds until a call owner from the base's.github/defaults-call-owners.json(lawrencecchen,austinywang,teamleaderleo) other than theauthor approves this head. No label and no agent review clears that; I have not added and will not
add
risk:human-ok. The other three follow-ups of #22951 carry no default and do not wait on this.Capture
The Settings page is a webview that renders one row per schema row, straight from the exported JSON
(
webviews/src/pages/settings/schema.ts:3), so committing the exports is what makes the row appear— and the gallery shows it without a Mac. Base vs this head, live from both builds:
https://cmux-lawrences-mac-mini.tail137216.ts.net:18796/compare/cmux-diff-groupbykind-setting/?entry=pages.settings
(tailnet only.) The Diff Viewer group gains Group Files by Kind, off, below Expand Unchanged
Lines; nothing else on the page moves.
No capture: both surfaces this touches are webviews, where the gallery above is the evidence this
repository asks for, and a Mac tour would add nothing — the diff viewer's own toggle is unchanged
pixel for pixel (only where its value is kept changed), and the native files here draw nothing.
Review, and what it does not do
An independent review passed the whole diff, including the generated files: it recomputed the two
source hashes and ran
sha256sum --checkover all 272 stamp entries, randefaults_guard.py check-stampandcheck-fresh(both exit 0), checked the row lands in exactly the two placessettings-schema.jsonkeeps a row and is field-for-field its siblings' shape, and confirmed thetwo exports with no
diff.*key are rightly absent from the diff. It traced the round trip(toolbar toggle ->
cmux.diff.prefs.set->DiffPrefKey.accepts->SettingsDiffPrefs.setPref->pagePrefs()-> seeded options) and ran the webview suites green.Two of its notes are worth having here:
sectionisappearance, like every other Diff Viewer row, but the page draws thatwhole group under General -- so it is Settings → General → Diff Viewer, which is what this
description now says.
webviews/src/diff/page.tsapplies only
colorsfrom thecmux.diff.prefsstream. That is true of every diff toggle todayand this PR does not change it, but now that the key has a Settings row, a reader can toggle it
there and see nothing until the tab is reopened. A follow-up, not this PR.
The ui-rules test that is red here fails on
webviews/src/pages/apps/AppsPage.tsx, which thisbranch does not touch: a base finding.
Changelog
Fixed: "Group files by kind" in the diff viewer is now a setting (General → Diff Viewer), so turning it on lasts past the tab instead of resetting on every reopen.
🤖 Generated with Claude Code
Agent session: big-red
Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Makes the diff viewer's "Group files by kind" toggle persist past the tab instead of resetting on every reopen.
The toggle never stuck because the host refuses
diff.*keys it has no schema row for, andsaveViewerPrefssilently drops that refusal. This givesgroupByKindits schema row and the full new-setting treatment.diff.groupByKind(Settings → General → Diff Viewer), agent-settable and titled in all 21 locales.DiffPrefKey.booleans; the page sanitizes and persists it like the other schema-backed defaults.settings-schema.json, defaults snapshot) and refreshes the export stamp after merging base.Written for commit 2dadcf8. Summary will update on new commits.