fix(web): preserve unsaved keybinding edits when saving another binding

[pnupu]

Saving one keybinding shifted the positional React keys of sibling editors, discarding their unsaved edits. Rows now use the saved command, normalized shortcut, and condition as a stable identity. Equivalent resolved duplicates share one editor; runtime shortcut resolution and precedence are unchanged.

Rebuilt against current main’s grouped settings and separate filtering APIs. The existing raw-string alias matching limitation in save/remove is unchanged.

Before — edit both conditions from terminalFocus to !terminalFocus, then save only the first. The second draft resets:

Before interaction recording

After — same data, viewport and actions. The second draft and Save button remain:

After interaction recording

Validation: 29 focused logic tests pass. The original three PR tests were reduced to two regressions, both red against main and green with the fix. Web typecheck, scoped lint, formatting and diff checks pass. Headless Chromium verified draft preservation and saving the retained draft afterward against isolated data. The shared editor covers web and desktop. Native Electron, macOS, Windows, iOS and Android were not run; mobile has no matching editor change.

Original work by @pnupu in #11109. Maintainer takeover: Codex gpt-6-astra tests, Codex gpt-6.1-sol implementation, Claude claude-opus-5-5 and Codex gpt-6-sol reviews, through T3 Code.

[macroscopeapp]

Approvability

Verdict: Approved at f22aa1b

Macroscope's review found this PR approvable — This is a focused keybindings-settings bug fix that stabilizes React row identities so sibling drafts survive saves, with regression coverage for reordered and equivalent bindings. Its impact is confined to the settings UI and does not alter runtime shortcut resolution, persistence contracts, or product defaults.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

• Last reviewed commit: e08271b
• Latest commit, not reviewed: f22aa1b

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: c0869cc9-bccc-4a7c-859b-03ab670a42e4

📥 Commits

Reviewing files that changed from the base of the PR and between 368324c and e08271b.

📒 Files selected for processing (2)

• apps/web/src/components/settings/KeybindingsSettings.logic.test.ts
• apps/web/src/components/settings/KeybindingsSettings.logic.ts

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

📝 Walkthrough

The keybinding row builder now groups bindings by command, shortcut, and condition. Duplicate bindings share one row, and the last matching binding supplies its data. Tests cover stable row IDs, normalized grouping, filtering, and conflict handling.

Changes: Keybinding row identity

Layer / File(s)	Summary
Deduplicate and validate keybinding rows apps/web/src/components/settings/KeybindingsSettings.logic.ts, apps/web/src/components/settings/KeybindingsSettings.logic.test.ts	buildKeybindingRows groups bindings by command, shortcut, and condition instead of assigning position-based IDs. Duplicate entries share one row, and tests check stable identities, normalized grouping, filtering, and conflict handling.

Priority: ⬇️ Low

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

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: bil0000, t3dotgg

Merge Risk: ⚪ Minimal · up to e0827

No actionable regression remains identified for this change; it is ready to merge after normal checks.

### Architecture Summary

Architecture risk: 🔵 Low · up to e0827

The change affects 1 system.

Changed systems: apps/web

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — apps/web (ui) was modified; 2 changed files map to changed impact.

Before / after behavior

• observed — Modified behavior in apps/web/src/components/settings/KeybindingsSettings.logic.test.ts: The shared keybindings import now includes compileResolvedKeybindingsConfig; the default bindings import remains.
• observed — Modified behavior in apps/web/src/components/settings/KeybindingsSettings.logic.test.ts: Adds a test asserting that moving a saved chat.new binding to the end preserves IDs for both terminal.toggle rows while the changed chat.new binding receives a different ID.
• observed — Modified behavior in apps/web/src/components/settings/KeybindingsSettings.logic.test.ts: Adds a test asserting that duplicate identical bindings produce one conflict-free row and that its ID remains stable when duplicates are removed, reordered with a replacement, or multiplied.
• observed — Modified behavior in apps/web/src/components/settings/KeybindingsSettings.logic.test.ts: Adds a test asserting that equivalent normalized shortcuts and conditions group together, while different conditions and commands remain separate; it also checks unique row IDs, the resulting conflict label, and ID consistency when filtering by command.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		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 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Linked Issues check		Issue #11108 requires saving one keybinding to preserve all other unsaved shortcut and When edits, regardless of save order, in the shared editor. buildKeybindingRows now uses command, normalized sh…
Out of Scope Changes check		The changes are limited to keybinding row identity logic and focused regression tests. The implementation and tests directly support issue #11108. No unrelated changes are shown in the supplied whole-…
Title check		The title clearly identifies the main fix: preserving unsaved keybinding edits when another binding is saved.
Description check		The description explains the problem, implementation, scope, regression coverage, manual verification, screenshots, and untested platforms. It does not use the template headings or explicitly document…

Full details: Linked Issues check

Explanation

Issue #11108 requires saving one keybinding to preserve all other unsaved shortcut and When edits, regardless of save order, in the shared editor. buildKeybindingRows now uses command, normalized shortcut, and when expression for row identity instead of array position. Identical resolved bindings share one row, while different commands, shortcuts, and conditions remain separate. The added tests cover sibling identity preservation, duplicate changes, normalized grouping, separation, and filtering. The supplied PR summary states that the shared web and desktop editor, including remote connections, uses this logic.

Full details: Out of Scope Changes check

Explanation

The changes are limited to keybinding row identity logic and focused regression tests. The implementation and tests directly support issue #11108. No unrelated changes are shown in the supplied whole-PR summary.

Full details: Description check

Explanation

The description explains the problem, implementation, scope, regression coverage, manual verification, screenshots, and untested platforms. It does not use the template headings or explicitly document maintainer approval, but it provides sufficient context for this focused fix.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

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

Dismissing prior approval to re-evaluate 75c739e

[maria-rcks]

Note

Written by claude-opus-5-5 on behalf of Maria

Hi! We are cleaning up open PRs, and this one appears to have been created with an older model (gpt-6). If this change is really important, we recommend rebuilding the PR with a newer model if possible.

[maria-rcks]

Note

Written by claude-opus-5-5 on behalf of Maria

Reopening, this was closed by mistake. Sorry for the noise!

[juliusmarminge]

Taking over this one. I will merge current main, preserve sibling editor drafts using semantic row identities, and verify the save flow in the isolated web app before pushing here. I will not merge it.

Dismissing prior approval to re-evaluate f22aa1b