KEY-GRID: refresh native handles when shortcut ownership changes

Include effective accelerators in the shared native menu cache key,
with a regression for same-text direct-to-cycle key transfer.
Document the touched menu helpers.

Co-Authored-By: Codex <noreply@openai.com>

apps/lightcraft/src/native_menu.rs

@@ -16,7 +16,9 @@ use std::collections::{HashMap, HashSet};
 use std::sync::mpsc::{Receiver, channel};
 
 use lightcraft_ui_egui::LightcraftApp;
-use lightcraft_ui_egui::menubar::{MenuNode, menu_bar, native_item_presentation, native_menu_delivers as menu_delivers};
+use lightcraft_ui_egui::menubar::{
+    MenuNode, menu_bar, native_item_presentation, native_menu_delivers as menu_delivers, native_menu_structure as structure_of,
+};
 use muda::accelerator::{Accelerator, Code, Modifiers};
 use muda::{CheckMenuItem, Menu, MenuEvent, MenuItem, PredefinedMenuItem, Submenu};
 use serde_json::Value;
@@ -164,34 +166,6 @@ fn label_text(s: &str) -> String {
     lightcraft_ui_egui::i18n::tr(s).replace('&', "&&")
 }
 
-fn structure_of(bar: &[(String, Vec<MenuNode>)]) -> String {
-    fn walk(n: &[MenuNode], out: &mut String) {
-        for x in n {
-            match x {
-                MenuNode::Item { id, params, checked, shortcut, .. } => {
-                    out.push_str(&MenuNode::key(id, params));
-                    out.push(if checked.is_some() { 'c' } else { 'i' });
-                    out.push_str(shortcut.as_deref().unwrap_or(""));
-                }
-                MenuNode::Separator => out.push('-'),
-                MenuNode::Submenu { label, children } => {
-                    out.push('[');
-                    out.push_str(label);
-                    walk(children, out);
-                    out.push(']');
-                }
-            }
-            out.push(';');
-        }
-    }
-    let mut s = format!("{};", lightcraft_ui_egui::i18n::language().code());
-    for (t, items) in bar {
-        s.push_str(t);
-        walk(items, &mut s);
-    }
-    s
-}
-
 /// Drop a trailing separator (left behind when an item moved to the app menu).
 fn tidy_separators(mut nodes: Vec<MenuNode>) -> Vec<MenuNode> {
     while matches!(nodes.last(), Some(MenuNode::Separator)) {
@@ -215,9 +189,10 @@ impl NativeMenu {
         m
     }
 
+    /// Recreate native handles and shortcut ownership from the current shared menu model.
     fn rebuild(&mut self, app: &mut LightcraftApp) {
         let bar = menu_bar(app);
-        self.structure = structure_of(&bar);
+        self.structure = structure_of(&bar, &app.ui.settings.keymap);
         self.menu = Menu::new();
         self.items.clear();
 
@@ -283,6 +258,7 @@ impl NativeMenu {
         self.publish_shortcuts(app);
     }
 
+    /// Install translated rows with their effective accelerators, preserving hint-only keys.
     fn append_nodes(&mut self, sub: &Submenu, nodes: &[MenuNode], keymap: &lightcraft_ui_egui::shortcuts::Keymap) {
         for n in nodes {
             match n {
@@ -361,10 +337,11 @@ impl NativeMenu {
             }
         }
         let bar = menu_bar(app);
-        if structure_of(&bar) != self.structure {
+        if structure_of(&bar, &app.ui.settings.keymap) != self.structure {
             self.rebuild(app);
             return quit;
         }
+        /// Synchronize live labels and states; accelerator ownership changes already triggered a rebuild.
         fn walk(nodes: &[MenuNode], items: &mut HashMap<String, Item>, keymap: &lightcraft_ui_egui::shortcuts::Keymap) {
             for n in nodes {
                 match n {
@@ -438,7 +415,7 @@ mod tests {
         let mut seen = std::collections::BTreeSet::new();
         for language in Locale::ALL {
             set_language(*language);
-            assert!(seen.insert(structure_of(&bar)), "{language:?} shares another language's menu structure key");
+            assert!(seen.insert(structure_of(&bar, &app.ui.settings.keymap)), "{language:?} shares another language's menu structure key");
         }
         set_language(Locale::ZhHant);
         assert_eq!(label_text("File"), "檔案");

crates/ui-egui/src/menubar.rs

@@ -81,6 +81,41 @@ pub fn native_item_presentation<'a>(
     }
 }
 
+/// Cache key for native menu structure and effective accelerator ownership. A transfer from
+/// a direct grid binding to a borrowed cycle hint must rebuild native handles and republish
+/// their owned shortcuts even when the visible menu model is identical.
+pub fn native_menu_structure(bar: &[(String, Vec<MenuNode>)], keymap: &crate::shortcuts::Keymap) -> String {
+    /// Add stable identities and effective native keys; live checked values stay outside the cache key.
+    fn walk(n: &[MenuNode], out: &mut String, keymap: &crate::shortcuts::Keymap) {
+        for x in n {
+            match x {
+                MenuNode::Item { id, params, label, checked, shortcut, .. } => {
+                    out.push_str(&MenuNode::key(id, params));
+                    out.push(if checked.is_some() { 'c' } else { 'i' });
+                    out.push_str(shortcut.as_deref().unwrap_or(""));
+                    // A key can change owners while both visible hints stay identical.
+                    out.push('|');
+                    out.push_str(native_item_presentation(keymap, id, params, label, shortcut.as_deref()).1.unwrap_or(""));
+                }
+                MenuNode::Separator => out.push('-'),
+                MenuNode::Submenu { label, children } => {
+                    out.push('[');
+                    out.push_str(label);
+                    walk(children, out, keymap);
+                    out.push(']');
+                }
+            }
+            out.push(';');
+        }
+    }
+    let mut s = format!("{};", crate::i18n::language().code());
+    for (t, items) in bar {
+        s.push_str(t);
+        walk(items, &mut s, keymap);
+    }
+    s
+}
+
 /// Top-level menus in order (the macOS app menu is added by the host).
 pub const MENUS: &[&str] = &["File", "Edit", "View", "Photo", "Window", "Help"];
 
@@ -760,6 +795,7 @@ pub(crate) fn expanded(app: &LightcraftApp, name: &str) -> Option<Vec<MenuNode>>
     })
 }
 
+/// Resolve a live menu row, including contextual keys and borrowed grid-cycle hints.
 fn node(app: &LightcraftApp, e: &MenuEntry) -> MenuNode {
     // Shift+P picks and advances in Library; don't advertise it on Presets in those views.
     let shortcut = if e.id == "panel.presets" && crate::shortcuts::library_grid(app) { None } else { e.shortcut.as_deref() };
@@ -1179,6 +1215,22 @@ mod tests {
         }
     }
 
+    /// #2513 review: moving a direct-grid key to the cycle command leaves the visible model
+    /// unchanged, but must invalidate the native cache so its old accelerator is removed.
+    #[test]
+    fn grid_menu_shortcut_hints_invalidate_native_ownership_cache() {
+        let mut a = app();
+        crate::shortcuts::assign(&mut a.ui.settings.keymap, "view.gridToggle", None).unwrap();
+        crate::shortcuts::assign(&mut a.ui.settings.keymap, "view.photoGrid", Some("Cmd+Shift+G")).unwrap();
+        crate::shortcuts::assign(&mut a.ui.settings.keymap, "view.squareGrid", Some("Cmd+Shift+2")).unwrap();
+        let before = menu_bar(&a);
+        let direct = native_menu_structure(&before, &a.ui.settings.keymap);
+        crate::shortcuts::assign(&mut a.ui.settings.keymap, "view.gridToggle", Some("Cmd+Shift+G")).unwrap();
+        let after = menu_bar(&a);
+        assert_eq!(before, after, "visible hint text does not expose the ownership change");
+        assert_ne!(direct, native_menu_structure(&after, &a.ui.settings.keymap), "rebuild removes the stale direct-grid accelerator");
+    }
+
     /// #2500: grid view rows show the separate cycle command's current shortcut.
     #[test]
     fn grid_menu_shortcut_hints_follow_the_cycle_key() {