Diff viewer: Group Files by Kind is a setting, so the toggle sticks

Merged
TE
15d ago
+244-31

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 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, through the whole new-setting checklist:

  • diff.groupByKind in DiffViewerSettingsSchema — a toggle in Settings → General → Diff
    Viewer, with keywords (group, kind, tests, generated, order, sort).
  • Agent-settable with the rest of the diff page's looks: SettingsSchema+AgentPolicy unions
    DiffViewerSettingsSchema.keys whole, so the row is covered by that decision rather than by a
    new one.
  • DiffPrefKey.booleans, so the host accepts the write at all.
  • Titled in all 21 locales in CmuxNextSettings/Localizable.xcstrings (en translated, the rest
    needs_review, as every sibling row in that table is).
  • 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. 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) receives false) 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.ts holds every key of defaultDiffViewerOptions to the
    exported schema, so it reads { key: "groupByKind", value: undefined } until
    schemas/settings/settings-schema.json has the new row. That export is a Mac swift test
    (regenerate-settings-exports.sh); no lane regenerates it by hand, so the cmux-next PR autofix
    generates 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, and
    the 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.json and the source stamp
schemas/settings-export/settings-schema.sources.sha256 — and they come from the sanctioned
producer, not from an editor: the PR autofix run of the previous head
(39660516674, artifact
pr-autofix-patch), which runs scripts/cmux-next/regenerate-settings-exports.sh on a Mac and
rewrites 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 only
trigger is pull_request, so the artifact is all of its output and the lane commits it. The base
merge in this branch touches no CmuxNextSettings source (git diff over that directory and
schemas/, docs/mdm is 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, and
defaults_guard.py counts an added default as a default change, so
cmux-next-defaults-guard.yml holds until a call owner from the base's
.github/defaults-call-owners.json (lawrencecchen, austinywang, teamleaderleo) other than the
author 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 --check over all 272 stamp entries, ran defaults_guard.py check-stamp and check-fresh (both exit 0), checked the row lands in exactly the two places
settings-schema.json keeps a row and is field-for-field its siblings' shape, and confirmed the
two 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:

  • The row's section is appearance, like every other Diff Viewer row, but the page draws that
    whole group under General -- so it is Settings → General → Diff Viewer, which is what this
    description now says.
  • Changing it in Settings does not reach a diff tab that is already open: webviews/src/diff/page.ts
    applies only colors from the cmux.diff.prefs stream. That is true of every diff toggle today
    and 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 this
branch 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


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with 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, and saveViewerPrefs silently drops that refusal. This gives groupByKind its schema row and the full new-setting treatment.

  • Adds diff.groupByKind (Settings → General → Diff Viewer), agent-settable and titled in all 21 locales.
  • Host accepts the key via DiffPrefKey.booleans; the page sanitizes and persists it like the other schema-backed defaults.
  • Regenerates the settings exports (MDM manifests, settings-schema.json, defaults snapshot) and refreshes the export stamp after merging base.
  • Stays off by default — the familiar order is the order the diff arrived in, and grouping is the reader's to enable.