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

[pnupu]

What Changed

Saving one keybinding currently changes the array positions used in other rows' React keys, causing those rows to remount and discard their unsaved edits.

Use the saved command, normalized shortcut, and When expression as each row's stable identity. Show identical resolved bindings as one editable row. Different commands, shortcuts, and conditions remain separate, and the runtime binding list and shortcut precedence are unchanged.

Why

Fixes #11108.

PR #4366 addressed the same bug but was closed without merging. No reason for the closure was stated. It proposed occurrence counters for duplicate bindings, but its review noted that those counters can shift when an earlier duplicate changes or disappears. A single editor per resolved binding avoids that identity reassignment without adding persistent binding IDs or changing the API.

Display grouping does not fix the existing alias-matching limitation: the editor sends normalized shortcut/condition values, while the server matches raw strings for save and remove.

Validation

• All 17 keybinding logic tests pass, including three new regression tests for sibling identity, duplicate changes, and normalized duplicates with distinct commands/conditions.
• All three new regression tests fail against the original implementation.
• Targeted lint and formatting checks pass.
• The web package typecheck passes.

The shared editor covers web and desktop, including remote connections. No mobile editor, provider adapter, wire contract, or documentation changes are required.

UI Changes

Identical resolved bindings now appear as one editable row. Saving another binding preserves unsaved sibling edits.

Verified in Chrome against an isolated checkout and disposable keybindings data. Both clips start with custom Thread: Jump: 1 and 2 bindings whose saved When condition is !modelPickerOpen, with the list filtered to custom. They change both conditions to terminalFocus, then save only the first binding. Before the fix, the second condition resets to !modelPickerOpen; after the fix, it retains terminalFocus and its Save button.

Before

keybindings-before.mp4

After

keybindings-after.mp4

Checklist

• This PR is small and focused
• I explained what changed and why
• I captured before/after screenshots
• I captured before/after interaction videos

Implemented with GPT-6 in Codex. Reviewed with Fable 5.1 in Claude Code and GPT-6 Astra through a Codex subagent.

[macroscopeapp]

Approvability

Verdict: Approved at 75c739e

Macroscope's review found this PR approvable — This is a localized keybinding-settings bug fix that stabilizes row identity and preserves unsaved drafts while leaving server contracts and runtime shortcut execution unchanged. The behavior change is small, self-contained, and covered by focused regression tests.

Notes:

• Diff unchanged. Approvability was decided on eligibility alone.

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

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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.

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

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	⚠️ Warning	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
Title check	✅ Passed	The title clearly and concisely describes the main fix: preserving unsaved keybinding edits when another binding is saved.
Description check	✅ Passed	The description clearly explains the problem, implementation, rationale, linked issue, validation results, and UI evidence. It does not use the template headings exactly and does not explicitly docume…
Linked Issues check	✅ Passed	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	✅ Passed	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-…

• 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!