From 80739642ecaab956ba62a7953ccd10e0cc9d4c13 Mon Sep 17 00:00:00 2001 From: guyverino Date: Sun, 6 Sep 2026 21:05:44 +0200 Subject: [PATCH 1/3] feat(strategies): follow the core's strategy order, and keep its folders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two halves of the same story, both of them things moonproto only started synchronizing in 2e67562 and this terminal had no way to represent before. Strategy order. A core's strategy list is an arrangement the operator made in MoonBot, and the library now carries it as the row sequence of a Full snapshot. The tree used to throw it away, sorting folders into a byte-wise alphabet that put Zeta above alpha and every Cyrillic name after every Latin one. A folder now sits where its first strategy sits in the core's own list, taken from the unfiltered list so that typing in the search box cannot rearrange the tree. The operator can also change that order: two footer buttons, Ctrl+Shift+Up/Down, and two context-menu items move the selection inside its folder, and the result goes to the core as a complete id sequence. Alt+Up/Down was avoided on purpose — it ships as "shift the sell order's price", and one chord with two meanings is a trap. Until the core echoes the new order back, the tree draws the sequence that was sent; without that a second press would be computed from the arrangement the first one had already replaced. The same sequence is applied to every other outgoing strategy sync, or a checkbox toggled in the meantime would hand the core back the order the operator had just replaced. Folders. The core keeps a versioned folder tree now, empty folders included, so "new folder" is a real edit rather than a mark that lives until the window closes. Edits travel as intents — add this folder, move that subtree — and the complete desired tree is assembled on the feed thread, where the newest tree is known: the wire form deletes every folder the list omits, so a tree assembled from a window's snapshot turns a create into a silent delete of whatever arrived meanwhile. A rename sends the rewritten tree with the rows, or the emptied old path stays behind as a folder of its own. Two capability flags, not one. `supported` means the core keeps a tree and its empty folders can be drawn; `editable` means every path in that tree would also survive moonproto's validator. They differ on a real account: MoonBot allows a `/` inside a folder name, moonproto's own state then reports the split halves as parent folders, and its validator refuses those — so on such a core every folder edit would be refused whole. The terminal asks once and keeps its local marks instead of failing per command. An empty folder draws neither a caret that opens onto nothing nor a checkbox that covers no strategy, and its tooltip says whether the core keeps it. Known gaps, deliberate: a refused reorder or folder edit reaches only the log, so the arrangement on screen reverts without saying why; the analytics purge still deletes an emptied folder the legacy way; empty folders are hidden while a search is running; their order among themselves is by name, which the protocol leaves unsynchronized and MoonBot orders differently. --- crates/moon-core/src/feed/folder_tree.rs | 185 ++++++++ .../moon-core/src/feed/folder_tree/tests.rs | 151 ++++++ crates/moon-core/src/feed/live/commands.rs | 440 +++++++++++++++++- .../moon-core/src/feed/live/commands/tests.rs | 179 ++++++- crates/moon-core/src/feed/live/convert.rs | 90 ++++ .../moon-core/src/feed/live/convert/tests.rs | 31 ++ crates/moon-core/src/feed/live/mod.rs | 52 ++- crates/moon-core/src/feed/mod.rs | 42 +- crates/moon-core/src/feed/strategy_order.rs | 77 +++ .../src/feed/strategy_order/tests.rs | 56 +++ crates/moon-core/src/feed/types.rs | 10 + .../moon-core/src/feed/types/core_folders.rs | 54 +++ crates/moon-core/src/session/commands.rs | 88 +++- crates/moon-core/src/session/lifecycle.rs | 10 + crates/moon-core/src/session/store.rs | 25 + crates/moon-core/src/session/store/tests.rs | 48 ++ crates/moon-ui-gpui/src/strategies/logic.rs | 135 +++++- .../src/strategies/logic/tests.rs | 68 ++- crates/moon-ui-gpui/src/strategies/mod.rs | 14 + crates/moon-ui-gpui/src/strategies/state.rs | 22 +- .../moon-ui-gpui/src/strategies/tree/cache.rs | 17 +- .../src/strategies/tree/dialogs.rs | 37 +- .../moon-ui-gpui/src/strategies/tree/dnd.rs | 43 +- .../moon-ui-gpui/src/strategies/tree/menu.rs | 36 ++ .../moon-ui-gpui/src/strategies/tree/mod.rs | 11 +- .../moon-ui-gpui/src/strategies/tree/moon.rs | 288 +++++++++++- .../src/strategies/tree/moon/tests.rs | 1 + .../moon-ui-gpui/src/strategies/tree/ops.rs | 99 ++++ .../src/strategies/tree/ops/tests.rs | 87 ++++ .../src/strategies/tree/pane_cache.rs | 74 +++ .../src/strategies/tree/pane_cache/tests.rs | 128 +++++ .../src/strategies/tree/reorder.rs | 345 ++++++++++++++ .../src/strategies/tree/reorder/tests.rs | 50 ++ .../moon-ui-gpui/src/strategies/tree/tests.rs | 1 + crates/moon-ui-gpui/src/strategies/tree/ui.rs | 166 ++++++- locales/strategies.yml | 35 ++ 36 files changed, 3083 insertions(+), 112 deletions(-) create mode 100644 crates/moon-core/src/feed/folder_tree.rs create mode 100644 crates/moon-core/src/feed/folder_tree/tests.rs create mode 100644 crates/moon-core/src/feed/strategy_order.rs create mode 100644 crates/moon-core/src/feed/strategy_order/tests.rs create mode 100644 crates/moon-core/src/feed/types/core_folders.rs create mode 100644 crates/moon-ui-gpui/src/strategies/tree/reorder.rs create mode 100644 crates/moon-ui-gpui/src/strategies/tree/reorder/tests.rs diff --git a/crates/moon-core/src/feed/folder_tree.rs b/crates/moon-core/src/feed/folder_tree.rs new file mode 100644 index 00000000..acabac0b --- /dev/null +++ b/crates/moon-core/src/feed/folder_tree.rs @@ -0,0 +1,185 @@ +//! Edits to a core's folder tree, applied where the newest tree is known. +//! +//! The protocol takes a folder edit as the COMPLETE desired set — the core deletes every empty +//! folder the list omits — so what an edit is worth depends entirely on the list it was built from. +//! Built from a stale one it silently deletes whatever arrived in between, which is exactly the +//! trap moonproto's own guidance names ("building the next edit from an older confirmed snapshot +//! can undo your own pending folder changes", `docs/strats.md`). +//! +//! So the window states its INTENT — add this folder, move that subtree — and the base list is +//! chosen here, on the feed thread, where both the core's confirmed tree and the one this terminal +//! last sent are known. Nothing upstream assembles a complete tree, because nothing upstream can. + +/// Remove one folder and everything under it from a tree. +/// +/// Removal IS omission — the wire form is the complete desired set — so this is the whole of it. A +/// folder still holding a strategy is kept by the core regardless of this list, which is the safety +/// the caller relies on rather than a check made here. +/// +/// Args: +/// paths: The tree to remove from. +/// path: Canonical path of the folder to remove. +/// +/// Returns: +/// The tree without that folder or any of its descendants. +pub fn without(paths: &[String], path: &str) -> Vec { + let folded = fold(path); + let under = format!("{folded}/"); + paths + .iter() + .filter(|seen| { + let seen = fold(seen); + seen != folded && !seen.starts_with(&under) + }) + .cloned() + .collect() +} + +/// Whether every path in a submission would pass moonproto's own folder validation. +/// +/// MIRRORS `validate_strategy_folder_paths` (moonproto `client/active_runtime/handles.rs`), which +/// rejects the WHOLE submission on one bad path. That matters because bundling a folder tree with +/// strategies puts every STRATEGY path through the same check, and this terminal knowingly holds +/// paths it would refuse: MoonBot allows a `/` inside a folder name — `"EMA / ORGANIC"` is one +/// folder there — while the validator splits on every `/` and refuses a segment with surrounding +/// whitespace. Asking first is what keeps a folder tree from turning a working rename into a +/// refusal that moves nothing at all. +/// +/// A hand-kept mirror, like the other cross-crate constants in this workspace: moonproto exposes no +/// validator, and being wrong here costs a skipped folder tree rather than a wrong edit. +/// +/// Args: +/// paths: Every folder path the submission would carry — the tree AND each strategy's own. +/// +/// Returns: +/// Whether moonproto would accept the set. +pub fn sendable<'a>(paths: impl Iterator) -> bool { + let mut folders = std::collections::HashSet::new(); + for path in paths { + if path.len() > usize::from(u8::MAX) || path.contains(['\r', '\n', '\0', '"']) { + return false; + } + let mut prefix = String::new(); + for part in path.split('/').filter(|_| !path.is_empty()) { + if part.is_empty() || part.trim() != part { + return false; + } + if !prefix.is_empty() { + prefix.push('/'); + } + prefix.push_str(part); + folders.insert(prefix.to_lowercase()); + } + } + folders.len() < usize::from(u16::MAX) +} + +/// Fold one path into the form two spellings of the same folder share. +/// +/// Lowercased because the core compares folder paths case-insensitively, and `\` read as `/` +/// because a strategy's stored path may carry either while the tree the core reports uses `/`. Two +/// spellings that fold alike name one folder, and an edit that missed that would look for a folder +/// the core has under a name it does not. +/// +/// Args: +/// path: A folder path in any of those spellings. +/// +/// Returns: +/// Its comparison form. +fn fold(path: &str) -> String { + path.replace('\\', "/").to_lowercase() +} + +/// Add one folder to a tree, or return it unchanged when the tree already holds it. +/// +/// Args: +/// paths: The tree to add to. +/// path: Canonical path of the folder to add. +/// +/// Returns: +/// The tree with that folder present exactly once. +pub fn with_added(paths: &[String], path: &str) -> Vec { + let mut out = paths.to_vec(); + let folded = fold(path); + if !out.iter().any(|seen| fold(seen) == folded) { + out.push(path.to_string()); + } + out +} + +/// Split one path into the segments the CORE reads it as. +/// +/// On every separator, which is the core's own rule for the tree it reports — deliberately NOT the +/// Strategies window's rule, which keeps a `/` with whitespace beside it inside a folder name. The +/// two disagree only for a name the core could never accept an edit to anyway, and on such a core +/// folder editing is switched off whole (`CoreFolders::editable`). +fn segments(path: &str) -> Vec<&str> { + path.split(['/', '\\']) + .filter(|part| !part.is_empty()) + .collect() +} + +/// Rewrite a tree for a subtree that was renamed or moved. +/// +/// The half a strategy edit cannot carry: rewriting the rows' paths moves the strategies, but the +/// OLD folder survives in the tree as a folder of its own, now holding nothing. Only a tree that +/// omits it removes it, and the protocol wants both halves in one snapshot. +/// +/// A path is rewritten when it IS the moved folder or lies under it. "Under" is a segment +/// boundary, never a character prefix — `Research2` is not inside `Research` — and the comparison +/// is case-insensitive because the core's own is; the surviving spelling is the one the caller +/// asked for, since that is what the operator typed, with the untouched tail kept as the core +/// spelled it. +/// +/// Compared segment by segment rather than by slicing the raw path at the length of a folded +/// prefix. That shortcut is wrong twice over: `to_lowercase` does not preserve byte length for +/// every character — `İ` grows — so the offset can land mid-character and panic, on a string this +/// terminal received from a core. +/// +/// Args: +/// paths: The tree to rewrite. +/// old_key: Canonical path of the folder that moved. +/// new_key: Canonical path it moved to. +/// +/// Returns: +/// The complete desired tree, in the order given, with no path listed twice. +pub fn rebase(paths: &[String], old_key: &str, new_key: &str) -> Vec { + let old_parts: Vec = segments(old_key) + .into_iter() + .map(|part| part.to_lowercase()) + .collect(); + if old_parts.is_empty() || fold(old_key) == fold(new_key) { + return paths.to_vec(); + } + let mut out: Vec = Vec::with_capacity(paths.len()); + // A rename onto an existing name merges the two folders, and the tree must say so once: the core + // reads a repeated path as one folder either way, but a list that repeats itself is one nobody + // can check against what they asked for. Through a set rather than a scan of what is already + // out — the tree runs to thousands of paths, and a scan would fold each one per comparison. + let mut seen: std::collections::HashSet = std::collections::HashSet::new(); + for path in paths { + let parts = segments(path); + let under = parts.len() >= old_parts.len() + && parts + .iter() + .zip(&old_parts) + .all(|(part, want)| part.to_lowercase() == *want); + let rebased = match under { + false => path.clone(), + true => { + let tail = &parts[old_parts.len()..]; + match tail.is_empty() { + true => new_key.to_string(), + false => format!("{new_key}/{}", tail.join("/")), + } + } + }; + if seen.insert(fold(&rebased)) { + out.push(rebased); + } + } + out +} + +#[cfg(test)] +mod tests; diff --git a/crates/moon-core/src/feed/folder_tree/tests.rs b/crates/moon-core/src/feed/folder_tree/tests.rs new file mode 100644 index 00000000..260e09a0 --- /dev/null +++ b/crates/moon-core/src/feed/folder_tree/tests.rs @@ -0,0 +1,151 @@ +//! Unit tests for folder-tree edits. + +use super::{rebase, sendable, with_added, without}; + +/// The rewrite covers the folder and everything under it. The tree is submitted as the COMPLETE +/// desired set, so a path left un-rewritten is a folder the core is told to keep at its old name, +/// with the renamed one arriving beside it. +#[test] +fn a_rename_rewrites_the_folder_and_its_whole_subtree() { + let tree = vec![ + "Research".to_string(), + "Research/Deep".to_string(), + "Live".to_string(), + ]; + assert_eq!( + rebase(&tree, "Research", "Archive"), + vec!["Archive", "Archive/Deep", "Live"] + ); +} + +/// "Under" is a segment boundary, never a character prefix. Rewriting a folder that merely starts +/// with the same letters would rename something nobody touched — and, the tree being complete, +/// delete the original by omission in the same breath. +#[test] +fn a_folder_sharing_a_prefix_is_left_alone() { + let tree = vec!["Research".to_string(), "Research2".to_string()]; + assert_eq!( + rebase(&tree, "Research", "Archive"), + vec!["Archive", "Research2"] + ); +} + +/// The core compares folder paths case-insensitively, so the rewrite must too: a tree that kept +/// `RESEARCH/Deep` while renaming `Research` would ask the core to hold one folder under two names, +/// and the one it dropped would be the operator's. +#[test] +fn the_match_ignores_case_while_the_new_spelling_is_kept() { + let tree = vec!["research".to_string(), "RESEARCH/Deep".to_string()]; + assert_eq!( + rebase(&tree, "Research", "Archive"), + vec!["Archive", "Archive/Deep"] + ); +} + +/// Renaming onto a name that already exists merges the two folders; the list must say so once. +#[test] +fn a_rename_onto_an_existing_folder_lists_it_once() { + let tree = vec!["Old".to_string(), "New".to_string()]; + assert_eq!(rebase(&tree, "Old", "New"), vec!["New"]); +} + +/// Nothing to rewrite leaves the tree exactly as it stands, including the case-only rename the +/// protocol states is not a distinct folder-tree change at all. +#[test] +fn a_tree_with_nothing_to_rebase_comes_back_unchanged() { + let tree = vec!["Alpha".to_string(), "Alpha/Deep".to_string()]; + assert_eq!(rebase(&tree, "Alpha", "alpha"), tree); + assert_eq!(rebase(&tree, "", "Beta"), tree); + assert_eq!(rebase(&tree, "Missing", "X"), tree); +} + +/// A move is the same rewrite with a longer destination — one operation, so a drag and a rename +/// cannot drift apart. +#[test] +fn a_move_reparents_the_subtree() { + let tree = vec![ + "Live".to_string(), + "Live/Deep".to_string(), + "Box".to_string(), + ]; + assert_eq!( + rebase(&tree, "Live", "Box/Live"), + vec!["Box/Live", "Box/Live/Deep", "Box"] + ); +} + +/// Adding is idempotent, case-insensitively: the same folder asked for twice is one folder, and a +/// second spelling of it would ask the core to create it again. +#[test] +fn adding_a_folder_the_tree_already_holds_changes_nothing() { + let tree = vec!["Research".to_string()]; + assert_eq!( + with_added(&tree, "Research/New"), + vec!["Research", "Research/New"] + ); + assert_eq!(with_added(&tree, "research"), tree); +} + +/// The mirror of moonproto's validator on the case this terminal actually holds: MoonBot allows a +/// `/` inside a folder name, the validator does not, and it refuses the WHOLE submission on one +/// such path — including the strategy moves bundled with it. A core reporting one of those cannot +/// have its folders edited at all, which is what `CoreFolders::editable` is for. +#[test] +fn a_moonbot_folder_name_containing_a_slash_is_not_sendable() { + assert!(sendable(["Research", "Research/Deep"].into_iter())); + assert!(!sendable(["EMA / ORGANIC"].into_iter())); + // The parent moonproto's own state derives from that name by splitting it. + assert!(!sendable(["EMA "].into_iter())); + assert!(!sendable([" leading"].into_iter())); +} + +/// The validator's remaining rules, so a submission cannot be refused for a reason this mirror does +/// not know about. +#[test] +fn quotes_control_characters_and_overlong_paths_are_not_sendable() { + assert!(!sendable(["say \"no\""].into_iter())); + assert!(!sendable(["two\nlines"].into_iter())); + assert!(!sendable(["a/"].into_iter())); + // 255 BYTES, not characters: a Cyrillic name half that long already exceeds it. + let long = "я".repeat(128); + assert!(!sendable([long.as_str()].into_iter())); + let fits = "я".repeat(127); + assert!(sendable([fits.as_str()].into_iter())); + // An empty path is the root, which is always acceptable. + assert!(sendable([""].into_iter())); +} + +/// Removal is omission, and it takes the whole subtree with it — a descendant left in the desired +/// tree would ask the core to keep the folder it was just told to drop. +#[test] +fn removing_a_folder_takes_its_subtree_and_nothing_else() { + let tree = vec![ + "Research".to_string(), + "Research/Deep".to_string(), + "Research2".to_string(), + "Live".to_string(), + ]; + assert_eq!(without(&tree, "Research"), vec!["Research2", "Live"]); + assert_eq!(without(&tree, "Missing"), tree); +} + +/// The two spellings of one folder fold together everywhere: a path stored with `\` names the same +/// folder as one with `/`, and the core's own compare ignores case. An edit that missed either +/// would look for a folder the core has under a name it does not. +#[test] +fn one_folder_spelled_two_ways_is_one_folder() { + let tree = vec![r"Deep\Inner".to_string()]; + assert_eq!(without(&tree, "deep/inner"), Vec::::new()); + assert_eq!(with_added(&tree, "DEEP/inner"), tree); + assert_eq!(rebase(&tree, "deep", "Box"), vec!["Box/Inner"]); +} + +/// A folder name whose lowercase form is LONGER than the name itself. `İ` (U+0130) lowercases to +/// two characters, so a rewrite that sliced the raw path at the folded prefix's byte length would +/// cut mid-character and panic — on a string this terminal received from a core. +#[test] +fn a_name_that_changes_length_when_folded_is_still_rebased() { + let tree = vec!["İ".to_string(), "İ/Deep".to_string(), "Other".to_string()]; + assert_eq!(rebase(&tree, "İ", "Box"), vec!["Box", "Box/Deep", "Other"]); + assert_eq!(without(&tree, "İ"), vec!["Other"]); +} diff --git a/crates/moon-core/src/feed/live/commands.rs b/crates/moon-core/src/feed/live/commands.rs index 58eacd23..df7112c9 100644 --- a/crates/moon-core/src/feed/live/commands.rs +++ b/crates/moon-core/src/feed/live/commands.rs @@ -107,17 +107,128 @@ fn strategy_placements_unchanged( /// changes: a conditional delete is allowed only when both views match the caller's evidence. pub(super) struct StrategyPlacementGuard { queued_sync: Option>, + queued_order: Option, + queued_folders: Option, +} + +/// The strategy sequence the last accepted sync carried, and the confirmed order it was built on. +/// +/// Kept because `strats.snapshots()` is the CORE-CONFIRMED order and moonproto rewrites it only +/// from the core's own Full echo. Between a reorder and that echo, every other strategy command — +/// a checkbox, a field edit, a move — rebuilds the outgoing list from the confirmed order and would +/// hand the core back the arrangement the operator had just replaced. So the queued sequence is +/// applied to every outgoing list until the core has answered. +/// The folder tree the last accepted folder edit carried, and the confirmed tree it was built on. +/// +/// The same shape as [`QueuedOrder`] and for the same reason: `folder_paths()` is what the CORE has +/// confirmed, and between an edit and its echo a second edit built on that list would drop the +/// first. The version is the folder tree's own — moonproto advances it only when the core publishes +/// one — so once it moves, the core has spoken and this is retired. +struct QueuedFolders { + /// Paths in the tree last sent. + paths: Vec, + /// `StratsState::folders_last_modified` at that moment. + base_modified: i64, +} + +struct QueuedOrder { + /// Strategy ids in the order last sent. + ids: Vec, + /// `StratsState::last_modified` at that moment: the version moonproto advances ONLY in + /// `apply_server_order` — that is, only when the core publishes a full snapshot. Once it moves, + /// the core has ruled, whether it accepted this order or overruled it, and re-asserting ours + /// past that point would be an argument with no end. + /// + /// KNOWN LIMIT: that version belongs to the snapshot, not to the order, and moonproto exposes + /// no order-specific one. So an unrelated full snapshot racing the send retires the sequence + /// before the core has applied it, and the reorder is lost — the window goes on drawing it + /// until its own confirmation window closes. The alternative, ignoring the version, is the + /// endless argument above. + base_modified: u64, } impl StrategyPlacementGuard { /// Create an empty guard before the feed thread has queued any full-list synchronization. pub(super) fn new() -> Self { - Self { queued_sync: None } + Self { + queued_sync: None, + queued_order: None, + queued_folders: None, + } } - /// Remember the placements in a full-list synchronization accepted by MoonProto's queue. - fn note_queued_sync(&mut self, placements: Vec<(u64, String)>) { + /// The newest folder tree this terminal knows: the one it last sent, or the core's own. + /// + /// Args: + /// confirmed: The core's confirmed tree. + /// last_modified: That tree's version. + /// + /// Returns: + /// The base a folder edit must be applied to. + fn folder_base(&mut self, confirmed: Vec, last_modified: i64) -> Vec { + if self + .queued_folders + .as_ref() + .is_some_and(|queued| queued.base_modified != last_modified) + { + self.queued_folders = None; + } + match &self.queued_folders { + Some(queued) => queued.paths.clone(), + None => confirmed, + } + } + + /// Remember a folder tree accepted by MoonProto's queue. + fn note_queued_folders(&mut self, paths: Vec, base_modified: i64) { + self.queued_folders = Some(QueuedFolders { + paths, + base_modified, + }); + } + + /// Remember what a full-list synchronization accepted by MoonProto's queue carried. + /// + /// Args: + /// placements: `(strategy id, raw folder path)` for every row in that list. + /// order: The same rows' ids, in the sequence they were sent. + /// base_modified: The confirmed order's version at the moment of sending. + fn note_queued_sync( + &mut self, + placements: Vec<(u64, String)>, + order: Vec, + base_modified: u64, + ) { self.queued_sync = Some(placements); + self.queued_order = Some(QueuedOrder { + ids: order, + base_modified, + }); + } + + /// The sequence still owed to the core, or `None` once the core has published its own. + /// + /// Args: + /// last_modified: The confirmed order's current version. + /// + /// Returns: + /// Ids in the order last sent, while that send is still the newest word on the subject. + fn pending_order(&mut self, last_modified: u64) -> Option<&[u64]> { + if self + .queued_order + .as_ref() + .is_some_and(|queued| queued.base_modified != last_modified) + { + self.queued_order = None; + } + self.queued_order + .as_ref() + .map(|queued| queued.ids.as_slice()) + } + + /// Drop the queued sequence, once something has established that the core no longer owes it. + fn retire_order(&mut self) { + self.queued_order = None; } /// Return whether live and still-pending placement views both match the caller's snapshot. @@ -300,6 +411,9 @@ impl LocalStratEdits { /// dropping a timed-out desired value here would convert a lost echo into a real revert or a /// real disappearance. /// +/// Returns the list, and whether the queued order it was given has been satisfied and can be +/// retired. +/// /// Appended entries are sorted by `(submitted_at, strategy_id)` because `strategy_edits()` is a /// `HashMap` iterator with no stable order — an unsorted append would make the outgoing list /// order vary between runs. @@ -307,7 +421,10 @@ impl LocalStratEdits { /// One accepted side effect: re-staging resets `submitted_at` and `deadline` for every still- /// open edit, so an unrelated edit EXTENDS another's 45 s confirmation window. It can only ever /// extend, never cause a false `TimedOut`. It is not fixed here — the fix belongs upstream. -fn overlay_pending_edits(strats: &StratsState) -> Vec { +fn overlay_pending_edits( + strats: &StratsState, + order: Option<&[u64]>, +) -> (Vec, bool) { let mut full: Vec = strats .snapshots() .map( @@ -326,7 +443,185 @@ fn overlay_pending_edits(strats: &StratsState) -> Vec { unconfirmed.sort_by_key(|(submitted_at, snapshot)| (*submitted_at, snapshot.strategy_id)); full.extend(unconfirmed.into_iter().map(|(_, snapshot)| snapshot)); - full + // The order this terminal last sent and the core has not answered yet. Without it every command + // here would rebuild the list in the CONFIRMED order and quietly undo a reorder still in + // flight — see [`QueuedOrder`]. + if let Some(order) = order { + let ranks: std::collections::HashMap = order + .iter() + .enumerate() + .map(|(rank, id)| (*id, rank)) + .collect(); + if crate::feed::strategy_order::resequence(&mut full, |sc| { + ranks.get(&sc.strategy_id).copied() + }) == 0 + { + // The confirmed list already holds this sequence, so there is nothing left to owe. The + // second retirement rule, and the one that covers a core whose Full carries no order + // version at all: `last_modified` then never moves, and the version test alone would + // keep re-asserting a sequence the core had already applied. + return (full, true); + } + } + + (full, false) +} + +/// Apply one folder-tree edit and send it, choosing the base and refusing what cannot be sent. +/// +/// The base is the newest tree this terminal knows: the one it last sent while the core has not +/// answered, otherwise the core's confirmed one. That is the whole reason folder edits arrive here +/// as intents — a tree assembled upstream is assembled from a snapshot that may already be stale, +/// and the wire form deletes every folder it omits. +/// +/// Refuses to send a tree moonproto would reject rather than discovering it as an error: some real +/// MoonBot folder names cannot survive its validator at all (see [`crate::feed::CoreFolders`]), and +/// on such a core a submission would be refused whole. +/// +/// Args: +/// client: The core's client. +/// server_id: Core id, for the log. +/// action: Log label. +/// strategy_placements: Guard holding the tree this terminal last sent. +/// edit: Rewrites the base into the desired tree. +/// +/// Returns: +/// Nothing; every refusal is logged where it happens. +fn folder_edit( + client: &MoonClient, + server_id: u64, + action: &str, + subject: &str, + strategy_placements: &mut StrategyPlacementGuard, + edit: impl FnOnce(&[String]) -> Vec, +) { + let Some(snap) = client.snapshot() else { + log::warn!( + "core {} {action} folder {subject:?} skipped: strategy state is not ready", + crate::feed::core_label(server_id) + ); + return; + }; + let strats = snap.strats(); + let last_modified = strats.folders_last_modified(); + if last_modified == 0 { + log::info!( + "core {} {action} folder {subject:?} skipped: this core keeps no folder tree", + crate::feed::core_label(server_id) + ); + return; + } + let confirmed: Vec = strats.folder_paths().map(str::to_string).collect(); + let base = strategy_placements.folder_base(confirmed, last_modified); + let desired = edit(&base); + // Validated the way moonproto validates it: a folder submission carries the tree, and the + // library checks every current strategy path alongside it. + let rows = strats.snapshots().map(|sc| sc.path.as_ref()); + if !crate::feed::folder_tree::sendable(desired.iter().map(String::as_str).chain(rows)) { + log::warn!( + "core {} {action} folder {subject:?} skipped: this core's tree holds a path MoonProto refuses", + crate::feed::core_label(server_id) + ); + return; + } + let count = desired.len(); + match client.strategies().sync_local_folders(desired.clone()) { + Ok(()) => { + strategy_placements.note_queued_folders(desired, last_modified); + log::info!( + "core {} {action} folder {subject:?}, {count} in the tree", + crate::feed::core_label(server_id) + ); + } + Err(error) => log::warn!( + "core {} {action} folder {subject:?} failed: {error}", + crate::feed::core_label(server_id) + ), + } +} + +/// Join every relocated row to the run its destination folder ALREADY occupies. +/// +/// Called after a move has rewritten `path`, so a folder the operator dropped rows into stays one +/// contiguous group rather than two — the core asks for that (moonproto `docs/strats.md`, "Strategy +/// Order"), and the tree places a folder where its first strategy appears, so a row left at its old +/// index can drag a whole folder to a new place from a gesture that named neither. +/// +/// Three rules keep it from moving anything it was not asked to: +/// +/// * Only rows whose path actually CHANGED are considered. A drag that includes rows already in +/// the destination leaves those exactly where they are. +/// * The anchor is a row that was NOT part of this move. So a folder RENAME — where every row +/// carrying the new name is one of the renamed ones — relocates nothing at all, and neither +/// does a move into a folder that does not exist yet. +/// * Rows joining the same run are placed in the order they were given, one after another. +/// +/// Built as one rebuilding pass rather than a sequence of `remove`/`insert` calls. That is not a +/// matter of cost: every removal shifts every later index, so a plan expressed in positions goes +/// stale the moment two destinations interleave — which `ops::move_folder` and `ops::rename_folder` +/// both produce — and the rows then land one slot early, splitting the very runs this repairs. +/// Positions here are only ever read from the ORIGINAL list, and each row is emitted exactly once. +/// +/// Args: +/// full: The complete strategy set, already carrying the new paths. +/// relocated: `(strategy id, new folder path)` for the rows whose folder actually changed. +/// +/// Returns: +/// Nothing; a row with no existing destination run to join is left untouched. +fn regroup_moved(full: &mut Vec, relocated: &[(u64, String)]) { + let moved: std::collections::HashSet = relocated.iter().map(|(id, _)| *id).collect(); + let index_of: std::collections::HashMap = full + .iter() + .enumerate() + .map(|(at, sc)| (sc.strategy_id, at)) + .collect(); + + // Per anchor row, the ids that follow it. The anchor is the LAST row of that destination this + // move did not touch; without one there is no run to join and the row is left alone. + let mut following: std::collections::HashMap> = + std::collections::HashMap::new(); + let mut joining: std::collections::HashSet = std::collections::HashSet::new(); + for (id, path) in relocated { + if !index_of.contains_key(id) { + continue; + } + let anchor = full + .iter() + .rposition(|sc| sc.path.as_ref() == path && !moved.contains(&sc.strategy_id)); + if let Some(anchor) = anchor { + following.entry(anchor).or_default().push(*id); + joining.insert(*id); + } + } + if joining.is_empty() { + return; + } + + let mut slots: Vec> = + std::mem::take(full).into_iter().map(Some).collect(); + let mut rebuilt: Vec = Vec::with_capacity(slots.len()); + for at in 0..slots.len() { + let Some(id) = slots[at].as_ref().map(|sc| sc.strategy_id) else { + continue; + }; + if joining.contains(&id) { + // Emitted behind its anchor instead, wherever that sits. + continue; + } + let Some(row) = slots[at].take() else { + continue; + }; + rebuilt.push(row); + let Some(ids) = following.get(&at) else { + continue; + }; + for id in ids { + if let Some(row) = index_of.get(id).and_then(|from| slots[*from].take()) { + rebuilt.push(row); + } + } + } + *full = rebuilt; } /// Shared strategy-sync path: load the COMPLETE current set, let `build` edit it (patch fields, @@ -342,22 +637,43 @@ fn rebuild_sync( server_id: u64, action: &str, strategy_placements: &mut StrategyPlacementGuard, + folders: Option>, build: impl FnOnce(&mut Vec, Option<&StrategySchema>, u64) -> usize, ) -> bool { if let Some(snap) = client.snapshot() { let strats = snap.strats(); let schema = strats.strategy_schema(); let now = now_ms() as u64; - let mut full: Vec = overlay_pending_edits(strats); + // Read before the list is built: it is both the baseline the queued order is judged against + // and the one recorded with the next send. + let last_modified = strats.last_modified(); + let (mut full, order_satisfied) = + overlay_pending_edits(strats, strategy_placements.pending_order(last_modified)); + if order_satisfied { + strategy_placements.retire_order(); + } let changed = build(&mut full, schema, now); - if changed > 0 { + // A folder tree is worth a snapshot on its own: a rename whose rows all vanished between + // queueing and here still has to take the emptied folder with it. + if changed > 0 || folders.is_some() { let placements = full .iter() .map(|strategy| (strategy.strategy_id, strategy.path.to_string())) .collect(); - match client.strategies().sync_local_strategies(full) { + let sequence: Vec = full.iter().map(|strategy| strategy.strategy_id).collect(); + // One snapshot for both when a folder tree comes along: the core applies the + // strategy changes first and the newer tree second, which is what removes a folder the + // strategies have just left. Sent as two commands they could arrive the other way + // round, and the core would then refuse to drop a folder that still held rows. + let queued = match folders { + Some(paths) => client + .strategies() + .sync_local_strategies_with_folders(full, paths), + None => client.strategies().sync_local_strategies(full), + }; + match queued { Ok(()) => { - strategy_placements.note_queued_sync(placements); + strategy_placements.note_queued_sync(placements, sequence, last_modified); log::info!( "core {} {action} {changed} strategies", crate::feed::core_label(server_id) @@ -467,6 +783,8 @@ pub(super) fn drain_commands( server.id, "edit", strategy_placements, + // No folder tree: these edit rows, never the set of folders. + None, |full, schema, now| { let mut edited = 0usize; for sc in full.iter_mut() { @@ -632,6 +950,8 @@ pub(super) fn drain_commands( server.id, "create", strategy_placements, + // No folder tree: these edit rows, never the set of folders. + None, |full, schema, now| { let mut next_id = full.iter().map(|s| s.strategy_id).max().unwrap_or(0) + 1; // Plan the whole batch before insertion because each insertion shifts every @@ -681,6 +1001,8 @@ pub(super) fn drain_commands( server.id, "restore", strategy_placements, + // No folder tree: these edit rows, never the set of folders. + None, |full, schema, now| { // It is already live (double-click in the menu or an echo), so do not duplicate it. if full.iter().any(|s| s.strategy_id == id) { @@ -708,28 +1030,122 @@ pub(super) fn drain_commands( local_strat_edits.mark(id); } } - Ok(CoreCmd::MoveStrategies { moves }) => { + Ok(CoreCmd::MoveStrategies { moves, rebase }) => { + // The folder half, built HERE from the newest tree this terminal knows. Declined + // whole — leaving the strategies to travel alone — when the result is something + // MoonProto would refuse, because it validates the bundle as one: a tree it will + // not take would otherwise turn a working rename into a refusal that moves nothing. + let planned = rebase.and_then(|(old_key, new_key)| { + let snap = client.snapshot()?; + let strats = snap.strats(); + let last_modified = strats.folders_last_modified(); + if last_modified == 0 { + return None; + } + let confirmed: Vec = + strats.folder_paths().map(str::to_string).collect(); + let base = strategy_placements.folder_base(confirmed, last_modified); + let desired = crate::feed::folder_tree::rebase(&base, &old_key, &new_key); + // Every path the submission carries goes through the same validator as the + // tree — the folders, the rows as they stand, and the paths this move is about + // to give them, which is where a name the operator just typed shows up. + let rows = strats.snapshots().map(|sc| sc.path.as_ref()); + let targets = moves.iter().map(|(_, path)| path.as_str()); + let sendable = crate::feed::folder_tree::sendable( + desired + .iter() + .map(String::as_str) + .chain(rows) + .chain(targets), + ); + if !sendable { + log::warn!( + "core {} move {old_key:?} -> {new_key:?}: folder tree left out, a path MoonProto refuses", + crate::feed::core_label(server.id) + ); + return None; + } + Some((desired, last_modified)) + }); + let folders = planned.as_ref().map(|(desired, _)| desired.clone()); // Change `path` and increment `last_date` for the selected strategies in one sync. - // Nothing is recorded on the strength of this send, so its answer is not needed. - let _ = rebuild_sync( + let queued = rebuild_sync( client, server.id, "move", strategy_placements, + folders, |full, _schema, now| { let mut changed = 0usize; + let mut relocated: Vec<(u64, String)> = Vec::new(); for sc in full.iter_mut() { if let Some((_, new_path)) = moves.iter().find(|(id, _)| *id == sc.strategy_id) { + if sc.path.as_ref() != new_path.as_str() { + relocated.push((sc.strategy_id, new_path.clone())); + } sc.path = new_path.as_str().into(); sc.last_date = now.max(sc.last_date + 1); changed += 1; } } + regroup_moved(full, &relocated); changed }, ); + // Recorded only once the queue has taken it. A tree noted before the send would + // become the base of the NEXT folder edit while the core never received it. + if let (true, Some((desired, base))) = (queued, planned) { + strategy_placements.note_queued_folders(desired, base); + } + } + Ok(CoreCmd::AddFolder { path }) => { + folder_edit( + client, + server.id, + "add", + &path, + strategy_placements, + |base| crate::feed::folder_tree::with_added(base, &path), + ); + } + Ok(CoreCmd::RemoveFolder { path }) => { + folder_edit( + client, + server.id, + "remove", + &path, + strategy_placements, + |base| crate::feed::folder_tree::without(base, &path), + ); + } + Ok(CoreCmd::ReorderStrategies { order }) => { + // The new SEQUENCE is the whole edit: no field is patched and no `last_date` moves, + // because moonproto versions strategy order separately from per-strategy edit + // dates and reads the order off the row sequence of the Full snapshot this sends. + let _ = rebuild_sync( + client, + server.id, + "reorder", + strategy_placements, + None, + |full, _schema, _now| { + let ranks: std::collections::HashMap = order + .iter() + .enumerate() + .map(|(rank, id)| (*id, rank)) + .collect(); + // Counted against the list as this terminal last left it — `full` + // arrives already carrying any order still owed to the core — so pressing + // Down and then Up inside one round trip is seen for what it is: a real + // change back, rather than a no-op against a confirmed order the core is + // no longer holding. + crate::feed::strategy_order::resequence(full, |sc| { + ranks.get(&sc.strategy_id).copied() + }) + }, + ); } Ok(CoreCmd::TransferAsset { asset, diff --git a/crates/moon-core/src/feed/live/commands/tests.rs b/crates/moon-core/src/feed/live/commands/tests.rs index 714de79c..57c7deae 100644 --- a/crates/moon-core/src/feed/live/commands/tests.rs +++ b/crates/moon-core/src/feed/live/commands/tests.rs @@ -1,7 +1,10 @@ //! Placement of newly created strategies plus snapshot guards for destructive strategy commands. +use moonproto::StrategySnapshot; + use super::{ - StrategyPlacementGuard, anchor_on_core, plan_insert_positions, strategy_placements_unchanged, + StrategyPlacementGuard, anchor_on_core, plan_insert_positions, regroup_moved, + strategy_placements_unchanged, }; /// An anchor is honoured only on the core it names. @@ -127,7 +130,7 @@ fn conditional_deletes_require_live_and_queued_placements_to_agree() { let original = vec![(1, "alpha".to_string())]; let moved = vec![(1, "beta".to_string())]; let mut guard = StrategyPlacementGuard::new(); - guard.note_queued_sync(moved.clone()); + guard.note_queued_sync(moved.clone(), vec![1], 0); assert!(!guard.allows_snapshot(Some(original.clone()), original)); assert!(guard.allows_snapshot(Some(moved.clone()), moved.clone())); @@ -178,7 +181,7 @@ fn every_full_list_sync_updates_the_placement_shadow() { .find("client.strategies().sync_local_strategies(full)") .expect("the rebuild path must queue its full list"); let shadow = body - .find("strategy_placements.note_queued_sync(placements)") + .find("strategy_placements.note_queued_sync(placements,") .expect("accepted full-list syncs must update the synchronous shadow"); assert!( @@ -275,3 +278,173 @@ fn only_an_applied_edit_claims_local_origin() { "the claim must follow the rebuild that decides what was actually edited" ); } + +/// `StrategyPlacementGuard::pending_order`: a reorder is owed to the core until the core itself +/// publishes an order. Every other strategy command in that window rebuilds its outgoing list from +/// the CONFIRMED order, so without this the next checkbox or field edit would hand the core back +/// the arrangement the operator had just replaced. +#[test] +fn a_queued_order_is_owed_to_the_core_until_it_publishes_one() { + let mut guard = StrategyPlacementGuard::new(); + assert_eq!(guard.pending_order(7), None); + + guard.note_queued_sync(vec![(1, String::new())], vec![3, 1, 2], 7); + // The confirmed order has not moved, so this terminal's sequence is still the newest word. + assert_eq!(guard.pending_order(7), Some([3, 1, 2].as_slice())); + assert_eq!(guard.pending_order(7), Some([3, 1, 2].as_slice())); +} + +/// The other half of the same rule, and the one that makes it terminate: once the core has +/// published an order — accepting ours or overruling it — the queued sequence is dropped. Kept, it +/// would be re-asserted on every later sync forever, against a core that had already answered. +#[test] +fn the_cores_own_published_order_retires_the_queued_one() { + let mut guard = StrategyPlacementGuard::new(); + guard.note_queued_sync(vec![(1, String::new())], vec![3, 1, 2], 7); + assert_eq!(guard.pending_order(9), None); + // ... and it stays retired, including for a later call that repeats the old version. + assert_eq!(guard.pending_order(7), None); +} + +/// Builds a snapshot carrying only what [`regroup_moved`] reads: its id and its folder path. +fn placed(id: u64, path: &str) -> StrategySnapshot { + StrategySnapshot::new( + id, + 1, + 0, + false, + moonproto::StrategyKind::from_ordinal(0), + path, + Default::default(), + ) +} + +/// Names the folder of each row in order, which is what the contiguity rule is about. +fn paths(full: &[StrategySnapshot]) -> Vec<(u64, String)> { + full.iter() + .map(|sc| (sc.strategy_id, sc.path.to_string())) + .collect() +} + +/// A drag into a folder that already holds strategies joins that folder's run, so the destination +/// stays ONE group. Left split, the tree — which places a folder where its first strategy appears — +/// can hoist that whole folder somewhere nobody asked for. +#[test] +fn a_move_joins_the_run_its_destination_already_occupies() { + let mut full = vec![ + placed(1, "X"), + placed(2, "X"), + placed(3, "Z"), + placed(4, "X"), + ]; + // 4 was relabelled to X by the caller and now has to reach it. + regroup_moved(&mut full, &[(4, "X".to_string())]); + assert_eq!( + paths(&full), + vec![ + (1, "X".into()), + (2, "X".into()), + (4, "X".into()), + (3, "Z".into()) + ] + ); +} + +/// Several rows moved at once queue up behind each other in the order they were given, instead of +/// all taking the same slot — which would reverse them — or anchoring on each other and splitting +/// the run they are trying to join. +#[test] +fn rows_moved_together_land_in_the_order_they_were_given() { + let mut full = vec![ + placed(1, "X"), + placed(2, "X"), + placed(3, "Z"), + placed(4, "X"), + ]; + regroup_moved(&mut full, &[(2, "X".to_string()), (4, "X".to_string())]); + assert_eq!( + paths(&full), + vec![ + (1, "X".into()), + (2, "X".into()), + (4, "X".into()), + (3, "Z".into()) + ] + ); +} + +/// A folder RENAME reaches this through the same command, and it must move nothing: every row +/// carrying the new name is one of the renamed ones, so there is no existing run to join. Relocating +/// on a rename would silently change the folder's place in the tree. +#[test] +fn a_rename_relocates_nothing() { + let mut full = vec![placed(1, "B"), placed(2, "C"), placed(3, "B")]; + regroup_moved(&mut full, &[(1, "B".to_string()), (3, "B".to_string())]); + assert_eq!( + paths(&full), + vec![(1, "B".into()), (2, "C".into()), (3, "B".into())] + ); +} + +/// A move into a folder that does not exist yet has nothing to join either, so the row stays where +/// it is and the new folder is created around it. The alternative — appending to the end of the +/// whole list — is the placement this module exists to avoid. +#[test] +fn a_move_into_a_new_folder_leaves_the_row_in_place() { + let mut full = vec![placed(1, "X"), placed(2, "NEW"), placed(3, "X")]; + regroup_moved(&mut full, &[(2, "NEW".to_string())]); + assert_eq!( + paths(&full), + vec![(1, "X".into()), (2, "NEW".into()), (3, "X".into())] + ); +} + +/// A row that sits BEFORE its destination run still joins it. The case is worth its own test +/// because a plan expressed in positions gets this one wrong in the opposite direction from the +/// backward move above. +#[test] +fn a_row_ahead_of_its_destination_still_joins_it() { + let mut full = vec![placed(1, "X"), placed(2, "Z"), placed(3, "Z")]; + regroup_moved(&mut full, &[(1, "Z".to_string())]); + assert_eq!( + paths(&full), + vec![(2, "Z".into()), (3, "Z".into()), (1, "X".into())] + ); +} + +/// Two destinations interleaved in one move — what a folder move or rename produces whenever it +/// merges into folders that already exist. Each run comes out whole: a plan carried as absolute +/// positions goes stale as soon as the first relocation crosses another destination's slot, and +/// then both folders end up split. +#[test] +fn two_destinations_in_one_move_each_come_out_contiguous() { + let mut full = vec![ + placed(101, "D2"), + placed(1, "D1"), + placed(11, "D1"), + placed(21, "D2"), + placed(22, "D2"), + placed(12, "D1"), + ]; + regroup_moved( + &mut full, + &[ + (11, "D1".to_string()), + (21, "D2".to_string()), + (22, "D2".to_string()), + (12, "D1".to_string()), + ], + ); + let order = paths(&full); + let at = |id: u64| order.iter().position(|(row, _)| *row == id).expect("row"); + // Every D1 row adjacent to the others, and likewise every D2 row. + let mut d1 = [at(1), at(11), at(12)]; + let mut d2 = [at(101), at(21), at(22)]; + d1.sort_unstable(); + d2.sort_unstable(); + assert_eq!(d1[2] - d1[0], 2, "D1 must be one contiguous run: {order:?}"); + assert_eq!(d2[2] - d2[0], 2, "D2 must be one contiguous run: {order:?}"); + // ... and the rows joining each run keep the order they were given. + assert!(at(11) < at(12), "D1 joiners keep their order: {order:?}"); + assert!(at(21) < at(22), "D2 joiners keep their order: {order:?}"); +} diff --git a/crates/moon-core/src/feed/live/convert.rs b/crates/moon-core/src/feed/live/convert.rs index ee53af53..0c969e55 100644 --- a/crates/moon-core/src/feed/live/convert.rs +++ b/crates/moon-core/src/feed/live/convert.rs @@ -1568,5 +1568,95 @@ pub(super) fn engine_action_result(e: &moonproto::EngineActionEvent) -> EngineAc } } +/// Longest folder path kept from the core's tree, in characters. +/// +/// A DISPLAY guard and nothing more: this projection is never the source of an outgoing tree — the +/// feed builds those from the core's own untouched paths — so clamping here cannot ask the core to +/// rename anything. Whether a path could be sent back at all is a separate question, answered once +/// per core by `CoreFolders::editable`. +const FOLDER_PATH_MAX_CHARS: usize = 255; + +/// How many folders one core's tree is kept to. +/// +/// moonproto's own ceiling is 65 534 dictionary entries; this sits far below it because the list is +/// retained per core and drawn as rows. Truncating is safe for the same reason the clamp above is: +/// nothing sends this list back. +const FOLDER_PATHS_MAX: usize = 5_000; + +/// Project the core's folder tree, empty folders included. +/// +/// `supported` is `folders_last_modified() > 0` and nothing else: the protocol states plainly that +/// zero means either "no versioned tree has arrived" or "this core does not synchronize folders", +/// and the two are indistinguishable from here. Both readings mean the same thing to a caller — an +/// empty folder cannot be sent to this core — so nothing is gained by guessing which one holds. +/// +/// Paths are cleaned like every other inbound string, and the caller does the path SPLITTING: which +/// slashes separate folders is decided in one place in the window, and a second rule here would +/// disagree with the tree it feeds. +/// +/// Args: +/// strats: The core's retained strategy state. +/// +/// Returns: +/// The reported tree, or a `supported: false` value with no paths. +pub(super) fn folders_from_proto( + strats: &moonproto::state::StratsState, +) -> crate::feed::CoreFolders { + let supported = strats.folders_last_modified() > 0; + if !supported { + return crate::feed::CoreFolders::default(); + } + // Asked of the paths as the CORE spells them, before any cleaning: whether an edit can be sent + // depends on what the core holds, not on what this projection made of it. + let editable = crate::feed::folder_tree::sendable(strats.folder_paths()); + let mut paths: Vec = strats + .folder_paths() + .map(|path| wire_text(path, FOLDER_PATH_MAX_CHARS)) + .filter(|path| !path.is_empty()) + .collect(); + // Sorted because the source order is explicitly meaningless — moonproto iterates a map and says + // so — and an unstable order would republish an unchanged tree on every rehash, waking the + // window and rebuilding its whole strategy tree for nothing. + // + // BEFORE the cap, so that which folders survive it is decided by the paths themselves rather + // than by where a rehash happened to put them. + paths.sort_unstable(); + paths.dedup(); + paths.truncate(FOLDER_PATHS_MAX); + crate::feed::CoreFolders { + supported, + editable, + paths, + } +} + +/// Fold the strategy set into the signature that decides whether the UI is told about it. +/// +/// ORDER-SENSITIVE, and that is the load-bearing property rather than an artefact of the fold. The +/// core's strategy list is an arrangement the operator made, moonproto synchronizes it, and the +/// terminal's whole confirmation path — the tree's unconfirmed-order overlay, which draws a sent +/// arrangement until the core answers — can only be retired by seeing the answer arrive. Folded +/// commutatively, as a sibling signature in `tree::cache` deliberately is, a reorder echo would be +/// invisible: the overlay would sit out its whole window and then expire onto a list it had already +/// been given. `a_reordered_set_is_a_different_signature` pins it. +/// +/// Args: +/// rows: Per strategy, `(id, revision, last edit date, checked)`, in the core's own order. +/// +/// Returns: +/// A signature that changes whenever the contents OR the sequence do. +pub(super) fn strategies_publish_sig(rows: impl Iterator) -> u64 { + let mut sig = 0u64; + for (id, ver, last_date, checked) in rows { + sig = sig + .wrapping_mul(1099511628211) + .wrapping_add(id) + .wrapping_add((ver as u32 as u64).wrapping_shl(1)) + .wrapping_add(last_date) + .wrapping_add(checked as u64); + } + sig +} + #[cfg(test)] mod tests; diff --git a/crates/moon-core/src/feed/live/convert/tests.rs b/crates/moon-core/src/feed/live/convert/tests.rs index 47af3a88..438431e7 100644 --- a/crates/moon-core/src/feed/live/convert/tests.rs +++ b/crates/moon-core/src/feed/live/convert/tests.rs @@ -421,3 +421,34 @@ fn a_delivered_finding_counts_as_support_before_the_first_list() { "an answered core with no findings is not silent either" ); } + +/// The publish gate is the ONLY way a reorder echo reaches the window: the tree draws a sent +/// arrangement until the core answers, and it recognises the answer by the strategy set arriving +/// again. A commutative fold here — the shape a sibling signature in `tree::cache` uses on purpose +/// — would make an order-only change invisible and strand that overlay for its whole window. +#[test] +fn a_reordered_set_is_a_different_signature() { + let rows = [(1u64, 1i32, 100u64, true), (2, 1, 200, false)]; + let forward = super::strategies_publish_sig(rows.iter().copied()); + let reversed = super::strategies_publish_sig(rows.iter().rev().copied()); + assert_ne!(forward, reversed); + // ... while the same set in the same order still says nothing changed. + assert_eq!(forward, super::strategies_publish_sig(rows.iter().copied())); +} + +/// The contents still matter as much as the sequence: one flag or one edit date moving on its own +/// must republish, which is what every non-order change relies on. Only that — the fold sums its +/// four per-row fields, so two changes that cancel out numerically are not separated, which is the +/// ordinary trade of a cheap signature and not something this asserts otherwise. +#[test] +fn a_changed_field_is_a_different_signature() { + let base = [(1u64, 1i32, 100u64, true)]; + let checked_off = [(1u64, 1i32, 100u64, false)]; + let edited = [(1u64, 1i32, 101u64, true)]; + let sig = super::strategies_publish_sig(base.iter().copied()); + assert_ne!( + sig, + super::strategies_publish_sig(checked_off.iter().copied()) + ); + assert_ne!(sig, super::strategies_publish_sig(edited.iter().copied())); +} diff --git a/crates/moon-core/src/feed/live/mod.rs b/crates/moon-core/src/feed/live/mod.rs index 376fecdf..0f246e06 100644 --- a/crates/moon-core/src/feed/live/mod.rs +++ b/crates/moon-core/src/feed/live/mod.rs @@ -574,6 +574,13 @@ pub(super) fn run( // changes because strategy fields are expensive and need not be sent every second. let mut last_schema_rev: u64 = u64::MAX; let mut last_strat_sig: u64 = u64::MAX; + // Folder-tree cursors. Two of them because the tree moves for two independent reasons: the core + // versions it whenever a folder is created or deleted, and the folders the STRATEGIES imply + // change whenever a strategy's path does — which the version does not see. + let mut last_folders_version: i64 = i64::MIN; + // Seeded with the digest of an EMPTY tree, which is what a core reports before it has said + // anything: `u64::MAX` would make the first pass publish that nothing-yet as a change. + let mut last_folders_digest: u64 = 0; // Strategy-edit publish cadence, independent of the 1 Hz strategies gate below. The retained // latch is set on ANY edit event and cleared only once a publish actually goes out, so a // resolution arriving inside the 250 ms shadow of the previous publish is delayed, never @@ -2200,15 +2207,11 @@ pub(super) fn run( } // Publish contents/values when the signature changes (id/ver/last_date/checked). - let mut sig = 0u64; - for s in strats.snapshots() { - sig = sig - .wrapping_mul(1099511628211) - .wrapping_add(s.strategy_id) - .wrapping_add((s.strategy_ver as u32 as u64).wrapping_shl(1)) - .wrapping_add(s.last_date) - .wrapping_add(s.checked as u64); - } + let sig = convert::strategies_publish_sig( + strats + .snapshots() + .map(|s| (s.strategy_id, s.strategy_ver, s.last_date, s.checked)), + ); let delivery_result = pending_strat_db_delivery .as_ref() @@ -2227,7 +2230,8 @@ pub(super) fn run( &mut strat_db_initial, ); } - if sig != last_strat_sig { + let strategies_changed = sig != last_strat_sig; + if strategies_changed { last_strat_sig = sig; // The order table's Strat column resolves strat_id to kind through this same // registry in `build_order_row`. The registry is populated AFTER orders, while @@ -2263,6 +2267,34 @@ pub(super) fn run( break; } } + + // The core's folder tree, empty folders included. Read on its own cursor rather + // than inside the block above: a folder created or deleted with no strategy in it + // moves nothing the strategy signature covers, and that folder is exactly the one + // this list exists to carry. + let folders_version = strats.folders_last_modified(); + if folders_version != last_folders_version || strategies_changed { + last_folders_version = folders_version; + let folders = convert::folders_from_proto(strats); + // Digested rather than compared: the tree is republished on every strategy + // change, and holding a second copy of every path per core to answer "did it + // move" costs more than the fold does. + let digest = folders.paths.iter().fold( + u64::from(folders.supported) | (u64::from(folders.editable) << 1), + |acc, path| { + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + std::hash::Hash::hash(path, &mut hasher); + acc.wrapping_mul(1099511628211) + .wrapping_add(std::hash::Hasher::finish(&hasher)) + }, + ); + if digest != last_folders_digest { + last_folders_digest = digest; + if tx.send(FeedMsg::Folders(folders)).is_err() { + break; + } + } + } // The database cursor is separate from the UI cursor: schema defaults can arrive // after an unchanged strategy set, and a full writer queue must leave the set due // for retry. Dumps are incomplete until defaults are available. diff --git a/crates/moon-core/src/feed/mod.rs b/crates/moon-core/src/feed/mod.rs index a3ffc553..83b1cb4a 100644 --- a/crates/moon-core/src/feed/mod.rs +++ b/crates/moon-core/src/feed/mod.rs @@ -4,12 +4,14 @@ pub(crate) mod assets; mod conn_verdict; mod core_label; +pub mod folder_tree; pub mod live; mod mode_advice; pub mod news; pub mod news_marks; mod order_edit; mod strategies; +pub mod strategy_order; pub mod synth; mod trade; pub mod types; @@ -366,7 +368,45 @@ pub enum CoreCmd { /// Move existing strategies or rename their folder. Each `moves` entry contains /// `(strategy_id, new_folder_path)`. The feed patches `path` for the listed strategies in the /// full set, advances `last_date`, and sends one `sync_local_strategies`. - MoveStrategies { moves: Vec<(u64, String)> }, + MoveStrategies { + moves: Vec<(u64, String)>, + /// The folder subtree this move renames or reparents, as `(old path, new path)`. + /// + /// An INTENT, not a tree. The core now keeps folders that hold no strategy, so rewriting + /// the rows' paths leaves the OLD path behind as an empty folder of its own, and only a + /// folder tree omitting it removes it — but which tree to omit it FROM is a question only + /// the feed can answer, because the newest one it knows includes edits the core has not + /// echoed yet. The protocol wants both halves in one snapshot, so the feed builds the tree + /// and sends it with these moves. + /// + /// `None` for a move that changes no folder's identity — dragging strategies between + /// folders, which must NOT delete the folder they came from. + rebase: Option<(String, String)>, + }, + /// Create one folder on the core, holding nothing. + /// + /// Carries the path alone, deliberately. The wire form is the complete desired tree — the core + /// deletes every empty folder the list omits — and a tree assembled by a window is assembled + /// from a snapshot that may already be stale, which turns a create into a silent delete of + /// whatever arrived meanwhile. The feed owns the list; this says only what to add to it. + AddFolder { path: String }, + /// Remove one folder and everything under it from the core's tree. + /// + /// For a folder that holds no strategy; rows are deleted separately and first. Same reasoning + /// as [`CoreCmd::AddFolder`]: the intent travels, the list is built where it is known. + RemoveFolder { path: String }, + /// Rearrange the core's strategy list. `order` is the complete desired id sequence. + /// + /// The sequence itself is the payload: moonproto synchronizes strategy order as the row order + /// of a Full snapshot, so the feed sorts the full set into `order` and sends one + /// `sync_local_strategies`. Nothing else about a strategy changes — no field is patched and no + /// `last_date` is advanced, because the order carries its own version on the wire (moonproto + /// `docs/strats.md`, "Strategy Order"). + /// + /// Ids the core does not have are ignored, and ids the core has but `order` omits keep their + /// relative places at the end, so a list that raced with a create or a delete still reorders + /// what it does name instead of dropping anything. + ReorderStrategies { order: Vec }, /// Transfer an asset between wallets of one core through drag and drop in the Assets tree. /// `from` and `to` are Spot, Futures, or Quarterly wallets; `qty` is in the base coin. TransferAsset { diff --git a/crates/moon-core/src/feed/strategy_order.rs b/crates/moon-core/src/feed/strategy_order.rs new file mode 100644 index 00000000..d5666a26 --- /dev/null +++ b/crates/moon-core/src/feed/strategy_order.rs @@ -0,0 +1,77 @@ +//! Applying a desired strategy sequence to a list that has moved on since it was chosen. +//! +//! A reorder names every strategy the operator could see when they pressed the button. By the time +//! it is applied — one frame later in the window, one round trip later in the core — the list can +//! hold strategies the sequence never named: one created, pasted or restored in between, or one +//! this side had simply not received yet. +//! +//! Where those unnamed rows land is not a detail. Sorting them to the END looks harmless and is +//! not: the core requires one folder's strategies to stay a contiguous group (moonproto +//! `docs/strats.md`, "Strategy Order"), so a new strategy flung to the tail leaves its folder split +//! in two, and the NEXT reorder sends that tail position back to the core as the deliberate +//! arrangement. One press then permanently moves a strategy nobody touched. +//! +//! So an unnamed row keeps the row it currently follows. That is the only placement that is a +//! no-op for it — it neither leaves its folder nor changes what it is adjacent to. + +/// Rearrange `items` into the desired sequence, keeping rows the sequence never named in place. +/// +/// Stable: rows sharing a position keep their current relative order. +/// +/// Args: +/// items: The full current list, in its current order. +/// rank_of: Position of one item in the desired sequence, or `None` when it names no position. +/// +/// Returns: +/// How many items ended up at a different index than they started at — zero when the list +/// already holds the desired arrangement, which callers use to send nothing at all. +pub fn resequence(items: &mut Vec, rank_of: impl Fn(&T) -> Option) -> usize { + /// Where one row sorts: its desired position, and whether it is a named row, one trailing a + /// named row, or one that precedes every named row. + #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] + struct Slot(usize, u8); + + let mut slots: Vec = Vec::with_capacity(items.len()); + // Rank of the last named row seen, so an unnamed row can attach itself to it. + let mut trailing: Option = None; + for item in items.iter() { + slots.push(match rank_of(item) { + Some(rank) => { + trailing = Some(rank); + Slot(rank, 1) + } + // Directly after the named row it currently follows — or, before any named row has + // been seen, ahead of the whole sequence, which is equally where it already sits. + None => match trailing { + Some(rank) => Slot(rank, 2), + None => Slot(0, 0), + }, + }); + } + + let mut order: Vec = (0..items.len()).collect(); + order.sort_by_key(|&at| (slots[at], at)); + let moved = order + .iter() + .enumerate() + .filter(|(now, was)| *now != **was) + .count(); + if moved > 0 { + // Moved, never cloned: `T` here is a whole `StrategySnapshot` with every field string it + // carries, and a reorder that deep-copied the account's entire strategy set would spend + // more on the copy than on the sync it exists to prepare. Taking each element out through + // an `Option` is what lets the permutation be applied by index without `T: Clone`. + let mut taken: Vec> = std::mem::take(items).into_iter().map(Some).collect(); + *items = order + .into_iter() + // `order` is a sorted `0..len`, so every slot is visited exactly once. Stated rather + // than skipped: were that ever untrue, dropping the slot would quietly SHORTEN the + // strategy list this becomes on the wire. + .map(|at| taken[at].take().expect("resequence visits each index once")) + .collect(); + } + moved +} + +#[cfg(test)] +mod tests; diff --git a/crates/moon-core/src/feed/strategy_order/tests.rs b/crates/moon-core/src/feed/strategy_order/tests.rs new file mode 100644 index 00000000..fe051b23 --- /dev/null +++ b/crates/moon-core/src/feed/strategy_order/tests.rs @@ -0,0 +1,56 @@ +//! Unit tests for applying a desired sequence to a list that has moved on. + +use super::resequence; + +/// Runs one resequence over ids, ranking the ones the desired sequence names. +fn apply(items: &[u64], desired: &[u64]) -> (Vec, usize) { + let mut items = items.to_vec(); + let moved = resequence(&mut items, |id| desired.iter().position(|want| want == id)); + (items, moved) +} + +/// The ordinary case: every row is named, and the list comes out in exactly that sequence. +#[test] +fn a_fully_named_list_takes_the_desired_sequence() { + assert_eq!(apply(&[1, 2, 3], &[3, 1, 2]), (vec![3, 1, 2], 3)); +} + +/// Nothing moved means nothing to send, and that zero is what a caller uses to stay off the wire. +#[test] +fn an_order_the_list_already_holds_moves_nothing() { + assert_eq!(apply(&[1, 2, 3], &[1, 2, 3]), (vec![1, 2, 3], 0)); +} + +/// The point of the module. A strategy created after the sequence was chosen keeps the row it +/// follows instead of being flung to the end, where it would leave its folder split in two and be +/// sent back to the core as a deliberate arrangement by the next press. +#[test] +fn an_unnamed_row_keeps_the_row_it_follows() { + assert_eq!(apply(&[1, 9, 2, 3], &[3, 1, 2]), (vec![3, 1, 9, 2], 4)); +} + +/// Several unnamed rows behind the same named one keep their own order behind it. +#[test] +fn unnamed_rows_behind_one_row_keep_their_relative_order() { + assert_eq!(apply(&[1, 8, 9, 2], &[2, 1]), (vec![2, 1, 8, 9], 4)); +} + +/// A row that precedes every named one has nothing to follow, so it stays at the front — which is +/// also where it already is. +#[test] +fn a_row_before_every_named_one_stays_at_the_front() { + assert_eq!(apply(&[7, 1, 2], &[2, 1]), (vec![7, 2, 1], 2)); +} + +/// Ids the sequence names but the list does not hold are simply absent; they must not create gaps, +/// duplicate anything, or drop a row. +#[test] +fn ids_the_list_no_longer_holds_are_ignored() { + assert_eq!(apply(&[1, 3], &[3, 2, 1]).0, vec![3, 1]); +} + +/// An empty desired sequence names nothing at all, so every row keeps its place. +#[test] +fn an_empty_sequence_leaves_the_list_alone() { + assert_eq!(apply(&[1, 2, 3], &[]), (vec![1, 2, 3], 0)); +} diff --git a/crates/moon-core/src/feed/types.rs b/crates/moon-core/src/feed/types.rs index 3bd2b1a7..9b8cc67e 100644 --- a/crates/moon-core/src/feed/types.rs +++ b/crates/moon-core/src/feed/types.rs @@ -1,10 +1,12 @@ //! Domain types sent from the backend to the UI. They are independent of moonproto so the UI and //! rendering layer do not need to know about the transport. +mod core_folders; mod core_problem; mod core_settings; mod core_status; +pub use core_folders::CoreFolders; pub use core_problem::{CoreProblem, CoreProblemCategory, CoreProblems}; pub use core_settings::{ AutoBuySettings, AutoStartSettings, BtcBlinkSettings, CORE_HOTKEY_ACTION_COUNT, CoreConfig, @@ -1290,6 +1292,14 @@ pub enum FeedMsg { /// counter rather than on arrival — the core republishes the same list on reconnect and on /// every newly confirmed row alike. Problems(CoreProblems), + /// The core's folder tree, empty folders included, whenever it or the folders the strategies + /// imply have changed. + /// + /// A FULL replace like `Problems`, for the same reason: the protocol delivers the whole tree + /// and a folder that vanished from it has no event of its own. `folders_rev` moves only when + /// the projection actually differs, so a core republishing an identical tree on reconnect + /// wakes nothing. + Folders(CoreFolders), /// Core startup progress and channel measurements, POLLED from the moonproto client rather /// than pushed by an event — MoonProto publishes it as a passive snapshot at its own bounded /// rate. Sent only while the core is starting, plus once when it settles, so an already-started diff --git a/crates/moon-core/src/feed/types/core_folders.rs b/crates/moon-core/src/feed/types/core_folders.rs new file mode 100644 index 00000000..520b2dbd --- /dev/null +++ b/crates/moon-core/src/feed/types/core_folders.rs @@ -0,0 +1,54 @@ +//! The core's own folder tree — including the folders that hold no strategy at all. +//! +//! Until moonproto `55f78d0` a folder existed only as a prefix of some strategy's path, so an empty +//! one could not be represented on the wire and the terminal kept its own local list instead. The +//! core now maintains a VERSIONED folder tree and reports every folder in it, empty ones included +//! (`docs/strats.md`, "Folders, Including Empty Folders"). That makes "new folder" a real edit the +//! core can hold, rather than a mark that lives until the window closes. +//! +//! Whether a given core can do that is not a question this terminal gets to ask directly. The +//! answer is [`CoreFolders::supported`], and it means exactly one thing: a versioned tree has +//! arrived. A core too old to send one leaves it false forever, and every folder path here is then +//! whatever the strategies themselves imply. + +/// Folders one core currently holds, as it reports them. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct CoreFolders { + /// Whether this core has published a versioned folder tree. + /// + /// False covers two situations the terminal cannot tell apart and does not need to: a core + /// whose build predates folder synchronization, and one whose first tree has not arrived yet. + /// In both, an empty folder cannot be sent, so the caller keeps its local fallback and does not + /// promise the operator that a new folder will survive. + /// + /// Deliberately NOT latched across connections: a replacement feed can point at a different + /// MoonBot, and a core downgraded below the extension has to be able to go back to "not known". + pub supported: bool, + /// Whether this terminal may EDIT that tree. + /// + /// Stricter than [`Self::supported`], and the difference is a real account rather than a + /// hypothetical one. A folder edit is submitted as the complete desired tree, and moonproto + /// validates every path in it — splitting on each `/` and refusing a segment with surrounding + /// whitespace. MoonBot, meanwhile, allows a `/` INSIDE a folder name, and for a strategy in one + /// of those moonproto's own state adds the split halves as parent folders. So a core holding a + /// folder called `"EMA / ORGANIC"` reports a tree containing `"EMA "`, which nothing can send + /// back: every folder edit on that core would be refused whole. + /// + /// Rather than discover that per command, the terminal asks once. False here means the window + /// keeps its local marks and promises the operator nothing, exactly as for a core too old to + /// synchronize folders at all — while [`Self::supported`] still lets it DRAW what the core + /// reports. + pub editable: bool, + /// Every confirmed folder the core reports, parents included, sorted. + /// + /// SORTED here rather than kept in whatever order the core hands them over: the protocol calls + /// that order unspecified, moonproto iterates a map to produce it, and a set that reshuffles + /// itself would republish as a change on every rehash. + /// + /// A DISPLAY projection: paths are cleaned of control characters and clamped, so this is not + /// the list an edit is built from — the feed builds those from the core's own untouched paths. + /// Nothing here splits them either. Which slashes separate folders is a question this terminal + /// answers in ONE place, the Strategies window's own path module, and a second opinion formed + /// at the boundary would invent folders that exist nowhere. + pub paths: Vec, +} diff --git a/crates/moon-core/src/session/commands.rs b/crates/moon-core/src/session/commands.rs index c458d304..19cfbebc 100644 --- a/crates/moon-core/src/session/commands.rs +++ b/crates/moon-core/src/session/commands.rs @@ -260,12 +260,92 @@ impl SessionManager { } /// Change the folder of existing core strategies to rename a folder or move strategies. - /// Each `moves` entry is `(strategy_id, new_folder_path)`. Send one batch per core. - pub fn move_strategies(&self, core: CoreId, moves: Vec<(u64, String)>) -> Result<()> { - if moves.is_empty() { + /// + /// Args: + /// core: Core owning the strategies. + /// moves: `(strategy id, new folder path)` per strategy. Send one batch per core. + /// rebase: The folder subtree this move renames or reparents, as `(old path, new path)`. + /// A rename or a folder drag must pass one, or the emptied old path stays behind on the + /// core as a folder of its own — see [`CoreCmd::MoveStrategies`]. + /// + /// Returns: + /// Success once the intent entered that core's command queue. A call with neither moves nor + /// a rebase asks for nothing and is dropped here. + pub fn move_strategies( + &self, + core: CoreId, + moves: Vec<(u64, String)>, + rebase: Option<(String, String)>, + ) -> Result<()> { + // A folder holding no strategy has no moves to carry it, and its rename is still a real + // edit — so emptiness alone is not the test. + if moves.is_empty() && rebase.is_none() { + return Ok(()); + } + self.send_core_cmd( + core, + CoreCmd::MoveStrategies { moves, rebase }, + "move strategies", + ) + } + + /// Create one folder on a core, holding nothing. + /// + /// Args: + /// core: Core to create it on. + /// path: Canonical folder path. + /// + /// Returns: + /// Success once the intent entered that core's command queue. Whether the core can hold an + /// empty folder at all is decided on the feed thread, which logs what it declines; callers + /// read `CoreFolders::editable` to decide what to PROMISE the operator, not to gate this. + pub fn add_core_folder(&self, core: CoreId, path: String) -> Result<()> { + if path.is_empty() { return Ok(()); } - self.send_core_cmd(core, CoreCmd::MoveStrategies { moves }, "move strategies") + self.send_core_cmd(core, CoreCmd::AddFolder { path }, "add folder") + } + + /// Remove one folder and its whole subtree from a core's tree. + /// + /// Args: + /// core: Core to remove it from. + /// path: Canonical folder path. + /// + /// Returns: + /// Success once the intent entered that core's command queue. Strategies inside are NOT + /// deleted by this — the core keeps a folder that still holds one — so a caller removing a + /// populated folder deletes its rows first. + pub fn remove_core_folder(&self, core: CoreId, path: String) -> Result<()> { + if path.is_empty() { + return Ok(()); + } + self.send_core_cmd(core, CoreCmd::RemoveFolder { path }, "remove folder") + } + + /// Rearrange a core's strategy list into `order`, the complete desired id sequence. + /// + /// The core confirms by echoing a full strategy snapshot in that sequence; nothing here waits + /// for it. See [`CoreCmd::ReorderStrategies`] for what the feed does with a list that raced + /// with a create or a delete. + /// + /// Args: + /// core: Core whose list is being rearranged. + /// order: Every strategy id of that core, in the desired order. + /// + /// Returns: + /// Success once the intent entered that core's command queue. + /// + /// Deliberately without a "too short to matter" early return: one that answered `Ok(())` + /// without queueing anything would hand the caller a success for a command that does not + /// exist, and the caller draws an unconfirmed arrangement on the strength of it. Whether an + /// order says anything is the feed's decision, taken against the list it actually holds. + pub fn reorder_strategies(&self, core: CoreId, order: Vec) -> Result<()> { + self.send_core_cmd( + core, + CoreCmd::ReorderStrategies { order }, + "reorder strategies", + ) } /// Transfer an asset between wallets of one core through drag and drop in the Assets window. diff --git a/crates/moon-core/src/session/lifecycle.rs b/crates/moon-core/src/session/lifecycle.rs index a2265ff4..a4a8f547 100644 --- a/crates/moon-core/src/session/lifecycle.rs +++ b/crates/moon-core/src/session/lifecycle.rs @@ -377,6 +377,16 @@ impl SessionManager { stats.ui_state |= core.problems_rev != before; } } + FeedMsg::Folders(folders) => { + // Gated for the same reason as `Problems`: the tree is republished whenever + // the strategies move, which on a busy account is constantly, and only an + // actual difference is worth a repaint. + if let Some(core) = self.store.core_mut(sess.id) { + let before = core.folders_rev; + core.apply(FeedMsg::Folders(folders)); + stats.ui_state |= core.folders_rev != before; + } + } other => { if let Some(core) = self.store.core_mut(sess.id) { core.apply(other); diff --git a/crates/moon-core/src/session/store.rs b/crates/moon-core/src/session/store.rs index 76ec8fe5..8671a780 100644 --- a/crates/moon-core/src/session/store.rs +++ b/crates/moon-core/src/session/store.rs @@ -263,6 +263,11 @@ pub struct CoreData { /// connection, so the cost of clearing is the seconds until it arrives, stated honestly as /// "not known" rather than as a clean bill. pub problems: crate::feed::CoreProblems, + /// The core's folder tree, empty folders included, observed through `folders_rev`. + /// + /// Its `supported` flag is what tells a caller whether an empty folder can be sent to this core + /// at all; see [`crate::feed::CoreFolders`]. + pub folders: crate::feed::CoreFolders, /// Latest startup progress and channel measurements polled from the moonproto client. /// The Core Status panel observes it through `startup_rev`. It FREEZES once the core settles, /// so after a successful startup `elapsed_ms` is how long that core took to come up, not a @@ -368,6 +373,8 @@ pub struct CoreData { /// list on every reconnect and again for each newly confirmed row, so an ungated counter would /// repaint the panel for a list that has not changed at all. pub problems_rev: u64, + /// Advances when the reported folder tree actually differs. + pub folders_rev: u64, /// Advances when the polled startup snapshot reports different PROGRESS, per /// `CoreStartupStatus::progress_eq`. Deliberately separate from `sys_rev`: that counter is /// documented as covering `KernelHealth` metrics and the decoded endpoint, its field is CLEARED @@ -463,7 +470,9 @@ impl CoreData { chart_alerts_rev: 0, sys_rev: 0, problems: crate::feed::CoreProblems::default(), + folders: crate::feed::CoreFolders::default(), problems_rev: 0, + folders_rev: 0, startup_rev: 0, news_rev: 0, time_offset: crate::feed::CoreTimeOffsetStatus::default(), @@ -608,6 +617,13 @@ impl CoreData { self.problems = crate::feed::CoreProblems::default(); self.problems_rev = self.problems_rev.wrapping_add(1); } + // The folder tree belongs to the replaced MoonBot too, and clearing `supported` with it is + // again the point rather than a side effect: a replacement feed may point at a core that + // cannot hold an empty folder, and a retained flag would keep promising that it can. + if self.folders != crate::feed::CoreFolders::default() { + self.folders = crate::feed::CoreFolders::default(); + self.folders_rev = self.folders_rev.wrapping_add(1); + } // A replacement feed may point at a different MoonBot on a different clock, so last // connection's estimate carries no evidence about this one. // @@ -1005,6 +1021,15 @@ impl CoreData { self.problems_rev = self.problems_rev.wrapping_add(1); } } + FeedMsg::Folders(folders) => { + // Compared before adopting, like the diagnostics above: the tree is republished + // whenever the strategies move, and an unconditional revision would rebuild the + // window's whole strategy tree for a folder list that did not change. + if self.folders != folders { + self.folders = folders; + self.folders_rev = self.folders_rev.wrapping_add(1); + } + } FeedMsg::ConnFault(fault) => { // A plain overwrite: the newest attempt is the one being explained, and the feed // emits this exactly once per terminal failure. No revision counter — see the diff --git a/crates/moon-core/src/session/store/tests.rs b/crates/moon-core/src/session/store/tests.rs index 92adffb1..70ae54f1 100644 --- a/crates/moon-core/src/session/store/tests.rs +++ b/crates/moon-core/src/session/store/tests.rs @@ -1045,3 +1045,51 @@ fn a_new_problem_list_replaces_the_old_one_and_gates_the_revision() { assert_eq!(core.problems.items[0].kind, 2); assert_eq!(core.problems_rev, after_first + 1); } + +/// The folder tree follows the same rule as the diagnostics beside it, and for the same reason: a +/// replacement feed may point at a MoonBot that cannot hold an empty folder at all. Carrying +/// `supported` forward would leave the window promising a persistence the new core does not offer. +#[test] +fn a_replacement_connection_returns_folders_to_nothing_known() { + let mut core = CoreData::new(); + assert!(!core.folders.supported, "nothing known before any tree"); + + core.apply(FeedMsg::Folders(crate::feed::CoreFolders { + supported: true, + editable: true, + paths: vec!["Research".to_string()], + })); + assert!(core.folders.supported); + assert_eq!(core.folders_rev, 1); + + core.begin_connection_attempt(); + + assert!(!core.folders.supported); + assert!(core.folders.paths.is_empty()); + assert_eq!(core.folders_rev, 2, "clearing is a change consumers see"); +} + +/// The tree is republished whenever the strategies move, which on a busy account is constantly. +/// Only a real difference may advance the revision — `session::lifecycle` wakes the window on that +/// counter, and the whole strategy tree is rebuilt behind it. +#[test] +fn an_unchanged_folder_tree_does_not_advance_its_revision() { + let mut core = CoreData::new(); + let tree = crate::feed::CoreFolders { + supported: true, + editable: true, + paths: vec!["Research".to_string(), "Research/Deep".to_string()], + }; + + core.apply(FeedMsg::Folders(tree.clone())); + let after_first = core.folders_rev; + core.apply(FeedMsg::Folders(tree)); + assert_eq!(after_first, core.folders_rev, "the same tree said twice"); + + core.apply(FeedMsg::Folders(crate::feed::CoreFolders { + supported: true, + editable: true, + paths: vec!["Research".to_string()], + })); + assert_ne!(after_first, core.folders_rev, "a folder that went away"); +} diff --git a/crates/moon-ui-gpui/src/strategies/logic.rs b/crates/moon-ui-gpui/src/strategies/logic.rs index 114ab5ac..d082d942 100644 --- a/crates/moon-ui-gpui/src/strategies/logic.rs +++ b/crates/moon-ui-gpui/src/strategies/logic.rs @@ -570,30 +570,81 @@ pub(super) fn toggle(set: &mut HashSet, ke } /// Folder-tree node containing named child folders and strategies directly in this folder. +/// +/// Children are kept in INSERTION order, not sorted by name. The strategy list a core sends is an +/// ordered list the operator arranges in MoonBot, and moonproto synchronizes that order as the row +/// sequence of a Full snapshot (`docs/strats.md`, "Strategy Order"), with one folder's strategies +/// forming a contiguous group. So the order a folder first appears in that sequence IS its place in +/// the core's own tree, and the previous `BTreeMap` threw it away for a byte-wise alphabet that +/// matched neither MoonBot nor anything the operator arranged — `Zeta` above `alpha`, Cyrillic +/// names after every Latin one. +/// +/// Folders holding no strategy have no place in that sequence; [`ensure_folder`] appends them, so +/// its caller decides their order among themselves. #[derive(Default)] pub(super) struct FolderNode<'a> { - pub(super) children: std::collections::BTreeMap>, + /// Child folders, in the order this level first saw each of them. + children: Vec<(String, FolderNode<'a>)>, pub(super) strategies: Vec<&'a StrategyRow>, } +impl<'a> FolderNode<'a> { + /// Child folders paired with their names, in display order. + pub(super) fn children(&self) -> impl Iterator)> { + self.children + .iter() + .map(|(name, node)| (name.as_str(), node)) + } + + /// Borrow one child folder by name, appending it when this level has not seen it yet. + /// + /// Scanned from the END because the core groups one folder's strategies contiguously: for a + /// run of rows sharing a path the match is the last child every time, which keeps the walk + /// linear in practice instead of quadratic in the sibling count. + /// + /// Args: + /// name: One path segment, exactly as the data spells it. + /// + /// Returns: + /// The existing child of that name, or a fresh empty one appended after every sibling. + fn child_mut(&mut self, name: &str) -> &mut FolderNode<'a> { + match self.children.iter().rposition(|(seen, _)| seen == name) { + Some(at) => &mut self.children[at].1, + None => { + self.children + .push((name.to_string(), FolderNode::default())); + let appended = self.children.len() - 1; + &mut self.children[appended].1 + } + } + } +} + /// Build a nested tree from strategy paths, split through [`path_segments`]. +/// +/// The iterator must arrive in the core's own strategy order — that sequence is what decides where +/// each folder sits. See [`FolderNode`]. pub(super) fn build_node<'a>(it: impl Iterator) -> FolderNode<'a> { let mut root = FolderNode::default(); for r in it { let mut node = &mut root; for part in path_segments(&r.folder_path) { - node = node.children.entry(part.to_string()).or_default(); + node = node.child_mut(part); } node.strategies.push(r); } root } -/// Ensure that a path exists for empty UI folders that contain no strategies yet. +/// Ensure that a path exists for empty folders that contain no strategies yet. +/// +/// A missing level is APPENDED after everything the strategy walk already placed, so the caller's +/// own order over these paths is the order they appear in — feed them in a deterministic one or the +/// tree reshuffles between frames. pub(super) fn ensure_folder(root: &mut FolderNode, parts: &[String]) { let mut node = root; for part in parts { - node = node.children.entry(part.clone()).or_default(); + node = node.child_mut(part); } } @@ -607,12 +658,34 @@ pub(super) fn ensure_folder(root: &mut FolderNode, parts: &[String]) { /// introduce none of their own. #[derive(Default)] pub(super) struct FolderCounts { - by_path: HashMap, + by_path: HashMap, + /// The same prefixes lowercased, for the question the CORE asks: it keys folders + /// case-insensitively, so a tree spelling `Research` against strategies spelling `research` + /// names one folder, and a case-sensitive answer would draw the second one as empty beside it. + /// Bounded by the folder count rather than the row count — one entry per prefix, on first + /// visit. + folded: HashSet, root: (usize, usize), /// Set for a core whose folders are not built, so no per-folder entry is worth allocating. totals_only: bool, } +/// What one folder prefix accumulates: its caption's numbers, and its place in the core's order. +struct FolderStat { + /// Checked strategies at or below this folder, after the caption's own filter. + active: usize, + /// All strategies at or below it, after that same filter. + total: usize, + /// Index of the FIRST strategy of this folder in the core's own list, filtered by nothing. + /// + /// The tree orders folders by this rather than by where their first VISIBLE row sits, and the + /// difference is not cosmetic: with a filter on, a folder whose early rows are hidden would + /// otherwise slide below a folder that starts later, so typing in the search box would + /// rearrange the tree around the reader. Recorded on the first visit to each prefix, which is + /// the earliest by construction — the walk goes through the list in order. + first_seen: usize, +} + impl FolderCounts { /// Counts only the core total, for a collapsed core whose folder rows are never built. /// [`Self::for_path`] reports nothing in this mode and must not be consulted. @@ -632,16 +705,20 @@ impl FolderCounts { /// Args: /// row: Strategy contributing to its folder chain when it passes the count predicate. /// filter: Prepared kind/direction count predicate. + /// at: Index of this strategy in the core's own list, for [`FolderStat::first_seen`]. /// /// Returns: /// Nothing; matching counters are updated in place. - pub(super) fn add(&mut self, row: &StrategyRow, filter: &PreparedFilter) { - if !filter.counts(row) { - return; - } - let hit = usize::from(row.checked); + pub(super) fn add(&mut self, row: &StrategyRow, filter: &PreparedFilter, at: usize) { + // The walk itself is NOT gated: a folder's place in the tree comes from the core's list as + // it stands, not from the rows a filter happens to leave. Only the numbers are gated. + // `(checked, counted)` as numbers, once: the three sites below add exactly these two. + let (hit, one) = match filter.counts(row) { + true => (usize::from(row.checked), 1), + false => (0, 0), + }; self.root.0 += hit; - self.root.1 += 1; + self.root.1 += one; if self.totals_only { return; } @@ -653,10 +730,18 @@ impl FolderCounts { key.push_str(seg); // Look up before inserting so a repeat visit to a known folder does not clone the key. if let Some(e) = self.by_path.get_mut(&key) { - e.0 += hit; - e.1 += 1; + e.active += hit; + e.total += one; } else { - self.by_path.insert(key.clone(), (hit, 1)); + self.folded.insert(key.to_lowercase()); + self.by_path.insert( + key.clone(), + FolderStat { + active: hit, + total: one, + first_seen: at, + }, + ); } } } @@ -664,7 +749,27 @@ impl FolderCounts { /// Returns `(active, total)` at or below a folder, or `(0, 0)` for a folder holding no /// strategies — the case of a UI-only folder created before its first strategy. pub(super) fn for_path(&self, path: &str) -> (usize, usize) { - self.by_path.get(path).copied().unwrap_or((0, 0)) + self.by_path + .get(path) + .map_or((0, 0), |stat| (stat.active, stat.total)) + } + + /// Whether any strategy of this core lives at or below a folder path. + /// + /// The accumulator walks every row's whole prefix chain regardless of the filter, so this + /// answers about the CORE's folders rather than about the rows currently drawn — which is what + /// a caller deciding "is this folder empty" has to ask. Case-insensitively, because that is how + /// the core decides whether two spellings name one folder. + pub(super) fn knows(&self, path: &str) -> bool { + self.folded.contains(&path.to_lowercase()) + } + + /// Where a folder sits in the core's own order, or `None` for one holding no strategy at all. + /// + /// `None` is the empty folder — it appears in no strategy's path, so the core's list says + /// nothing about where it belongs, and the tree appends those instead. + pub(super) fn order_of(&self, path: &str) -> Option { + self.by_path.get(path).map(|stat| stat.first_seen) } /// Returns `(active, total)` over every counted strategy, which is the core's own caption. diff --git a/crates/moon-ui-gpui/src/strategies/logic/tests.rs b/crates/moon-ui-gpui/src/strategies/logic/tests.rs index 7e3f60fc..d4224b35 100644 --- a/crates/moon-ui-gpui/src/strategies/logic/tests.rs +++ b/crates/moon-ui-gpui/src/strategies/logic/tests.rs @@ -108,8 +108,8 @@ fn prefixes() -> Vec> { /// `add`, so pre-filtering here would hide a regression in that gate. fn accumulate(rows: &[StrategyRow], filter: &PreparedFilter) -> FolderCounts { let mut counts = FolderCounts::default(); - for r in rows { - counts.add(r, filter); + for (at, r) in rows.iter().enumerate() { + counts.add(r, filter, at); } counts } @@ -508,3 +508,67 @@ fn a_checkbox_a_colour_or_a_string_is_never_rejected() { assert!(!super::draft_rejected(&color, "FF00AA")); assert!(!super::draft_rejected(&edit_field("String"), "")); } + +/// Names every folder of a built node in the order the tree would draw them, depth first. +/// +/// Written as its own walk rather than by calling the renderer, so the assertion below compares the +/// structure with an expectation instead of with the production traversal. +fn folder_order(node: &super::FolderNode, prefix: &str) -> Vec { + let mut out = Vec::new(); + for (name, child) in node.children() { + let path = match prefix.is_empty() { + true => name.to_string(), + false => format!("{prefix}/{name}"), + }; + out.push(path.clone()); + out.extend(folder_order(child, &path)); + } + out +} + +/// The core's strategy list is an ORDER the operator arranged, and moonproto synchronizes it as the +/// row sequence of a Full snapshot. So a folder belongs where it first appears in that sequence — +/// not where a byte-wise alphabet would put it, which is what the previous `BTreeMap` did and which +/// sorted `Zeta` above `alpha` and every Cyrillic name after every Latin one. +#[test] +fn folders_follow_the_cores_own_strategy_order() { + let rows = [ + row(1, "Zeta", 0, false, true), + row(2, "alpha", 0, false, true), + row(3, "Zeta/inner", 0, false, true), + row(4, "омега", 0, false, true), + row(5, "alpha", 0, false, true), + ]; + let node = super::build_node(rows.iter()); + assert_eq!( + folder_order(&node, ""), + vec!["Zeta", "Zeta/inner", "alpha", "омега"] + ); +} + +/// A folder's strategies stay in the order the core sent them, which is the same claim one level +/// down: the rows inside one folder are a sequence, not a set. +#[test] +fn strategies_inside_a_folder_keep_the_cores_order() { + let rows = [ + row(7, "a", 0, false, true), + row(3, "a", 0, false, true), + row(5, "a", 0, false, true), + ]; + let node = super::build_node(rows.iter()); + let (_, folder) = node.children().next().expect("one folder"); + let ids: Vec = folder.strategies.iter().map(|r| r.id).collect(); + assert_eq!(ids, vec![7, 3, 5]); +} + +/// An empty folder appears in no strategy's path, so it has no place in the core's order and is +/// appended after the folders that do. Two rows are the point of the case: `ensure_folder` must +/// find the existing `a` rather than append a second one beside it. +#[test] +fn an_empty_folder_is_appended_without_duplicating_a_live_one() { + let rows = [row(1, "b", 0, false, true), row(2, "a", 0, false, true)]; + let mut node = super::build_node(rows.iter()); + super::ensure_folder(&mut node, &["a".to_string(), "ghost".to_string()]); + super::ensure_folder(&mut node, &["zz".to_string()]); + assert_eq!(folder_order(&node, ""), vec!["b", "a", "a/ghost", "zz"]); +} diff --git a/crates/moon-ui-gpui/src/strategies/mod.rs b/crates/moon-ui-gpui/src/strategies/mod.rs index 9cea9386..d29be88d 100644 --- a/crates/moon-ui-gpui/src/strategies/mod.rs +++ b/crates/moon-ui-gpui/src/strategies/mod.rs @@ -189,6 +189,11 @@ pub struct StrategiesView { selected_folder: Option<(CoreId, String)>, /// Empty UI folders before their first strategy is added, keyed by core and slash-separated path. ui_folders: HashSet<(CoreId, String)>, + /// Strategy orders sent to a core and not yet echoed back by it, keyed by core. + /// + /// The tree draws these instead of the core's own sequence until it answers — see + /// [`tree::reorder`] for why the answer cannot simply be waited for. + pending_order: HashMap, /// Active create, rename, or confirmation modal for a tree operation. op: Option, /// Create/rename modal input, recreated on each opening to use the current initial value. @@ -265,6 +270,15 @@ impl Render for StrategiesView { // rate, and rebuilding the whole adapter for it measured 1.0-1.3 ms per frame on a live // account. `tree::cache` owns the signature and the argument for why it is complete. let tree_timer = crate::diag::timer(); + // Before the signature, because the signature covers the overlay this may drop, and a frame + // that hashed an overlay it then stopped drawing would cache the wrong tree under it. + // + // Here as well as in the backend observer: an unconfirmed order's deadline has to elapse for + // a core that has gone quiet, and a quiet core is exactly the one that raises no notify. + if !self.pending_order.is_empty() { + let backend = self.backend.clone(); + self.reconcile_pending_order(backend.read(cx).session.store()); + } let sig = { let backend = self.backend.read(cx); let store = backend.session.store(); diff --git a/crates/moon-ui-gpui/src/strategies/state.rs b/crates/moon-ui-gpui/src/strategies/state.rs index f59941f4..917344e1 100644 --- a/crates/moon-ui-gpui/src/strategies/state.rs +++ b/crates/moon-ui-gpui/src/strategies/state.rs @@ -206,6 +206,12 @@ fn strategies_sig(b: &Backend, workspace_cores: Option<&[CoreId]>) -> u64 { a.wrapping_mul(31) .wrapping_add(c.strategies_rev) .wrapping_mul(31) + // The core's folder tree, which moves on its own: an empty folder created or + // deleted changes no strategy, and without this the window would not repaint for + // it — not even for the confirmation of a folder it created itself. It is also what + // makes `reconcile_ui_folders` run at the moment its new rule becomes true. + .wrapping_add(c.folders_rev) + .wrapping_mul(31) .wrapping_add(c.schema_rev) .wrapping_mul(31) .wrapping_add(c.strategy_edit_rev) @@ -339,6 +345,11 @@ impl StrategiesView { marker_moved = true; } } + // Asked on EVERY backend tick, not only on a strategy change: a reorder the core + // silently drops produces no strategy change at all, and its overlay would then outlive + // its window with nothing to notice. Costs a walk of a map that is empty except in the + // seconds after the operator pressed a move button. + let order_dropped = this.reconcile_pending_order(b.session.store()); if strategies_changed || goto { if strategies_changed { this.reconcile_ui_folders(b.session.store()); @@ -348,11 +359,12 @@ impl StrategiesView { this.clamp_selected_section(cx); this.persist_session(cx); cx.notify(); - } else if marker_moved { - // The arm above already repainted. This one covers the case it does not reach: + } else if marker_moved || order_dropped { + // The arm above already repainted. This one covers the cases it does not reach: // a core the preset HIDES connecting or leaving moves the marker's counts while // `strategies_sig` — which folds only over cores the scope already shows — does - // not budge. + // not budge; and an expired reorder overlay changes what the tree draws without + // any core having sent anything. cx.notify(); } }) @@ -546,6 +558,10 @@ impl StrategiesView { .as_ref() .map(|s| s.ui_folders.clone()) .unwrap_or_default(), + // Deliberately not restored from the session snapshot: an order sent before the window + // closed was either confirmed by the core — in which case the store already holds it — + // or lost, and a reopened window must not re-assert it. + pending_order: HashMap::new(), op: None, op_input: None, op_input_init: String::new(), diff --git a/crates/moon-ui-gpui/src/strategies/tree/cache.rs b/crates/moon-ui-gpui/src/strategies/tree/cache.rs index 6dee9a9d..549a1036 100644 --- a/crates/moon-ui-gpui/src/strategies/tree/cache.rs +++ b/crates/moon-ui-gpui/src/strategies/tree/cache.rs @@ -95,7 +95,8 @@ impl TreeCache { /// The two halves are the store and the window's own state: /// /// * per core, in the order the window lists them: its id, its display name, venue presence and -/// identity/caption fields, `strategies_rev` (the strategy snapshot), and the rendered +/// identity/caption fields, `strategies_rev` (the strategy snapshot), `folders_rev` (the +/// core's own folder tree, which carries the folders no strategy implies), and the rendered /// open-order digest. A core appearing, disappearing or being renamed moves the list itself. /// * per window field: venue grouping, the filter — search, kind, direction, EXCHANGE and /// active-only — the three expansion sets plus the Auto rail overlay, the UI-only @@ -136,6 +137,10 @@ pub(crate) fn data_sig( continue; }; cd.strategies_rev.hash(&mut h); + // The folder tree, which the build reads for the folders holding no strategy. Its own + // counter rather than `strategies_rev`: an empty folder created or deleted moves nothing + // about the strategies, and that folder is exactly what this input contributes. + cd.folders_rev.hash(&mut h); open_orders_digest(cd).hash(&mut h); } let store_sig = h.finish(); @@ -158,6 +163,16 @@ pub(crate) fn data_sig( unordered(view.expanded_folders.iter()).hash(&mut h); unordered(view.expanded_deleted.iter()).hash(&mut h); unordered(view.ui_folders.iter()).hash(&mut h); + // The unconfirmed orders themselves, each folded over its own SEQUENCE. A cheaper key — which + // cores carry one, or how long each is — would collide on the case this exists for: a second + // move pressed before the core answered replaces one overlay with another of the same length on + // the same core, and the tree would then keep drawing the arrangement from the press before. + unordered( + view.pending_order + .iter() + .map(|(core, pending)| (*core, pending.ids())), + ) + .hash(&mut h); unordered(view.sel.iter()).hash(&mut h); let staged = unordered(view.staged.iter()); staged.hash(&mut h); diff --git a/crates/moon-ui-gpui/src/strategies/tree/dialogs.rs b/crates/moon-ui-gpui/src/strategies/tree/dialogs.rs index 2dc967b0..ad41283a 100644 --- a/crates/moon-ui-gpui/src/strategies/tree/dialogs.rs +++ b/crates/moon-ui-gpui/src/strategies/tree/dialogs.rs @@ -604,7 +604,7 @@ impl StrategiesView { core, ) { - self.add_ui_folder(core, &target, name.trim()); + self.create_folder(core, &target, name.trim(), cx); self.persist_session(cx); } } @@ -868,6 +868,10 @@ impl StrategiesView { ) { return Ok(()); } + let mut new_path = old_path.to_vec(); + if let Some(leaf) = new_path.last_mut() { + *leaf = new_name.to_string(); + } let moves = { let store = self.backend.read(cx).session.store(); let Some(cd) = store.core(core) else { @@ -875,7 +879,14 @@ impl StrategiesView { }; ops::rename_folder(&cd.strategies, old_path, new_name) }; - self.backend.read(cx).session.move_strategies(core, moves)?; + // The subtree that moved travels WITH the rows, because rewriting their paths leaves the + // old folder behind on a core that keeps folders of its own. A folder holding no strategy + // has no rows at all, and its rename is this same edit with an empty move list. + self.backend.read(cx).session.move_strategies( + core, + moves, + Some((ops::join_path(old_path), ops::join_path(&new_path))), + )?; // Rename an empty UI-only folder locally only after the move command succeeds. self.rename_ui_folder(core, old_path, new_name); self.persist_session(cx); @@ -970,10 +981,24 @@ impl StrategiesView { ) { return Ok(()); } - self.backend - .read(cx) - .session - .delete_folder(core, ops::join_path(path))?; + // Two shapes, and which one applies is decided by what the folder HOLDS, not only by what + // the core can do. Omission from the desired tree removes a folder and nothing else — the + // core keeps any folder a strategy still occupies, and moonproto re-adds it — so it reaches + // exactly the folder the legacy command cannot: an empty one on a core that keeps a tree. + // A folder with strategies in it still goes the legacy way, which deletes the rows with it. + let by_omission = { + let store = self.backend.read(cx).session.store(); + store + .core(core) + .is_some_and(|cd| cd.folders.editable && !ops::has_row_under(&cd.strategies, path)) + }; + let backend = self.backend.read(cx); + match by_omission { + true => backend + .session + .remove_core_folder(core, ops::join_path(path))?, + false => backend.session.delete_folder(core, ops::join_path(path))?, + } self.remove_ui_folder(core, path); self.persist_session(cx); Ok(()) diff --git a/crates/moon-ui-gpui/src/strategies/tree/dnd.rs b/crates/moon-ui-gpui/src/strategies/tree/dnd.rs index 549c9db4..e93c815d 100644 --- a/crates/moon-ui-gpui/src/strategies/tree/dnd.rs +++ b/crates/moon-ui-gpui/src/strategies/tree/dnd.rs @@ -160,11 +160,14 @@ impl StrategiesView { .unwrap_or_default(); ops::move_to(&rows, &target) }; - if let Err(error) = self - .backend - .read(cx) - .session - .move_strategies(target_core, moves) + // No folder tree: dragging strategies OUT of a folder must not delete that folder. The + // core keeps an emptied folder now, which is the behaviour to preserve — a tree omitting + // it would take it away as a side effect of moving rows. + if let Err(error) = + self.backend + .read(cx) + .session + .move_strategies(target_core, moves, None) { log::warn!("move strategies failed: {error}"); return; @@ -220,22 +223,28 @@ impl StrategiesView { } let path = drag.path.clone(); if drag.core == target_core { + let mut moved_to = target.clone(); + moved_to.extend(path.last().cloned()); let moves = { let store = self.backend.read(cx).session.store(); - store - .core(target_core) - .map(|c| ops::move_folder(&c.strategies, &path, &target)) - .unwrap_or_default() + let Some(cd) = store.core(target_core) else { + return; + }; + ops::move_folder(&cd.strategies, &path, &target) }; - if moves.is_empty() { - return; // Reject self/descendant targets and empty folders. + // Rejects a drop onto the folder itself or into its own subtree, where source and + // destination are the same place. An EMPTY folder is no longer rejected here: it has no + // rows to move, and its relocation travels as the subtree intent alone. + if moves.is_empty() && (moved_to == path || target.starts_with(&path)) { + return; } - if let Err(error) = self - .backend - .read(cx) - .session - .move_strategies(target_core, moves) - { + // Same pairing as a rename: without the subtree the folder's old path survives on the + // core as an empty folder, now that empty folders are something it can hold. + if let Err(error) = self.backend.read(cx).session.move_strategies( + target_core, + moves, + Some((ops::join_path(&path), ops::join_path(&moved_to))), + ) { log::warn!("move strategy folder failed: {error}"); return; } diff --git a/crates/moon-ui-gpui/src/strategies/tree/menu.rs b/crates/moon-ui-gpui/src/strategies/tree/menu.rs index 3a4afb12..68bc332e 100644 --- a/crates/moon-ui-gpui/src/strategies/tree/menu.rs +++ b/crates/moon-ui-gpui/src/strategies/tree/menu.rs @@ -145,6 +145,42 @@ impl StrategiesView { ); } MenuTarget::Strategy(id) => { + // Acts on the SELECTION, which already contains this row: opening the menu on an + // unselected strategy focuses it first (`strategy_row`). So right-clicking one row + // moves that row, and right-clicking inside a multi-selection moves the block. + let (can_up, can_down) = { + let backend = self.backend.read(cx); + let store = backend.session.store(); + self.move_availability(store, backend.session.core_venues()) + }; + for (step, enabled) in + [(ops::MoveStep::Up, can_up), (ops::MoveStep::Down, can_down)] + { + let (key, label, chord) = match step { + ops::MoveStep::Up => ( + "move-up", + t!("strat.menu_move_up"), + t!("strat.move_up_chord"), + ), + ops::MoveStep::Down => ( + "move-down", + t!("strat.menu_move_down"), + t!("strat.move_down_chord"), + ), + }; + items.push( + MoonMenuItem::with_key(key, label.to_string()) + .right_label(chord.to_string()) + .disabled(!enabled) + .on_click({ + let view = view.clone(); + move |_, window, app| { + window.close_context_menu(app); + view.update(app, |this, cx| this.move_selection(step, cx)); + } + }), + ); + } items.push( MoonMenuItem::with_key("copy-strategy", t!("strat.menu_copy").to_string()) .on_click({ diff --git a/crates/moon-ui-gpui/src/strategies/tree/mod.rs b/crates/moon-ui-gpui/src/strategies/tree/mod.rs index eafaed61..bd141d63 100644 --- a/crates/moon-ui-gpui/src/strategies/tree/mod.rs +++ b/crates/moon-ui-gpui/src/strategies/tree/mod.rs @@ -12,6 +12,7 @@ pub(crate) mod menu; pub(crate) mod moon; pub(crate) mod ops; pub(in crate::strategies) mod pane_cache; +pub(crate) mod reorder; pub(crate) mod ui; #[cfg(test)] @@ -575,6 +576,10 @@ impl StrategiesView { let measured_label_width = pane.footer_label_width; // Five native leading icons remain in both densities. The full state additionally reserves // their label gaps, six group gaps, outer padding, and the hairline divider. + // + // The two move buttons are counted separately below because they never take a label: they + // cost their own width and one group gap each in BOTH densities, and leaving them out of + // this sum would let the labelled state be chosen for a row that no longer fits it. let action_icon_width = (design::font_value(cx, design::ACTION_LABEL_BASE) + 1.0).clamp(10.0, 14.0); // Action size ships with pad_x = 0. Labeled footer buttons opt into the same 7-unit @@ -583,7 +588,9 @@ impl StrategiesView { let fixed_width = 5.0 * action_icon_width + design::ui_value(cx, 5.0 * 6.0 + 6.0 * design::CHROME_GAP + 16.0) + 1.0 - + labeled_pad; + + labeled_pad + + 2.0 * design::glyph_btn_w(cx) + + 2.0 * design::ui_value(cx, design::CHROME_GAP); let show_labels = ui::footer_labels_fit(self.panels.tree_w, fixed_width, measured_label_width); @@ -657,7 +664,7 @@ impl StrategiesView { // The footer's leading icons carry no text of their own; its action labels are prose. // `staged_slot` pins its mixed caption/count node back to mono above. .font_family(design::ui_font()) - .child(self.selection_toolbar(store, show_labels, !cores.is_empty(), cx)) + .child(self.selection_toolbar(store, show_labels, !cores.is_empty(), pane.moves, cx)) .child(design::chrome_divider(cx, MoonPalette::active(cx))) .child(staged_slot) .child(right) diff --git a/crates/moon-ui-gpui/src/strategies/tree/moon.rs b/crates/moon-ui-gpui/src/strategies/tree/moon.rs index 21034fdd..8656b9b7 100644 --- a/crates/moon-ui-gpui/src/strategies/tree/moon.rs +++ b/crates/moon-ui-gpui/src/strategies/tree/moon.rs @@ -23,6 +23,7 @@ use super::super::logic::{ }; use super::super::{Key, StrategiesView, moon_alpha}; use super::checks; +use super::ops; use super::ui::{ContextMenu, DragChip, FolderDrag, MenuTarget, StratDrag}; use crate::design; use moon_core::feed::StrategyRow; @@ -195,6 +196,8 @@ pub(crate) enum NodeData { checked: bool, }, Folder { + /// What the folder holds, which decides whether its caret and checkbox are drawn at all. + fill: FolderFill, core: CoreId, path: Vec, label: String, @@ -404,8 +407,8 @@ fn build_core_root( }; let mut matched: Vec<&StrategyRow> = Vec::new(); let mut any_matched = false; - for row in &cd.strategies { - counts.add(row, filter); + for (at, row) in cd.strategies.iter().enumerate() { + counts.add(row, filter, at); if filter.matches(row) { any_matched = true; if core_open { @@ -413,9 +416,30 @@ fn build_core_root( } } } - if !any_matched { + // A core with no matching strategy is still worth a row when it holds folders that hold + // none: on an account whose folders were prepared before its strategies, that is everything + // there is to show. Asked of the folders that would actually be DRAWN — a core whose every + // folder is occupied by strategies the filter removed has nothing to show and stays hidden. + let empty_folders = empty_folder_paths(view, cd, core, searching); + if !any_matched && empty_folders.is_empty() { return None; } + // Only for a core whose rows are actually built: `matched` is filled solely when the core is + // open, so resequencing it for a collapsed one is a whole-list walk nothing reads. + if let Some(pending) = core_open.then(|| view.pending_order.get(&core)).flatten() { + // A reorder this core has not echoed yet. Drawn instead of its own sequence because the + // library keeps the confirmed order until the core answers, and the operator would + // otherwise watch their own press do nothing for a whole round trip — see `tree::reorder`. + // + // Applied to the WHOLE list and filtered afterwards, not to the rows that survived the + // filter. The rule places a strategy the sent sequence never named directly after the row + // it follows, and "the row it follows" is a different row once a filter has removed its + // neighbours — so ordering the filtered list would draw an arrangement the planner, which + // reads the whole one, does not agree with. + let mut all: Vec<&StrategyRow> = cd.strategies.iter().collect(); + moon_core::feed::strategy_order::resequence(&mut all, |row| pending.rank(row.id)); + matched = all.into_iter().filter(|row| filter.matches(row)).collect(); + } let (active, total) = counts.root(); let open_orders_total = cd.orders.iter().filter(|order| !order.job_is_done).count(); @@ -431,6 +455,7 @@ fn build_core_root( searching, &counts, &matched, + &empty_folders, &mut children, data, flat, @@ -465,6 +490,89 @@ fn build_core_root( ) } +/// Folders of one core that hold no strategy, in the order the tree appends them. +/// +/// Two sources, and which one a core uses is the core's own answer: one that keeps a folder tree +/// reports its empty folders itself, and the local marks are what a core that cannot keep them — +/// or has not confirmed one yet — leaves the window to draw. +/// +/// Answered once per build and used twice: it decides both what to append and whether a core with +/// no matching strategy is worth a row at all. +/// +/// Occupancy is derived from the strategies HERE rather than read from [`FolderCounts`], and that +/// is not duplication: a collapsed core's counts are totals-only and hold no folder at all, so +/// asking them would call every folder of every collapsed core empty. The walk costs nothing on +/// the ordinary core, which reports no folders and holds no marks and returns below immediately. +/// +/// Args: +/// view: Window holding the local marks. +/// cd: The core's live data, including the tree it reports. +/// core: Core being built. +/// searching: Whether a text query is narrowing the tree. +/// +/// Returns: +/// Segment paths of the folders to draw as empty; always empty while searching, since an empty +/// folder matches no query and cannot contain a match. +fn empty_folder_paths( + view: &StrategiesView, + cd: &moon_core::session::store::CoreData, + core: CoreId, + searching: bool, +) -> Vec> { + if searching { + return Vec::new(); + } + let marks = view.ui_folder_paths(core); + let reported: Vec> = match cd.folders.supported { + false => Vec::new(), + true => cd + .folders + .paths + .iter() + // Paths MoonProto itself would refuse are skipped, and that is not a formality: for a + // strategy in MoonBot's `"EMA / ORGANIC"` — one folder there — moonproto's own state + // adds the split halves as parent folders, so the tree reports `"EMA "`. Drawn, that is + // a folder which exists on no core and which no edit could ever name. + .filter(|path| moon_core::feed::folder_tree::sendable([path.as_str()].into_iter())) + .map(|path| ops::split_path(path)) + .collect(), + }; + if marks.is_empty() && reported.is_empty() { + return Vec::new(); + } + + // Every folder the strategies occupy, folded as the core folds them when deciding whether two + // spellings are one folder. + let mut occupied: std::collections::HashSet = std::collections::HashSet::new(); + for row in &cd.strategies { + let mut key = String::new(); + for segment in ops::path_segments(&row.folder_path) { + if !key.is_empty() { + key.push('/'); + } + key.push_str(segment); + occupied.insert(key.to_lowercase()); + } + } + + let mut empty: Vec> = marks + .into_iter() + .chain(reported) + .filter(|parts| !parts.is_empty() && !occupied.contains(&parts.join("/").to_lowercase())) + .collect(); + // One order over both sources: the core lists its folders in an order the protocol calls + // meaningless, and the local marks come out of a set, so without this two frames drawing + // identical data would place the same folders differently. Folded first so that a mark and a + // reported path differing only in case land together — and are then deduped as the one folder + // they are. + empty.sort_by_cached_key(|parts| { + let joined = parts.join("/"); + (joined.to_lowercase(), joined) + }); + empty.dedup_by(|a, b| a.join("/").to_lowercase() == b.join("/").to_lowercase()); + empty +} + /// Builds the folder and strategy rows of one open core, followed by its Deleted folder. #[allow(clippy::too_many_arguments)] fn build_core_subtree( @@ -475,6 +583,7 @@ fn build_core_subtree( searching: bool, counts: &FolderCounts, matched: &[&StrategyRow], + empty_folders: &[Vec], children: &mut Vec, data: &mut HashMap, flat: &mut Vec, @@ -491,17 +600,33 @@ fn build_core_subtree( open: core_paths(&view.expanded_folders, core), }; - // Build the folder tree from visible strategies plus empty UI-only folders. + // Build the folder tree from visible strategies plus empty UI-only folders. `matched` holds + // the core's own strategy order, which is what places the folders that hold strategies. let mut root = build_node(matched.iter().copied()); - for parts in view.ui_folder_paths(core) { - ensure_folder(&mut root, &parts); + for parts in empty_folders { + ensure_folder(&mut root, parts); } + // Folders the core keeps in its own tree, folded for the case-insensitive compare it uses. An + // empty folder outside that set exists only in this window, which is what its row says. Asked + // of `supported` and NOT of `editable`: a core whose tree cannot be edited still KEEPS the + // folders it reports, and calling one of those local would tell the operator it disappears on + // restart when it does not. + let confirmed_folders: std::collections::HashSet = match cd.folders.supported { + false => std::collections::HashSet::new(), + true => cd + .folders + .paths + .iter() + .map(|path| ops::join_path(&ops::split_path(path)).to_lowercase()) + .collect(), + }; let mut prefix: Vec = Vec::new(); convert_node( &root, core, counts, + &confirmed_folders, &order_counts, &selected_ids, &folders, @@ -627,6 +752,7 @@ fn convert_node( node: &super::super::logic::FolderNode, core: CoreId, counts: &FolderCounts, + confirmed_folders: &std::collections::HashSet, order_counts: &HashMap, selected_ids: &Rc<[u64]>, folders: &FolderSets<'_>, @@ -640,11 +766,33 @@ fn convert_node( flat: &mut Vec, expanded: &mut Vec, ) { - for (name, child) in &node.children { - prefix.push(name.clone()); - // One joined path keeps the node id, expansion probe, selection comparison, and count - // lookup on the same folder identity without repeated allocation. - let path = prefix.join("/"); + // Ordered by where each folder's first strategy sits in the core's OWN list — not by where its + // first visible one sits, which is what the child order alone would say and which would let a + // search box rearrange the tree. An empty folder has no such place and keeps the order it was + // appended in, after every folder that does. One allocation per child per level, inside a build + // the frame cache already skips on an unchanged signature. + let parent = prefix.join("/"); + let mut children: Vec<(usize, String, &str, &super::super::logic::FolderNode)> = node + .children() + .map(|(name, child)| { + // One joined path per child for the whole level. It decides the order here and is then + // handed to the loop, which needs the same string for this node's id, its expansion + // probe, its selection comparison and its count lookup. + let path = match parent.is_empty() { + true => name.to_string(), + false => format!("{parent}/{name}"), + }; + ( + counts.order_of(&path).unwrap_or(usize::MAX), + path, + name, + child, + ) + }) + .collect(); + children.sort_by_key(|(at, _, _, _)| *at); + for (_, path, name, child) in children { + prefix.push(name.to_string()); let fid = id_folder(core, &path); let fopen = searching || folders.open.contains(path.as_str()); // Read before `path` is moved into the selection comparison below. @@ -654,6 +802,16 @@ fn convert_node( core, ); let (active, total) = counts.for_path(&path); + // Asked of the COUNTS, not of `total`, which the kind and direction filters narrow: a + // folder whose strategies are all filtered away is not an empty folder, and drawing it as + // one would take its caret away while its contents are one filter click from returning. + let fill = match counts.knows(&path) { + true => FolderFill::Populated, + false => match confirmed_folders.contains(&path.to_lowercase()) { + true => FolderFill::EmptyOnCore, + false => FolderFill::EmptyLocal, + }, + }; let mut fchildren = Vec::new(); if fopen { expanded.push(fid.clone()); @@ -661,6 +819,7 @@ fn convert_node( child, core, counts, + confirmed_folders, order_counts, selected_ids, folders, @@ -680,15 +839,16 @@ fn convert_node( NodeData::Folder { core, path: prefix.clone(), - label: name.clone(), + label: name.to_string(), active, total, selected: view.selected_folder.as_ref() == Some(&(core, path)), checked: fchecked, + fill, }, ); out.push( - MoonTreeItem::new(fid, name.clone()) + MoonTreeItem::new(fid, name.to_string()) .folder(true) .children(fchildren), ); @@ -935,11 +1095,15 @@ fn render_row( p.blue, 600.0, ToggleTarget::Core(core), + // A core root is a heading, not a folder: it keeps its caret and its bulk box even + // with nothing under it, because what it covers is the whole core. + FolderFill::Populated, step, app, ) } NodeData::Folder { + fill, core, path, label, @@ -959,10 +1123,22 @@ fn render_row( indent, label.clone(), // A folder carries no order count of its own; the core root above it owns that. - RowCounts::subtree(*active, *total, 0), - p.text_soft, + // An empty one shows no numbers at all rather than `0/0`: there is nothing to + // count, and the slot's tooltip says what the row is instead. + match fill.has_contents() { + true => RowCounts::subtree(*active, *total, 0), + false => RowCounts::empty_folder(fill.empty_tip()), + }, + // An empty folder reads as quieter than one with strategies in it, which is the + // whole of what its emptiness looks like — nothing is hidden and no glyph is + // invented for it. + match fill.has_contents() { + true => p.text_soft, + false => p.text_muted, + }, 400.0, ToggleTarget::Folder(core, path), + *fill, step, app, ) @@ -1008,6 +1184,8 @@ fn render_row( p.text_muted, 400.0, ToggleTarget::Deleted(core), + // Deleted is only ever drawn when it holds rows. + FolderFill::Populated, step, app, ) @@ -1133,6 +1311,24 @@ impl RowCounts { } } + /// Counters for a folder holding nothing: no numbers, and a tooltip that says why. + /// + /// Both slots stay reserved by the row itself, so an empty folder's caption keeps the column + /// its siblings' captions sit on. + /// + /// Args: + /// tip: What this row is, from [`FolderFill::empty_tip`]. + /// + /// Returns: + /// Empty counters carrying that tooltip. + fn empty_folder(tip: Option) -> Self { + Self { + primary: String::new(), + orders: String::new(), + tip: SharedString::from(tip.unwrap_or_default()), + } + } + /// Counters for a core's Deleted heading, which carries one number and no orders. fn deleted(count: usize) -> Self { Self { @@ -1170,6 +1366,39 @@ fn counts_slot(text: String, width: f32, color: u32, step: f32, app: &App) -> im ) } +/// What a folder row has inside it, which decides how much of a row it draws. +#[derive(Clone, Copy, PartialEq, Eq)] +pub(crate) enum FolderFill { + /// Holds at least one strategy, filtered away or not. + Populated, + /// Holds nothing, and the CORE says so — it keeps this folder in its own tree. + EmptyOnCore, + /// Holds nothing and exists only in this window: either the core cannot keep empty folders, or + /// it has not confirmed this one yet. + EmptyLocal, +} + +impl FolderFill { + /// Whether this row has anything to expand or to check. + /// + /// Both controls are omitted when it does not, because both would otherwise be live controls + /// that cannot do anything: a caret that opens onto nothing, and a box whose "check every + /// strategy below" covers no strategy. Their space is still reserved, so the caption of an + /// empty folder stays on the same column as its siblings'. + fn has_contents(self) -> bool { + matches!(self, Self::Populated) + } + + /// The tooltip an empty folder's counter carries, or `None` for a populated one. + fn empty_tip(self) -> Option { + match self { + Self::Populated => None, + Self::EmptyOnCore => Some(rust_i18n::t!("strat.folder_empty_tip").to_string()), + Self::EmptyLocal => Some(rust_i18n::t!("strat.folder_empty_local_tip").to_string()), + } + } +} + enum ToggleTarget { Core(CoreId), Folder(CoreId, Vec), @@ -1191,7 +1420,6 @@ impl ToggleTarget { } } -#[allow(clippy::too_many_arguments)] /// Render a clickable core, folder, or Deleted heading in the strategy tree. /// /// Actual folders also receive the context menu assembled inside this function. The disclosure @@ -1214,6 +1442,7 @@ impl ToggleTarget { /// /// Returns: /// The complete interactive tree row. +#[allow(clippy::too_many_arguments)] fn core_folder_row( view: &Entity, row_id: SharedString, @@ -1226,6 +1455,7 @@ fn core_folder_row( color: u32, weight: f32, target: ToggleTarget, + fill: FolderFill, step: f32, app: &App, ) -> AnyElement { @@ -1235,6 +1465,7 @@ fn core_folder_row( // The bulk checkbox stages this row's own subtree, which the core root spells as the empty // path. Deleted holds no live strategy, so it addresses no folder and carries no checkbox. // Resolved once: the row renders on every repaint, and each resolve deep-clones the path. + let expandable = fill.has_contents(); let folder_key = target.folder_key(); let check_target = folder_key.clone().map(|(core, path)| (core, path, checked)); let menu = match &target { @@ -1273,12 +1504,19 @@ fn core_folder_row( // The unscaled base rides the pane's local text step so the caret stays proportional to // the row it marks; `MoonDisclosure` still applies the UI scale on top of it internally, // so the value passed here must stay unscaled. - .child( - MoonDisclosure::glyph(expanded) + .child(match fill.has_contents() { + true => MoonDisclosure::glyph(expanded) .size(design::DISCLOSURE_GLYPH_MARKER + step) - .box_size(design::DISCLOSURE_BOX + step), - ) - .child(match check_target { + .box_size(design::DISCLOSURE_BOX + step) + .into_any_element(), + // Reserved, not omitted: the caption of an empty folder belongs on the same column as + // every sibling's, and a caret that opens onto nothing is a control that lies. + false => div() + .flex_none() + .w(design::ui_px(app, design::DISCLOSURE_BOX + step)) + .into_any_element(), + }) + .child(match check_target.filter(|_| fill.has_contents()) { Some((core, path, checked)) => { checks::bulk_check(view, &check_row_id, core, path, checked) } @@ -1338,7 +1576,13 @@ fn core_folder_row( this.selected_folder = Some((*c, String::new())); } ToggleTarget::Folder(c, path) => { - toggle(&mut this.expanded_folders, (*c, path.join("/"))); + // Nothing to open, so nothing is toggled: the caret is not drawn for an + // empty folder, and flipping hidden expansion state would still churn the + // hashed set the whole tree is cached on. Selecting it below is what a + // click on it is for. + if expandable { + toggle(&mut this.expanded_folders, (*c, path.join("/"))); + } // Match Moonbot by selecting the clicked folder for highlighting and Ctrl+C. this.selected_folder = Some((*c, path.join("/"))); } diff --git a/crates/moon-ui-gpui/src/strategies/tree/moon/tests.rs b/crates/moon-ui-gpui/src/strategies/tree/moon/tests.rs index d3815fc5..3cb23924 100644 --- a/crates/moon-ui-gpui/src/strategies/tree/moon/tests.rs +++ b/crates/moon-ui-gpui/src/strategies/tree/moon/tests.rs @@ -71,6 +71,7 @@ fn core_node(core: CoreId) -> NodeData { /// Folder row with the given path segments; must remain a live drop destination. fn folder_node(core: CoreId, path: &[&str]) -> NodeData { NodeData::Folder { + fill: super::FolderFill::Populated, core, path: path.iter().map(|p| (*p).to_string()).collect(), label: "folder".into(), diff --git a/crates/moon-ui-gpui/src/strategies/tree/ops.rs b/crates/moon-ui-gpui/src/strategies/tree/ops.rs index 68046674..5fae8325 100644 --- a/crates/moon-ui-gpui/src/strategies/tree/ops.rs +++ b/crates/moon-ui-gpui/src/strategies/tree/ops.rs @@ -511,5 +511,104 @@ pub fn move_to(rows: &[&StrategyRow], target: &[String]) -> Vec<(u64, String)> { rows.iter().map(|r| (r.id, path.clone())).collect() } +// --- Reordering inside a folder ------------------------------------------- + +/// Which way one reorder step moves the selection. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum MoveStep { + /// One place towards the start of the folder. + Up, + /// One place towards its end. + Down, +} + +/// Move every selected strategy one place inside its own folder, and return the core's new order. +/// +/// A core's strategy list is an ORDER the operator arranged in MoonBot, and moonproto synchronizes +/// it as the row sequence of a Full snapshot (`docs/strats.md`, "Strategy Order"). So a reorder is +/// not a local view preference: the whole list goes back to the core in its new sequence. +/// +/// Two rules keep it behaving like every other list reorder: +/// +/// * A row moves only inside its own folder. The protocol asks that one folder's strategies stay +/// a contiguous group, and a row that could walk out the top of its folder would silently +/// change what folder it is in — that is what dragging is for. +/// * Only rows the tree currently DRAWS take part. With a filter on, "up" therefore means above +/// the row visibly above it, while a hidden strategy keeps the slot it holds in the core's own +/// list. Moving against invisible neighbours instead would spend a press on nothing. +/// +/// A block of adjacent selected rows moves together and stops at its folder's edge, one row at a +/// time, which is the usual behaviour of such a control. +/// +/// Args: +/// rows: The core's complete strategy list, in the order the tree currently shows it. +/// selected: Strategy ids the operator is moving. +/// visible: Whether one row is drawn under the active filter. +/// step: Direction to move the selection. +/// +/// Returns: +/// The core's complete new id sequence, or `None` when nothing in the selection can move — +/// an empty selection, or a block already sitting against the edge of its folder. +pub fn reorder_step( + rows: &[&StrategyRow], + selected: &HashSet, + visible: impl Fn(&StrategyRow) -> bool, + step: MoveStep, +) -> Option> { + // Per folder, the INDEXES of the rows that folder draws. Indexes rather than ids because the + // permutation is written back into exactly these slots, which leaves every hidden row and every + // other folder's row untouched wherever it sits. + let mut slots_by_folder: Vec> = Vec::new(); + let mut folder_at: std::collections::HashMap = std::collections::HashMap::new(); + for (at, row) in rows.iter().enumerate() { + if !visible(row) { + continue; + } + // The tree's own folder identity, so a row groups with the folder it is drawn under even + // when the wire spelled the path with a backslash or a doubled separator. + let folder = join_path(&split_path(&row.folder_path)); + let group = match folder_at.get(&folder) { + Some(&group) => group, + None => { + folder_at.insert(folder, slots_by_folder.len()); + slots_by_folder.push(Vec::new()); + slots_by_folder.len() - 1 + } + }; + slots_by_folder[group].push(at); + } + + let mut order: Vec = rows.iter().map(|row| row.id).collect(); + let mut moved = false; + for slots in &slots_by_folder { + let mut ids: Vec = slots.iter().map(|&at| order[at]).collect(); + match step { + // Front to back going up, back to front going down: each selected row is swapped with + // the neighbour beyond it only when that neighbour is NOT itself selected, so a block + // shifts by one and cannot pass through its own members. + MoveStep::Up => { + for at in 1..ids.len() { + if selected.contains(&ids[at]) && !selected.contains(&ids[at - 1]) { + ids.swap(at - 1, at); + moved = true; + } + } + } + MoveStep::Down => { + for at in (0..ids.len().saturating_sub(1)).rev() { + if selected.contains(&ids[at]) && !selected.contains(&ids[at + 1]) { + ids.swap(at, at + 1); + moved = true; + } + } + } + } + for (&at, id) in slots.iter().zip(ids) { + order[at] = id; + } + } + moved.then_some(order) +} + #[cfg(test)] mod tests; diff --git a/crates/moon-ui-gpui/src/strategies/tree/ops/tests.rs b/crates/moon-ui-gpui/src/strategies/tree/ops/tests.rs index 84f0a858..cb1cf5ff 100644 --- a/crates/moon-ui-gpui/src/strategies/tree/ops/tests.rs +++ b/crates/moon-ui-gpui/src/strategies/tree/ops/tests.rs @@ -359,3 +359,90 @@ fn move_to_flattens_to_target() { assert!(edits.contains(&(1, "dest".to_string()))); assert!(edits.contains(&(2, "dest".to_string()))); } + +// --- reorder_step --------------------------------------------------------- + +/// Runs one step over a core list, with every row visible unless `hidden` names it. +fn step(rows: &[StrategyRow], selected: &[u64], hidden: &[u64], dir: MoveStep) -> Option> { + let refs: Vec<&StrategyRow> = rows.iter().collect(); + let picked: HashSet = selected.iter().copied().collect(); + reorder_step(&refs, &picked, |row| !hidden.contains(&row.id), dir) +} + +/// The whole core list comes back, not just the moved pair: the sequence IS the payload the core +/// is sent, so a partial list would tell it to rearrange everything else too. +#[test] +fn one_step_swaps_with_the_neighbour_and_returns_the_whole_list() { + let rows = vec![ + row(1, "a", "f", false), + row(2, "b", "f", false), + row(3, "c", "f", false), + ]; + assert_eq!(step(&rows, &[3], &[], MoveStep::Up), Some(vec![1, 3, 2])); + assert_eq!(step(&rows, &[1], &[], MoveStep::Down), Some(vec![2, 1, 3])); +} + +/// A row at the edge of its folder has nowhere to go, and `None` is what disables the button rather +/// than sending the core an order identical to the one it holds. +#[test] +fn a_row_against_the_edge_of_its_folder_reports_nothing_to_do() { + let rows = vec![row(1, "a", "f", false), row(2, "b", "f", false)]; + assert_eq!(step(&rows, &[1], &[], MoveStep::Up), None); + assert_eq!(step(&rows, &[2], &[], MoveStep::Down), None); + assert_eq!(step(&rows, &[], &[], MoveStep::Up), None); +} + +/// Folders are independent lists. A row must not walk out of the top of its own folder into the one +/// drawn above it — that would change what folder it is in, silently, through a button whose whole +/// promise is that it only moves things around. +#[test] +fn a_row_never_leaves_its_own_folder() { + let rows = vec![ + row(1, "a", "one", false), + row(2, "b", "two", false), + row(3, "c", "two", false), + ]; + assert_eq!(step(&rows, &[2], &[], MoveStep::Up), None); + // ... and moving inside the second folder leaves the first one's row exactly where it sits. + assert_eq!(step(&rows, &[3], &[], MoveStep::Up), Some(vec![1, 3, 2])); +} + +/// The same folder spelled two ways is ONE folder in the tree, so it has to be one list here too. +#[test] +fn folder_identity_follows_the_trees_own_path_split() { + let rows = vec![ + row(1, "a", "deep/inner", false), + row(2, "b", "deep\\inner", false), + ]; + assert_eq!(step(&rows, &[2], &[], MoveStep::Up), Some(vec![2, 1])); +} + +/// A selected block moves as a block and stops at the edge together, instead of collapsing onto +/// itself when its members swap through each other. +#[test] +fn a_block_moves_together_and_stops_at_the_edge() { + let rows = vec![ + row(1, "a", "f", false), + row(2, "b", "f", false), + row(3, "c", "f", false), + row(4, "d", "f", false), + ]; + assert_eq!( + step(&rows, &[3, 4], &[], MoveStep::Up), + Some(vec![1, 3, 4, 2]) + ); + assert_eq!(step(&rows, &[1, 2], &[], MoveStep::Up), None); +} + +/// With a filter on, "up" means above the row visibly above it. A hidden strategy keeps the slot it +/// holds in the core's list — the press moves the selection past it, not into its place. +#[test] +fn a_hidden_row_keeps_its_slot_while_the_visible_ones_move_around_it() { + let rows = vec![ + row(1, "a", "f", false), + row(2, "hidden", "f", false), + row(3, "c", "f", false), + ]; + // Slots 0 and 2 are the drawn ones; they exchange, and slot 1 still holds the hidden row. + assert_eq!(step(&rows, &[3], &[2], MoveStep::Up), Some(vec![3, 2, 1])); +} diff --git a/crates/moon-ui-gpui/src/strategies/tree/pane_cache.rs b/crates/moon-ui-gpui/src/strategies/tree/pane_cache.rs index 64b1b0ca..d2f61bf1 100644 --- a/crates/moon-ui-gpui/src/strategies/tree/pane_cache.rs +++ b/crates/moon-ui-gpui/src/strategies/tree/pane_cache.rs @@ -63,6 +63,8 @@ pub(in crate::strategies) struct LeftPaneFrame { pub(super) footer_label_width: f32, /// Staged checkbox count the footer both measures against and renders. pub(super) staged: usize, + /// Whether the footer's two move buttons have anything to do, as `(up, down)`. + pub(super) moves: (bool, bool), } /// Everything the Start/Stop plan reads, as one comparable key. @@ -95,6 +97,24 @@ pub(in crate::strategies) struct PaneCache { exchanges: Option<(u64, SharedString, ExchangeList)>, plan: Option<(PlanKey, Arc)>, labels: Option<(LabelKey, f32)>, + moves: Option<(MoveKey, (bool, bool))>, +} + +/// Everything the move buttons' enablement reads, as one comparable key. +/// +/// Both halves of the tree signature, because the answer depends on the core's strategy list AND on +/// the window: the selection, the filter that decides which rows are drawn, and any unconfirmed +/// order overlaying them. All three live in the view half already. +#[derive(Clone, Copy, PartialEq, Eq)] +struct MoveKey { + /// Cores and their strategy snapshots. + store: u64, + /// Selection, filter, expansion, and the unconfirmed-order overlay. + view: u64, + /// The workspace scope itself, which neither half of the tree signature carries: a preset can + /// change which cores the plan admits while naming exactly the cores already listed, moving + /// nothing the other two hash. + workspace: u64, } impl StrategiesView { @@ -121,7 +141,42 @@ impl StrategiesView { plan: self.pane_plan(sig, cores, cx), footer_label_width: self.pane_footer_label_width(staged, cx), staged, + moves: self.pane_moves(sig, cx), + } + } + + /// Enablement of the two move buttons, recomputed only when its inputs moved. + /// + /// Retained for the same reason as everything else here: answering it walks every strategy of + /// every core the selection touches and allocates a canonical folder path per row, and this row + /// is rebuilt on every hover repaint. Asked once per button, per frame, it was the largest new + /// per-frame cost in the pane. + /// + /// Args: + /// sig: Tree signature already computed for this frame. + /// cx: Application context used to read the store. + /// + /// Returns: + /// Whether a move up, and a move down, would rearrange anything. + fn pane_moves(&mut self, sig: TreeSig, cx: &App) -> (bool, bool) { + let key = MoveKey { + store: sig.store, + view: sig.view, + workspace: workspace_digest(self.workspace_cores.as_deref()), + }; + if let Some((cached, moves)) = &self.pane_cache.moves + && *cached == key + { + return *moves; } + crate::diag::bump(&crate::diag::STRAT_PANE_BUILD); + let built = { + let backend = self.backend.read(cx); + let store = backend.session.store(); + self.move_availability(store, backend.session.core_venues()) + }; + self.pane_cache.moves = Some((key, built)); + built } /// Strategy kinds across the visible cores, rebuilt only when the store moved. @@ -352,3 +407,22 @@ fn footer_label_width(cx: &App, staged: usize) -> f32 { } width } + +/// Fold the effective workspace scope into one comparable number. +/// +/// Ordered, because the scope IS an ordered list of ids and two different scopes must not collide; +/// `None` — unscoped Classic — is distinct from an empty scope, which admits no core at all. +/// +/// Args: +/// cores: Concrete scoped core ids, or `None` when the window is unscoped. +/// +/// Returns: +/// A digest that changes whenever the scope does. +fn workspace_digest(cores: Option<&[CoreId]>) -> u64 { + match cores { + None => 0, + Some(cores) => cores + .iter() + .fold(1u64, |acc, core| acc.wrapping_mul(31).wrapping_add(*core)), + } +} diff --git a/crates/moon-ui-gpui/src/strategies/tree/pane_cache/tests.rs b/crates/moon-ui-gpui/src/strategies/tree/pane_cache/tests.rs index ec0677ce..1aa5f359 100644 --- a/crates/moon-ui-gpui/src/strategies/tree/pane_cache/tests.rs +++ b/crates/moon-ui-gpui/src/strategies/tree/pane_cache/tests.rs @@ -135,3 +135,131 @@ fn the_selection_toolbar_takes_core_visibility_from_its_caller() { ); assert!(can_paste.contains("has_visible_cores")); } + +/// The same demand as the plan key, for the move buttons' enablement: every piece of WINDOW state +/// the planner touches must be covered by a named component of `MoveKey`. +/// +/// Two functions are scanned, not one. `movable_selection` reads most of it, but it reaches the +/// unconfirmed-order overlay through `displayed_rows`; whitelisting that name as if it were a field +/// would put everything behind it — today `pending_order`, tomorrow whatever is added — outside the +/// scan this test is named for. +/// +/// And covering the view HALF is only half an argument, since that half is built elsewhere. So the +/// second block below checks the other end: that `data_sig` still folds each of these fields into +/// it. Without that pair, `MoveKey` could name a component that no longer carries what it claims. +/// +/// One input the scan cannot see either way: `workspace_cores`, which the planner reads through +/// `selected_keys` and which NEITHER half of the tree signature hashes — a preset naming exactly +/// the connected cores moves no signature at all. It gets its own component, asserted at the end. +#[test] +fn the_move_key_covers_every_field_the_enablement_reads() { + /// Window state the planner reads directly, all of it inside the signature's view half. + const VIEW_FIELDS: [&str; 4] = [ + "filter", + "expanded_cores", + "rail_expanded_core", + // Read by `displayed_rows`, which is scanned alongside the planner for exactly this reason. + "pending_order", + ]; + + let reorder = include_str!("../reorder.rs"); + let body = |marker: &str| { + reorder + .split(marker) + .nth(1) + .and_then(|tail| tail.split("\n }").next()) + .expect("the scanned function must exist") + .to_string() + }; + let scanned = format!( + "{}{}", + body("fn movable_selection<"), + body("fn displayed_rows<") + ); + let packed: String = scanned.chars().filter(|c| !c.is_whitespace()).collect(); + let cache: String = include_str!("../pane_cache.rs") + .chars() + .filter(|c| !c.is_whitespace()) + .collect(); + let signature = include_str!("../cache.rs"); + + let mut missing = Vec::new(); + let mut checked = 0; + for chunk in packed.split("self.").skip(1) { + let field: String = chunk + .chars() + .take_while(|c| c.is_alphanumeric() || *c == '_') + .collect(); + // `displayed_rows` is the scanned callee itself, not an input of its own. + if field.is_empty() || field == "displayed_rows" { + continue; + } + checked += 1; + if !VIEW_FIELDS.contains(&field.as_str()) { + missing.push(field); + } + } + assert!( + checked >= 5, + "the scan found only {checked} field reads — a read moved into a helper and left this \ + test guarding less than it claims" + ); + missing.sort(); + missing.dedup(); + assert!( + missing.is_empty(), + "these inputs decide the move buttons but are absent from MoveKey, so their enablement \ + would go stale:\n{}", + missing.join("\n") + ); + + // The key names the view half... + assert!( + cache.contains("view:sig.view"), + "the view half must be a component of MoveKey, since every field above lives in it" + ); + // ... and the view half still carries each of those fields. + for field in VIEW_FIELDS { + assert!( + signature.contains(&format!("view.{field}")), + "data_sig no longer folds `{field}` into the view half, so MoveKey's claim to cover it \ + through `sig.view` is empty" + ); + } + + // The one input neither scan nor signature can account for. + assert!( + packed.contains("selected_keys(self)"), + "the selection must come from the canonical resolver, which is what applies the workspace \ + scope the component below stands for" + ); + assert!( + cache.contains("workspace:workspace_digest(self.workspace_cores"), + "the workspace scope is hashed by neither half of the tree signature, so MoveKey must \ + carry it directly" + ); +} + +/// The enablement walks every strategy of every selected core and allocates a canonical folder path +/// per row. It belongs behind the pane cache, and every render path must take the answer from its +/// caller — the same rule `the_plan_key_covers_every_field_the_plan_reads` enforces for the +/// Start/Stop plan. +/// +/// The context menu is exempt and named here so the exemption is deliberate: it is built on a +/// right-click, not on a frame. +#[test] +fn the_move_buttons_take_their_enablement_from_the_pane_cache() { + for (name, source) in [ + ("tree/ui.rs", include_str!("../ui.rs")), + ("tree/mod.rs", include_str!("../mod.rs")), + ("tree/moon.rs", include_str!("../moon.rs")), + ("tree/cache.rs", include_str!("../cache.rs")), + ("strategies/mod.rs", include_str!("../../mod.rs")), + ] { + assert!( + !source.contains("move_availability("), + "{name} must render the cached answer, not re-derive it per frame" + ); + } + assert!(include_str!("../pane_cache.rs").contains("self.move_availability(")); +} diff --git a/crates/moon-ui-gpui/src/strategies/tree/reorder.rs b/crates/moon-ui-gpui/src/strategies/tree/reorder.rs new file mode 100644 index 00000000..3f1774ad --- /dev/null +++ b/crates/moon-ui-gpui/src/strategies/tree/reorder.rs @@ -0,0 +1,345 @@ +//! Moving strategies up and down inside their folder, and holding the result on screen until the +//! core confirms it. +//! +//! A core's strategy list is an ORDER, not a set: the operator arranges it in MoonBot and moonproto +//! synchronizes that arrangement as the row sequence of a Full snapshot. The permutation itself is +//! [`ops::reorder_step`]; this module is the part that has a window — which rows a press acts on, +//! the command that carries the result to the core, and the overlay below. +//! +//! ## Why the overlay +//! +//! `sync_local_strategies` does NOT reorder moonproto's retained list. The library keeps the +//! core-confirmed order and rewrites it only from the core's own Full echo (`apply_server_order`), +//! so between the press and that echo `CoreData::strategies` still holds the OLD sequence. Drawn +//! straight, the tree would sit still for the whole round trip and — far worse — a second press +//! would be computed from the arrangement the first one already replaced, so holding the button +//! would produce one move instead of five. [`PendingOrder`] is that gap, and nothing more: it +//! overlays the sequence already sent, and it is dropped the moment the core answers. +//! +//! ## What it does not do +//! +//! Nothing tells the window that a reorder was REFUSED. The command's outcome lives on the feed +//! thread — a core whose state is not ready, a send that fails — and reaches only the log. Such an +//! overlay is retired by [`CONFIRMATION_WINDOW`] instead of by an answer, so the arrangement on +//! screen reverts silently rather than saying why. Closing that would take a result path from the +//! feed back to the view, which no strategy command has today. + +use std::collections::{HashMap, HashSet}; +use std::time::{Duration, Instant}; + +use moon_core::feed::StrategyRow; +use moon_core::session::{CoreId, CoreStore}; +use moon_core::venue::CoreVenue; + +use super::super::StrategiesView; +use super::super::filter::PreparedFilter; +use super::super::logic::selected_keys; +use super::ops::{self, MoveStep}; + +/// How long an unconfirmed order keeps overriding what the core reports. +/// +/// OUR bound, not a protocol promise. moonproto's `Edit*` lifecycle — including its 45-second +/// confirmation timeout — tracks strategy ROWS and explicitly not reorder-only actions, so a +/// reorder has no deadline of its own on the wire. The number is borrowed from that window for +/// consistency; what matters is only that one exists, because the alternative is an arrangement +/// that no core holds staying on screen for the rest of the session. +const CONFIRMATION_WINDOW: Duration = Duration::from_secs(45); + +/// One core's strategy order that has been sent and not yet echoed back. +pub(in crate::strategies) struct PendingOrder { + /// The sequence that was sent, which the tree cache hashes and a confirmation is compared to. + ids: Vec, + /// Position of each of those ids, so the tree can place a row without scanning the sequence. + ranks: HashMap, + /// When it went to the core, for [`CONFIRMATION_WINDOW`]. + sent: Instant, +} + +impl PendingOrder { + /// Record a freshly sent sequence. + fn new(ids: Vec) -> Self { + let ranks = ids + .iter() + .enumerate() + .map(|(rank, id)| (*id, rank)) + .collect(); + Self { + ids, + ranks, + sent: Instant::now(), + } + } + + /// The sent sequence, for the tree cache's signature — which must separate two orders of the + /// same ids, the case a second press before the core answers produces. + pub(in crate::strategies) fn ids(&self) -> &[u64] { + &self.ids + } + + /// Position of one strategy in the sent sequence, or `None` for an id it never named. + pub(in crate::strategies) fn rank(&self, id: u64) -> Option { + self.ranks.get(&id).copied() + } + + /// Whether `live` — the core's own current sequence — already agrees with what was sent. + /// + /// Read as: the ids the two have in COMMON appear in the live list in ascending sent position. + /// Only the shared ones, because a strategy created, deleted or restored in the meantime is not + /// a disagreement about order — without that, one unrelated create would pin the overlay open + /// for the whole confirmation window. + /// + /// Args: + /// live: The core's current strategy ids, in the order it reports them. + /// + /// Returns: + /// Whether the two sequences order their shared ids the same way. + fn confirmed_by(&self, live: impl Iterator) -> bool { + let mut previous: Option = None; + for id in live { + let Some(rank) = self.rank(id) else { continue }; + if previous.is_some_and(|prior| rank < prior) { + return false; + } + previous = Some(rank); + } + true + } +} + +/// What one core contributes to a move: its rows as the tree draws them, and the ids selected in it. +struct MovableCore<'a> { + /// Core these rows belong to. + core: CoreId, + /// Its strategies in display order — the core's own, or an unconfirmed order overlaying it. + rows: Vec<&'a StrategyRow>, + /// Strategy ids of the current selection that live in this core. + selected: HashSet, +} + +/// Everything one press acts on, resolved once for both directions. +struct MoveScope<'a> { + /// Row predicate deciding which strategies are drawn, shared by every core below. + filter: PreparedFilter, + /// The cores the tree actually shows, in the order the selection first named them. + cores: Vec>, +} + +impl StrategiesView { + /// The sequence one core's rows are currently DRAWN in: its own, unless an unconfirmed reorder + /// is overlaying it. + /// + /// This is what a reorder must be computed from, or a second press repeats the first. + /// + /// Args: + /// store: Live per-core strategy snapshots. + /// core: Core to read. + /// + /// Returns: + /// Borrowed rows in display order; empty when the core is not in the store. + fn displayed_rows<'a>(&self, store: &'a CoreStore, core: CoreId) -> Vec<&'a StrategyRow> { + let Some(data) = store.core(core) else { + return Vec::new(); + }; + let mut rows: Vec<&StrategyRow> = data.strategies.iter().collect(); + if let Some(pending) = self.pending_order.get(&core) { + // The same rule the feed and the tree apply, so all three agree on where a strategy the + // sent sequence never named belongs. + moon_core::feed::strategy_order::resequence(&mut rows, |row| pending.rank(row.id)); + } + rows + } + + /// Group the selection by core, keeping only the cores the tree actually shows. + /// + /// A core is skipped when the exchange filter has taken it off the screen or when its row is + /// collapsed: in both cases the operator cannot see the rows, so a press that rearranged them + /// would be a change nobody watched being made. + /// + /// One pass, and shared by both directions, because the two move buttons ask this same question + /// on every pane-cache miss — which is every strategy revision on any core. + /// + /// Args: + /// store: Live per-core strategy snapshots. + /// venues: Session venue identities, for the exchange filter. + /// + /// Returns: + /// The prepared row filter, and per core its drawn row order with the ids selected in it. + fn movable_selection<'a>( + &self, + store: &'a CoreStore, + venues: &HashMap, + ) -> MoveScope<'a> { + let filter = self.filter.prepare(); + // Search forces every core and folder open, exactly as the tree build reads it. + let searching = filter.searching(); + let mut cores: Vec> = Vec::new(); + for (core, id) in selected_keys(self) { + if let Some(entry) = cores.iter_mut().find(|entry| entry.core == core) { + entry.selected.insert(id); + continue; + } + if !self.filter.core_matches(venues.get(&core)) { + continue; + } + let open = searching + || super::super::state::core_is_open( + &self.expanded_cores, + self.rail_expanded_core, + core, + ); + if !open { + continue; + } + cores.push(MovableCore { + core, + rows: self.displayed_rows(store, core), + selected: HashSet::from([id]), + }); + } + MoveScope { filter, cores } + } + + /// Plan one reorder step for every core the selection reaches. + /// + /// Args: + /// store: Live per-core strategy snapshots. + /// venues: Session venue identities, for the exchange filter. + /// step: Direction the operator asked for. + /// + /// Returns: + /// `(core, complete new id sequence)` for each core that can actually move. + fn reorder_plan( + &self, + store: &CoreStore, + venues: &HashMap, + step: MoveStep, + ) -> Vec<(CoreId, Vec)> { + let scope = self.movable_selection(store, venues); + scope + .cores + .into_iter() + .filter_map(|entry| { + ops::reorder_step( + &entry.rows, + &entry.selected, + |row| scope.filter.matches(row), + step, + ) + .map(|order| (entry.core, order)) + }) + .collect() + } + + /// Whether each move button has anything to do, as `(up, down)`. + /// + /// Derived from the same rows the click acts on, so a button is enabled exactly when pressing + /// it would change something — a selection at the top of its folder disables Up and nothing + /// else. Both directions come out of ONE grouping pass; asking twice walked every strategy of + /// every selected core a second time for an answer built from identical inputs. + /// + /// Called through the pane cache rather than per frame: see [`super::pane_cache`]. + /// + /// Args: + /// store: Live per-core strategy snapshots. + /// venues: Session venue identities, for the exchange filter. + /// + /// Returns: + /// Whether a move up, and a move down, would rearrange at least one core. + pub(in crate::strategies) fn move_availability( + &self, + store: &CoreStore, + venues: &HashMap, + ) -> (bool, bool) { + let scope = self.movable_selection(store, venues); + let mut up = false; + let mut down = false; + for entry in &scope.cores { + let visible = |row: &StrategyRow| scope.filter.matches(row); + let ask = |step| ops::reorder_step(&entry.rows, &entry.selected, visible, step); + up = up || ask(MoveStep::Up).is_some(); + down = down || ask(MoveStep::Down).is_some(); + } + (up, down) + } + + /// Move the selection one place inside its folder and send the new order to each core. + /// + /// Args: + /// step: Direction the operator asked for. + /// cx: View context used to reach the session and repaint. + /// + /// Returns: + /// Nothing; a selection that cannot move, and a core that refuses the command, both leave + /// the view untouched. + pub(in crate::strategies) fn move_selection( + &mut self, + step: MoveStep, + cx: &mut gpui::Context, + ) { + let plan = { + let backend = self.backend.read(cx); + self.reorder_plan(backend.session.store(), backend.session.core_venues(), step) + }; + let mut sent = false; + for (core, order) in plan { + // Read per core rather than hoisted: `self.pending_order` is written inside this loop, + // and the borrow checker is right that the two cannot overlap. + let result = self + .backend + .read(cx) + .session + .reorder_strategies(core, order.clone()); + match result { + Ok(()) => { + self.pending_order.insert(core, PendingOrder::new(order)); + sent = true; + } + // Left OUT of `pending_order` on purpose: an overlay for a command that never + // reached the queue would show an arrangement no core will ever confirm. A + // multi-core selection applies core by core, so the ones that were queued stay + // queued — there is no arrangement spanning two cores to roll back to. + Err(error) => log::warn!("reorder strategies failed: {error}"), + } + } + if sent { + cx.notify(); + } + } + + /// Drop overlays the core has answered, or waited long enough for. + /// + /// Called from two places, and it needs both: from the backend observer, where the core's echo + /// arrives, and from render, because the deadline has to elapse even for a core that has gone + /// quiet — which is the very failure the deadline exists for, and the one case that produces no + /// backend notify at all. + /// + /// Args: + /// store: Live per-core strategy snapshots. + /// + /// Returns: + /// Whether anything was dropped, so the caller can repaint on the frame the tree stops + /// showing the overlay. + pub(in crate::strategies) fn reconcile_pending_order(&mut self, store: &CoreStore) -> bool { + if self.pending_order.is_empty() { + return false; + } + let before = self.pending_order.len(); + let now = Instant::now(); + self.pending_order.retain(|core, pending| { + // A core REMOVED FROM CONFIGURATION, which is the only thing that takes its data out of + // the store — a disconnect leaves the snapshot in place, and that overlay is retired by + // the deadline below or by the full list the core resends when it comes back. + let Some(data) = store.core(*core) else { + return false; + }; + if pending.confirmed_by(data.strategies.iter().map(|row| row.id)) { + return false; + } + now.duration_since(pending.sent) < CONFIRMATION_WINDOW + }); + before != self.pending_order.len() + } +} + +#[cfg(test)] +mod tests; diff --git a/crates/moon-ui-gpui/src/strategies/tree/reorder/tests.rs b/crates/moon-ui-gpui/src/strategies/tree/reorder/tests.rs new file mode 100644 index 00000000..2db30539 --- /dev/null +++ b/crates/moon-ui-gpui/src/strategies/tree/reorder/tests.rs @@ -0,0 +1,50 @@ +//! Unit tests for the unconfirmed-order overlay. +//! +//! Imports are explicit rather than `use super::*`: this module's ancestors re-export `gpui::*`, +//! whose `test` shadows the attribute and makes `#[test]` recurse (see `presentation/tests.rs`). + +use super::PendingOrder; + +/// The core has applied exactly what was sent, so the overlay has done its job and must go — a +/// confirmed overlay that stayed would keep outranking the core on every later change. +#[test] +fn an_echo_of_the_sent_sequence_confirms_it() { + let pending = PendingOrder::new(vec![3, 1, 2]); + assert!(pending.confirmed_by([3, 1, 2].into_iter())); + assert!(!pending.confirmed_by([1, 2, 3].into_iter())); +} + +/// A strategy created, deleted or restored between the press and the echo is not a disagreement +/// about order. Comparing the raw lists instead would leave the overlay open for its full window +/// after any unrelated create, and the tree would keep drawing a stale arrangement all that time. +#[test] +fn a_created_or_deleted_strategy_does_not_block_confirmation() { + let pending = PendingOrder::new(vec![3, 1, 2]); + // 9 was created after the press; the shared ids still read 3, 1, 2. + assert!(pending.confirmed_by([3, 1, 9, 2].into_iter())); + // 1 was deleted; what remains is still in the order that was asked for. + assert!(pending.confirmed_by([3, 2].into_iter())); + // ... but a genuine disagreement among the survivors is still one. + assert!(!pending.confirmed_by([2, 3].into_iter())); +} + +/// The sequence is what the tree cache hashes, and its one job there is to separate two orders of +/// the SAME ids — the case a second press before the core answers produces. +#[test] +fn the_sequence_is_exposed_in_order_for_the_cache() { + assert_eq!(PendingOrder::new(vec![1, 3, 2]).ids(), &[1, 3, 2]); + assert_ne!( + PendingOrder::new(vec![1, 2, 3]).ids(), + PendingOrder::new(vec![1, 3, 2]).ids() + ); +} + +/// Rank is what places a row, and an id the sequence never named has none — such a row keeps the +/// row it follows instead of taking a position the operator never chose. +#[test] +fn an_unnamed_id_has_no_rank() { + let pending = PendingOrder::new(vec![7, 4]); + assert_eq!(pending.rank(7), Some(0)); + assert_eq!(pending.rank(4), Some(1)); + assert_eq!(pending.rank(5), None); +} diff --git a/crates/moon-ui-gpui/src/strategies/tree/tests.rs b/crates/moon-ui-gpui/src/strategies/tree/tests.rs index 825ed9c6..5ed412d1 100644 --- a/crates/moon-ui-gpui/src/strategies/tree/tests.rs +++ b/crates/moon-ui-gpui/src/strategies/tree/tests.rs @@ -95,6 +95,7 @@ impl Render for StratDragHarness { checked: false, }); let folder_dest = drop_dest(&NodeData::Folder { + fill: super::moon::FolderFill::Populated, core: 7, path: vec!["desk".into(), "live".into()], label: "folder".into(), diff --git a/crates/moon-ui-gpui/src/strategies/tree/ui.rs b/crates/moon-ui-gpui/src/strategies/tree/ui.rs index 7288f161..ebd8d5a1 100644 --- a/crates/moon-ui-gpui/src/strategies/tree/ui.rs +++ b/crates/moon-ui-gpui/src/strategies/tree/ui.rs @@ -31,6 +31,24 @@ pub(super) fn footer_labels_fit( available_width >= fixed_width + measured_label_width } +/// Return whether a keystroke is the reorder chord, Ctrl+Shift with an up or down arrow. +/// +/// Compared against the whole modifier set rather than testing the two that must be down, and that +/// is load bearing for one of them: `alt-up`/`alt-down` ship as the bindings for "shift the sell +/// order's price" (`moon_core::config::hotkeys`), and one chord that rearranges a list in one +/// window and moves a live order's price in another is a trap regardless of which window has focus +/// today. +/// +/// Args: +/// modifiers: Modifier state of the keystroke. +/// key: Key name from the keystroke. +/// +/// Returns: +/// `true` for exactly Ctrl+Shift plus an arrow. +fn reorder_chord(modifiers: &Modifiers, key: &str) -> bool { + *modifiers == Modifiers::control_shift() && matches!(key, "up" | "down") +} + /// Where a paste — or a Create — should land, given the tree's two kinds of selection. /// /// The folder outranks the strategy, and that is structural rather than a preference: making a @@ -331,10 +349,46 @@ impl StrategiesView { // ── UI-only folders, empty until populated ──────────────────────────────── - pub(super) fn add_ui_folder(&mut self, core: CoreId, parent: &str, name: &str) { + /// Create an empty folder: asked of the core, and marked locally either way. + /// + /// The local mark is not a fallback but a latency answer — a core that accepts the folder still + /// has to echo its tree back, and an operator who just typed a name should not watch nothing + /// happen for a round trip. `reconcile_ui_folders` drops the mark once the core reports the + /// folder as its own; on a core that keeps no folder tree the mark is all there ever is. + /// + /// Only the PATH goes to the feed. The wire form is the complete desired tree — the core + /// deletes every folder the list omits — and a tree assembled here would be assembled from a + /// snapshot that may already be stale, turning a create into a silent delete of whatever + /// arrived meanwhile. The feed owns the list; see `CoreCmd::AddFolder`. + /// + /// Args: + /// core: Core to create the folder on. + /// parent: Folder path to create it under; empty for the core root. + /// name: Leaf name the operator typed, already trimmed. + /// cx: View context used to send the command. + /// + /// Returns: + /// Nothing; a core that cannot hold the folder keeps the local mark and nothing else. + pub(super) fn create_folder( + &mut self, + core: CoreId, + parent: &str, + name: &str, + cx: &mut Context, + ) { let mut parts = ops::split_path(parent); parts.push(name.to_string()); let key = ops::join_path(&parts); + + if let Err(error) = self + .backend + .read(cx) + .session + .add_core_folder(core, key.clone()) + { + log::warn!("create folder failed: {error}"); + } + self.ui_folders.insert((core, key)); // Expand the core and parent chain, excluding the new folder itself, so it is immediately // visible. @@ -370,33 +424,53 @@ impl StrategiesView { } } - /// Returns empty UI-only folder paths for a core so they can be merged into the tree. + /// Returns empty UI-only folder paths for a core, in the order the tree appends them. + /// + /// SORTED here rather than by the caller: these come out of a `HashSet`, whose iteration order + /// is not stable, and the tree appends them in the order it receives them — so unsorted, two + /// frames rendering identical data would put the same folders in different places. Ordered + /// case-insensitively first and by the spelling itself second, because on the folded key alone + /// two siblings differing only in case compare equal and the tie goes back to the set. pub(super) fn ui_folder_paths(&self, core: CoreId) -> Vec> { - self.ui_folders + let mut paths: Vec> = self + .ui_folders .iter() .filter(|(c, _)| *c == core) .map(|(_, p)| ops::split_path(p)) - .collect() + .collect(); + paths.sort_by_cached_key(|parts| { + let joined = parts.join("/"); + (joined.to_lowercase(), joined) + }); + paths } - /// Drop UI-only ownership once live core data represents a folder through a strategy row. + /// Drop UI-only ownership once the core itself represents the folder. /// - /// Empty folders exist only in this view until first use. Retaining their local marker after - /// a strategy arrives would make the folder reappear as a ghost if another surface later - /// deletes that strategy and asks the core to remove the now-empty folder. + /// Two ways that happens: a strategy arrives in it, or — on a core that synchronizes folders — + /// the core reports the folder in its own tree. Retaining the local marker past either would + /// make the folder reappear as a ghost after another surface deleted it, which is precisely + /// what the mark cannot be allowed to do once the core owns the answer. /// /// Args: - /// store: Current per-core live strategy snapshots. + /// store: Current per-core live strategy and folder snapshots. pub(in crate::strategies) fn reconcile_ui_folders(&mut self, store: &CoreStore) { self.ui_folders.retain(|(core, path)| { - keep_ui_folder( - path, - store.core(*core).map(|data| data.strategies.as_slice()), - ) + let Some(data) = store.core(*core) else { + // The core is gone from the store entirely; nothing can contradict the mark. + return true; + }; + let confirmed = data.folders.supported + && data + .folders + .paths + .iter() + .any(|seen| seen.to_lowercase() == path.to_lowercase()); + !confirmed && keep_ui_folder(path, Some(data.strategies.as_slice())) }); } - // ── Keyboard: Ctrl+C, Ctrl+V, and Delete ────────────────────────────────── + // ── Keyboard: Ctrl+C, Ctrl+V, Ctrl+Shift+Up/Down, and Delete ────────────── /// Copy the last clicked visible folder/core root, otherwise the visible strategy selection. /// @@ -444,6 +518,18 @@ impl StrategiesView { self.default_target(b.session.store(), &cores) }; self.paste_into(core, target, cx); + } else if reorder_chord(m, key) { + // A HELD arrow is refused. OS auto-repeat fires this handler tens of times a second, + // and each pass queues a whole-list reorder that the feed turns into a full snapshot to + // the core; the repo's own keyboard path (`hotkeys::pre_dispatch`) refuses repeats for + // the same reason. One press, one move. + if !ev.is_held { + let step = match key == "up" { + true => ops::MoveStep::Up, + false => ops::MoveStep::Down, + }; + self.move_selection(step, cx); + } } else if key == "delete" { self.request_delete_selection(window, cx); } @@ -466,6 +552,7 @@ impl StrategiesView { store: &CoreStore, show_labels: bool, has_visible_cores: bool, + moves: (bool, bool), cx: &Context, ) -> AnyElement { let (has_sel, all_off) = self.selection_summary(store); @@ -478,6 +565,9 @@ impl StrategiesView { let delete_label = t!("strat.action_delete").to_string(); let icon_width = design::glyph_btn_w(cx); + let move_up = self.move_button(ops::MoveStep::Up, moves.0, icon_width, cx); + let move_down = self.move_button(ops::MoveStep::Down, moves.1, icon_width, cx); + let mut copy = MoonButton::new("sel-copy") .outline() .size(MoonButtonSize::Action) @@ -525,12 +615,60 @@ impl StrategiesView { .flex_none() .items_center() .gap(design::ui_px(cx, group_gap)) + .child(move_up.render()) + .child(move_down.render()) .child(copy.render()) .child(paste.render()) .child(delete.render()) .into_any_element() } + /// Build one of the footer's two move buttons. + /// + /// Icon-only in BOTH densities, unlike its neighbours: the group already carries three labelled + /// buttons at the labelled density, and two more would push it past the footer's width at the + /// pane sizes this window is normally used at. An arrow needs the word less than "Copy" does, + /// and the tooltip names both the action and the chord. + /// + /// Args: + /// step: Direction this button moves the selection. + /// enabled: Whether the cached plan says it would rearrange anything. + /// icon_width: Shared icon-density button width. + /// cx: View context used to build the click listener. + /// + /// Returns: + /// The configured button, ready to render. + fn move_button( + &self, + step: ops::MoveStep, + enabled: bool, + icon_width: f32, + cx: &Context, + ) -> MoonButton { + let (id, icon, label, chord) = match step { + ops::MoveStep::Up => ( + "sel-move-up", + "icons/arrow-up.svg", + t!("strat.action_move_up"), + t!("strat.move_up_chord"), + ), + ops::MoveStep::Down => ( + "sel-move-down", + "icons/arrow-down.svg", + t!("strat.action_move_down"), + t!("strat.move_down_chord"), + ), + }; + MoonButton::new(id) + .outline() + .size(MoonButtonSize::Action) + .width(icon_width) + .leading_icon(MoonButtonIconSlot::new(icon)) + .tooltip(format!("{label} · {chord}")) + .disabled(!enabled) + .on_click(cx.listener(move |this, _, _, cx| this.move_selection(step, cx))) + } + /// Builds the Create dropdown for a strategy or folder in the tree header. pub(super) fn create_dropdown( &self, diff --git a/locales/strategies.yml b/locales/strategies.yml index e18cdd92..6558547c 100644 --- a/locales/strategies.yml +++ b/locales/strategies.yml @@ -369,6 +369,14 @@ strat.menu_delete_strategy: ru: "Удалить…" en: "Delete…" es: "Eliminar…" +strat.menu_move_up: + ru: "Переместить выше" + en: "Move up" + es: "Mover arriba" +strat.menu_move_down: + ru: "Переместить ниже" + en: "Move down" + es: "Mover abajo" strat.action_copy: ru: "копировать" en: "copy" @@ -382,6 +390,33 @@ strat.action_delete: en: "delete" es: "eliminar" +strat.folder_empty_tip: + ru: "Пустая папка" + en: "Empty folder" + es: "Carpeta vacía" +strat.folder_empty_local_tip: + ru: "Пустая папка, известная только этому окну: ядро её не хранит и после перезапуска её не будет" + en: "Empty folder, known only to this window: the core does not keep it, and a restart loses it" + es: "Carpeta vacía, conocida solo por esta ventana: el núcleo no la guarda y se pierde al reiniciar" +strat.action_move_up: + ru: "выше в папке" + en: "move up in folder" + es: "subir en la carpeta" +strat.action_move_down: + ru: "ниже в папке" + en: "move down in folder" + es: "bajar en la carpeta" +# The chord itself, in one place: the footer tooltips and the context-menu hints both read it, and +# a keystroke spelled two ways is a keystroke the reader stops trusting. +strat.move_up_chord: + ru: "Ctrl+Shift+↑" + en: "Ctrl+Shift+↑" + es: "Ctrl+Shift+↑" +strat.move_down_chord: + ru: "Ctrl+Shift+↓" + en: "Ctrl+Shift+↓" + es: "Ctrl+Shift+↓" + # --- Versions pane (strategy version history and statistics) --- strat.versions: ru: "Версии" From d5dc538e2c8db7ad3cd29efd975f8696e8125f85 Mon Sep 17 00:00:00 2001 From: guyverino Date: Sun, 6 Sep 2026 21:39:12 +0200 Subject: [PATCH 2/3] fix(diagnostics): record what crashed before capturing its backtrace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both handlers — the Rust panic hook and the native SEH filter — captured a backtrace first and wrote the record afterwards. Symbolizing goes through dbghelp, which is not thread-safe and cannot run on an exhausted stack, so when the capture itself faulted the process died inside the handler with nothing written at all: an empty panic.log, an empty app log, and a Windows crash record naming dbghelp.dll as the faulting module rather than anything of ours. That is not a hypothetical. A startup stack overflow presented exactly this way and was undiagnosable until the order was reversed; with the record written first, the same run reported `NATIVE CRASH: code=0xC00000FD` and named its address, which is the whole of what was needed. The backtrace is still attempted, appended as a second write — panic_log opens for append, so a crash between the two costs the backtrace and nothing else. --- crates/moon-ui-gpui/src/diagnostics/crash.rs | 16 ++++++++++------ crates/moon-ui-gpui/src/startup.rs | 17 +++++++++++------ 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/crates/moon-ui-gpui/src/diagnostics/crash.rs b/crates/moon-ui-gpui/src/diagnostics/crash.rs index 018787fd..7a302877 100644 --- a/crates/moon-ui-gpui/src/diagnostics/crash.rs +++ b/crates/moon-ui-gpui/src/diagnostics/crash.rs @@ -81,16 +81,20 @@ unsafe extern "system" fn native_exception_filter( body.push_str(""); } + // WHAT faulted, written before the backtrace is attempted. Symbolizing goes through + // `dbghelp`, which is not thread-safe and can fault while handling a fault — and a second + // access violation inside this handler takes the process down with nothing recorded at all. + // Written first, the crash's own code and address survive that. + // + // Writes the file directly, without the global logger: the faulting thread may already hold its + // lock. Same sink as the Rust panic hook, which is why it lives in `applog`. + moon_core::applog::panic_log(&format!("NATIVE CRASH: {body}")); + // The filter runs on the thread that faulted, and `force_capture` walks its current stack while // the handler is executing. It does not unwind from the saved `ContextRecord`; available PDBs // symbolize the captured frames in the same way as the panic hook. let bt = std::backtrace::Backtrace::force_capture(); - - // Writes the file directly, without the global logger: the faulting thread may already hold its - // lock. Same sink as the Rust panic hook, which is why it lives in `applog`. - moon_core::applog::panic_log(&format!( - "NATIVE CRASH: {body}\n--- backtrace ---\n{bt}\n--- end ---" - )); + moon_core::applog::panic_log(&format!("--- backtrace ---\n{bt}\n--- end ---")); EXCEPTION_CONTINUE_SEARCH } diff --git a/crates/moon-ui-gpui/src/startup.rs b/crates/moon-ui-gpui/src/startup.rs index 6f50ab2a..9190e40d 100644 --- a/crates/moon-ui-gpui/src/startup.rs +++ b/crates/moon-ui-gpui/src/startup.rs @@ -402,15 +402,20 @@ pub(crate) fn run(startup_update: Option) -> anyho .copied() .or_else(|| info.payload().downcast_ref::().map(|s| s.as_str())) .unwrap_or(""); - // Force a backtrace without RUST_BACKTRACE: clamp panics report a location inside core, - // while we need the CALLING frame in our code. - let bt = std::backtrace::Backtrace::force_capture(); + // WHAT panicked, written before anything else is attempted. Capturing a backtrace + // means symbolizing through `dbghelp`, which is not thread-safe and can fault on its + // own — and when it does, the process dies inside the handler with an access violation + // in `dbghelp.dll` and NOTHING is recorded: no location, no message, an empty log. This + // ordering means the worst such a failure can cost is the backtrace. + // // Both sinks redact on their own: `panic_log` owns the file, `TeeLogger` the log. // A panic message can quote foreign text carrying an endpoint. - moon_core::applog::panic_log(&format!( - "PANIC at {loc}: {payload}\n--- backtrace ---\n{bt}\n--- end ---" - )); + moon_core::applog::panic_log(&format!("PANIC at {loc}: {payload}")); log::error!("PANIC at {loc}: {payload}"); + // Forced rather than left to RUST_BACKTRACE: clamp panics report a location inside + // core, while we need the CALLING frame in our code. + let bt = std::backtrace::Backtrace::force_capture(); + moon_core::applog::panic_log(&format!("--- backtrace ---\n{bt}\n--- end ---")); default_hook(info); })); } From 939e7ceba26f8e4ed833ed2ef3b9c9671c5654f9 Mon Sep 17 00:00:00 2001 From: guyverino Date: Sun, 6 Sep 2026 21:39:23 +0200 Subject: [PATCH 3/3] fix(startup): build the translation backend before the stack runs out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rust-i18n builds its backend once, lazily, on the first t!() — and its generated initializer materialises all ~2600 keys in ONE stack frame. Whether that frame fits depends entirely on where the first lookup happens, which nothing was choosing deliberately: it landed deep inside chart-tab construction, the __chkstk probe entering the frame faulted, and the application died at startup with 0xC00000FD before opening a window. The threshold moved because the dictionary grew — this branch adds thirteen keys and #441 added the field-label table — but the trigger is the call site, not the count, so a dictionary this size will cross it again from anywhere. Touched at the base of the stack instead, the frame is built where there is room and every later t!() is a map lookup. Verified live: the same configuration that crashed on every launch now starts. --- crates/moon-ui-gpui/src/main.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/moon-ui-gpui/src/main.rs b/crates/moon-ui-gpui/src/main.rs index 9e396784..18571044 100644 --- a/crates/moon-ui-gpui/src/main.rs +++ b/crates/moon-ui-gpui/src/main.rs @@ -640,6 +640,18 @@ struct CoreFilterRevision; /// Returns: /// Success after the selected process role exits. fn main() -> anyhow::Result<()> { + // FIRST, and at the base of the stack on purpose. `rust_i18n` builds its translation backend + // once, lazily, on the first `t!()` — and its generated initializer materialises all ~2600 keys + // in ONE stack frame, large enough that the `__chkstk` probe entering it faults outright when + // the first lookup happens deep inside window construction. That is not hypothetical: it is a + // 0xC00000FD stack overflow at startup, and because the stack was already exhausted the crash + // handler could not even capture a backtrace — the process died inside `dbghelp` instead, + // leaving an empty log and a crash that named the wrong module. + // + // Touched here, the frame is built where there is room, and every later `t!()` is a map lookup + // costing nothing. The key is arbitrary; only the initialization it forces matters. + let _ = rust_i18n::t!("common.loading"); + // Before the updater, before the configuration, before a window: the UI-atlas tools that work // on a file the crawl already wrote need none of it, and running them through a normal launch // would put a six-minute walk between a rule and its result.