diff --git a/Cargo.lock b/Cargo.lock index 12c21f5d..6bb63bc6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3771,7 +3771,7 @@ dependencies = [ [[package]] name = "moon-collections" version = "0.0.0" -source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#a262ff8f2fa832a245859add9d1db5622c8e2afe" +source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#2277810dc9757c01c29065f3d665f431b94e520f" dependencies = [ "indexmap", "moon-gpui-util", @@ -3818,7 +3818,7 @@ dependencies = [ [[package]] name = "moon-derive-refineable" version = "0.0.0" -source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#a262ff8f2fa832a245859add9d1db5622c8e2afe" +source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#2277810dc9757c01c29065f3d665f431b94e520f" dependencies = [ "proc-macro2", "quote", @@ -3828,7 +3828,7 @@ dependencies = [ [[package]] name = "moon-gpui" version = "0.0.0" -source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#a262ff8f2fa832a245859add9d1db5622c8e2afe" +source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#2277810dc9757c01c29065f3d665f431b94e520f" dependencies = [ "accesskit", "anyhow", @@ -3912,7 +3912,7 @@ dependencies = [ [[package]] name = "moon-gpui-linux" version = "0.0.0" -source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#a262ff8f2fa832a245859add9d1db5622c8e2afe" +source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#2277810dc9757c01c29065f3d665f431b94e520f" dependencies = [ "accesskit", "accesskit_unix", @@ -3963,7 +3963,7 @@ dependencies = [ [[package]] name = "moon-gpui-macos" version = "0.0.0" -source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#a262ff8f2fa832a245859add9d1db5622c8e2afe" +source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#2277810dc9757c01c29065f3d665f431b94e520f" dependencies = [ "accesskit", "accesskit_macos", @@ -4010,7 +4010,7 @@ dependencies = [ [[package]] name = "moon-gpui-macros" version = "0.0.0" -source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#a262ff8f2fa832a245859add9d1db5622c8e2afe" +source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#2277810dc9757c01c29065f3d665f431b94e520f" dependencies = [ "heck 0.5.0", "proc-macro2", @@ -4021,7 +4021,7 @@ dependencies = [ [[package]] name = "moon-gpui-platform" version = "0.0.0" -source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#a262ff8f2fa832a245859add9d1db5622c8e2afe" +source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#2277810dc9757c01c29065f3d665f431b94e520f" dependencies = [ "console_error_panic_hook", "moon-gpui", @@ -4034,7 +4034,7 @@ dependencies = [ [[package]] name = "moon-gpui-shared-string" version = "0.0.0" -source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#a262ff8f2fa832a245859add9d1db5622c8e2afe" +source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#2277810dc9757c01c29065f3d665f431b94e520f" dependencies = [ "schemars", "serde", @@ -4044,7 +4044,7 @@ dependencies = [ [[package]] name = "moon-gpui-util" version = "0.0.0" -source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#a262ff8f2fa832a245859add9d1db5622c8e2afe" +source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#2277810dc9757c01c29065f3d665f431b94e520f" dependencies = [ "anyhow", "log", @@ -4054,7 +4054,7 @@ dependencies = [ [[package]] name = "moon-gpui-web" version = "0.0.0" -source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#a262ff8f2fa832a245859add9d1db5622c8e2afe" +source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#2277810dc9757c01c29065f3d665f431b94e520f" dependencies = [ "anyhow", "console_error_panic_hook", @@ -4078,7 +4078,7 @@ dependencies = [ [[package]] name = "moon-gpui-wgpu" version = "0.0.0" -source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#a262ff8f2fa832a245859add9d1db5622c8e2afe" +source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#2277810dc9757c01c29065f3d665f431b94e520f" dependencies = [ "anyhow", "bytemuck", @@ -4107,7 +4107,7 @@ dependencies = [ [[package]] name = "moon-gpui-windows" version = "0.0.0" -source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#a262ff8f2fa832a245859add9d1db5622c8e2afe" +source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#2277810dc9757c01c29065f3d665f431b94e520f" dependencies = [ "accesskit", "accesskit_windows", @@ -4135,7 +4135,7 @@ dependencies = [ [[package]] name = "moon-http-client" version = "0.0.0" -source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#a262ff8f2fa832a245859add9d1db5622c8e2afe" +source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#2277810dc9757c01c29065f3d665f431b94e520f" dependencies = [ "anyhow", "async-compression", @@ -4155,7 +4155,7 @@ dependencies = [ [[package]] name = "moon-media" version = "0.0.0" -source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#a262ff8f2fa832a245859add9d1db5622c8e2afe" +source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#2277810dc9757c01c29065f3d665f431b94e520f" dependencies = [ "anyhow", "bindgen", @@ -4170,7 +4170,7 @@ dependencies = [ [[package]] name = "moon-perf" version = "0.0.0" -source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#a262ff8f2fa832a245859add9d1db5622c8e2afe" +source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#2277810dc9757c01c29065f3d665f431b94e520f" dependencies = [ "moon-collections", "serde", @@ -4180,7 +4180,7 @@ dependencies = [ [[package]] name = "moon-refineable" version = "0.0.0" -source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#a262ff8f2fa832a245859add9d1db5622c8e2afe" +source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#2277810dc9757c01c29065f3d665f431b94e520f" dependencies = [ "moon-derive-refineable", ] @@ -4188,7 +4188,7 @@ dependencies = [ [[package]] name = "moon-scheduler" version = "0.0.0" -source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#a262ff8f2fa832a245859add9d1db5622c8e2afe" +source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#2277810dc9757c01c29065f3d665f431b94e520f" dependencies = [ "async-task", "backtrace", @@ -4203,7 +4203,7 @@ dependencies = [ [[package]] name = "moon-sum-tree" version = "0.0.0" -source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#a262ff8f2fa832a245859add9d1db5622c8e2afe" +source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#2277810dc9757c01c29065f3d665f431b94e520f" dependencies = [ "heapless", "log", @@ -4214,7 +4214,7 @@ dependencies = [ [[package]] name = "moon-ui" version = "0.1.0" -source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#a262ff8f2fa832a245859add9d1db5622c8e2afe" +source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#2277810dc9757c01c29065f3d665f431b94e520f" dependencies = [ "moon-ui-components", "moon-ui-components-assets", @@ -4223,7 +4223,7 @@ dependencies = [ [[package]] name = "moon-ui-components" version = "0.1.0" -source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#a262ff8f2fa832a245859add9d1db5622c8e2afe" +source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#2277810dc9757c01c29065f3d665f431b94e520f" dependencies = [ "aho-corasick", "anyhow", @@ -4273,7 +4273,7 @@ dependencies = [ [[package]] name = "moon-ui-components-assets" version = "0.1.0" -source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#a262ff8f2fa832a245859add9d1db5622c8e2afe" +source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#2277810dc9757c01c29065f3d665f431b94e520f" dependencies = [ "anyhow", "log", @@ -4287,7 +4287,7 @@ dependencies = [ [[package]] name = "moon-ui-components-macros" version = "0.1.0" -source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#a262ff8f2fa832a245859add9d1db5622c8e2afe" +source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#2277810dc9757c01c29065f3d665f431b94e520f" dependencies = [ "proc-macro2", "quote", @@ -4338,7 +4338,7 @@ dependencies = [ [[package]] name = "moon-util-macros" version = "0.0.0" -source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#a262ff8f2fa832a245859add9d1db5622c8e2afe" +source = "git+https://github.com/Moonbot-Tech/MoonUI?branch=master#2277810dc9757c01c29065f3d665f431b94e520f" dependencies = [ "moon-perf", "quote", diff --git a/crates/moon-core/src/config/layout.rs b/crates/moon-core/src/config/layout.rs index 728a789a..0b0b4bae 100644 --- a/crates/moon-core/src/config/layout.rs +++ b/crates/moon-core/src/config/layout.rs @@ -772,7 +772,7 @@ pub struct WindowLayout { /// hand-edited, and one wrongly typed value must not cost the user the rest of the file. #[serde(default, deserialize_with = "de_lenient")] pub analytics_strategy_mask: Option, - /// Bitmask of the visible columns in the Tuning strategy list (the ▦ selector). + /// Bitmask of the visible columns in the Tuning strategy list (the column selector). /// None = default (all columns). /// /// Version 2 of the key. The bit layout is positional (metric columns sit at diff --git a/crates/moon-core/src/db/mod.rs b/crates/moon-core/src/db/mod.rs index 945ac596..965a033b 100644 --- a/crates/moon-core/src/db/mod.rs +++ b/crates/moon-core/src/db/mod.rs @@ -38,8 +38,9 @@ pub mod valuation; pub use dates::{fmt_unix, fmt_unix_date, fmt_unix_secs, parse_ymd}; pub use quote::{ - OpenPositions, ProfitScope, ProfitUnit, QuoteBreakdown, QuoteCurrency, QuoteScope, QuoteTotal, - QuoteVolume, TradedVolume, UsdtTotal, ValuationCoverage, + AverageOrderReturn, EntrySpend, OpenPositions, ProfitScope, ProfitUnit, QuoteBreakdown, + QuoteCurrency, QuoteScope, QuoteSpend, QuoteTotal, QuoteVolume, TradedVolume, UsdtTotal, + ValuationCoverage, }; pub use read_cancel::{ReadCancellation, with_read_cancellation}; pub use read_fail::{FailKind, ReadFail, ReadResult}; diff --git a/crates/moon-core/src/db/quote.rs b/crates/moon-core/src/db/quote.rs index a50f7eeb..a598d345 100644 --- a/crates/moon-core/src/db/quote.rs +++ b/crates/moon-core/src/db/quote.rs @@ -597,6 +597,153 @@ pub struct UsdtTotal { pub spent: Option, } +/// One known-currency spend/profit subtotal over the CLOSED, non-Funding, positively-spent rows +/// that [`QuoteBreakdown::average_order_return`] counts. +/// +/// A separate carrier from [`QuoteTotal`] rather than a widened field on it: `QuoteTotal` is +/// shared by [`QuoteBreakdown::from_groups`] and [`OpenPositions::from_groups`] through +/// `group_quotes`, so adding a spend leg there would drag a permanently-empty field through the +/// open pass and Analytics, which never accounts a spend this way. +#[derive(Clone, Copy, Debug, PartialEq)] +pub struct QuoteSpend { + /// Currency shared by every contributing row. + pub currency: QuoteCurrency, + /// Sum of settled `spentbtc` over the counted rows of this currency. + pub spent: f64, + /// Sum of settled `profitbtc` over the SAME counted rows. + pub profit: f64, + /// Counted rows contributing to both sums. + pub orders: i64, +} + +/// Per-quote spend/profit subtotals feeding [`QuoteBreakdown::average_order_return`], carrying its +/// OWN unified USDT leg rather than reading [`UsdtTotal::spent`]. +/// +/// [`UsdtTotal::spent`] rests on [`super::valuation::SourcePredicates::spent_value`], which tests +/// only that `spentbtc` is numeric — no positive-spend guard, no Funding exclusion. Averaging over +/// it would count a WIDER row set than the native arm while still reporting a complete `counted`, +/// a dishonest denominator. [`TradedVolume`] already carries its own `usdt` leg for the identical +/// reason; this follows that precedent instead of borrowing the valuation cache's. +#[derive(Clone, Debug, Default, PartialEq)] +pub struct EntrySpend { + /// Per known quote, over COUNTED rows only, sorted by persisted currency ordinal. + pub totals: Vec, + /// Every counted row in the scope, including rows whose quote identity is unknown. + pub counted_orders: i64, + /// Counted rows carrying an active-mode USDT rate. + pub valued_orders: i64, + /// Sigma spent converted at the active-mode rate, over valued counted rows. + pub usdt_spent: f64, + /// Sigma profit converted at the same rate, over the same rows. + pub usdt_profit: f64, +} + +impl EntrySpend { + /// Build per-quote spend/profit subtotals, and the unified USDT leg, from physical-source + /// quote groups. + /// + /// Args: + /// groups: `(ordinal, counted, Sigma spent, Sigma profit, valued, Sigma USDT spent, Sigma + /// USDT profit)` aggregates, one row per physical source and quote group. + /// + /// Returns: + /// Ordinal-sorted known buckets plus the scope-wide counted/valued/USDT tallies. A KNOWN + /// ordinal folds into a [`QuoteSpend`] bucket; an UNKNOWN one still contributes to + /// [`Self::counted_orders`] but to no bucket. Such a row can carry no per-quote rate, so it + /// can never be valued either, and [`Self::unified`] fails closed on it by construction. + pub(crate) fn from_groups( + groups: impl IntoIterator, i64, f64, f64, i64, f64, f64)>, + ) -> Self { + let mut known: BTreeMap = BTreeMap::new(); + let mut out = Self::default(); + for (ordinal, counted, spent, profit, valued, usdt_spent, usdt_profit) in groups { + if counted == 0 { + continue; + } + out.counted_orders += counted; + out.valued_orders += valued; + out.usdt_spent += usdt_spent; + out.usdt_profit += usdt_profit; + if let Some(currency) = ordinal.and_then(QuoteCurrency::from_report_ordinal) { + let bucket = known.entry(currency).or_default(); + bucket.0 += counted; + bucket.1 += spent; + bucket.2 += profit; + } + } + out.totals = known + .into_iter() + .map(|(currency, (orders, spent, profit))| QuoteSpend { + currency, + spent, + profit, + orders, + }) + .collect(); + out + } + + /// The unified USDT pair, available ONLY over a completely valued counted scope. + /// + /// Mirrors [`TradedVolume::from_groups`]'s completeness rule: a partial unified figure is + /// never published, because a mixed scope has no second bucket to fall back on and a partial + /// sum would read exactly like a complete one. + /// + /// Returns: + /// `(Sigma spent, Sigma profit)` in USDT, or `None` for an empty or incompletely-valued + /// scope. + pub fn unified(&self) -> Option<(f64, f64)> { + (self.counted_orders > 0 && self.valued_orders == self.counted_orders) + .then_some((self.usdt_spent, self.usdt_profit)) + } +} + +/// Realized profit stated as a percentage of the average order over a single-core Report scope. +/// +/// Denominated the same way the head footer figure is: a native currency where [`QuoteBreakdown`] +/// carries exactly one, or a unified USDT total where the scope is mixed, the head figure is +/// itself complete ([`QuoteBreakdown::unified_usdt`] is `Some`), and [`EntrySpend::unified`] is +/// complete over the COUNTED rows. The unified arm can still be PARTIAL — [`Self::excluded`] can +/// be positive even there, because Funding, non-positive-spend, and unknown-quote rows shrink the +/// counted scope below [`QuoteBreakdown::orders`] independently of whether every counted row was +/// valued. +#[derive(Clone, Copy, Debug, PartialEq)] +pub struct AverageOrderReturn { + /// `100 * Sigma profit / avg_order`, over the counted rows. + pub pct: f64, + /// `Sigma spent / counted`, denominated in [`Self::currency`]. + pub avg_order: f64, + /// Currency both sums are denominated in. + pub currency: QuoteCurrency, + /// Rows contributing to both sums. + pub counted: i64, + /// Rows this scope's total row count could not account for, derived as `orders - counted`, + /// never carried as a second field that could disagree with the first. + pub excluded: i64, + /// Whether [`Self::pct`] and [`Self::avg_order`] are the unified USDT conversion rather than a + /// native currency. + pub unified: bool, +} + +/// Whether an average order size is a figure worth dividing a profit by. +/// +/// A positive test alone is NOT enough, and the gap is not theoretical: replica ingestion stores +/// an unvalidated float straight into SQLite as `REAL`, and a `SUM` over a large scope can +/// overflow, so `spent` can arrive as positive infinity. Infinity passes any `> 0.0` check, and +/// the percentage taken from it is `100 * profit / inf` — a FINITE zero. The footer would then +/// state a confident `+0.0%` beside an average rendered `inf`, and the trailing `is_finite` check +/// on the ratio cannot see it, because by then the ratio looks perfectly ordinary. Rejecting the +/// AVERAGE is the only place that catches it. +/// +/// Args: +/// avg_order: Candidate average order size. +/// +/// Returns: +/// Whether the value is finite and strictly positive. NaN fails both halves. +fn usable_average(avg_order: f64) -> bool { + avg_order.is_finite() && avg_order > 0.0 +} + /// One known-currency traded-volume bucket over an exact Report scope. #[derive(Clone, Copy, Debug, PartialEq)] pub struct QuoteVolume { @@ -817,6 +964,9 @@ pub struct QuoteBreakdown { /// Two-sided volume computed over this same filter and snapshot; [`TradedVolume`] states its /// own completeness rather than withholding a native amount. pub traded_volume: TradedVolume, + /// Per-quote spend/profit subtotals over the counted rows of this same filter and snapshot, + /// feeding [`Self::average_order_return`]. + pub entry_spend: EntrySpend, } impl QuoteBreakdown { @@ -861,6 +1011,18 @@ impl QuoteBreakdown { self } + /// Attach entry-spend subtotals computed over the same filter and read snapshot. + /// + /// Args: + /// spend: Per-quote counted spend/profit subtotals feeding [`Self::average_order_return`]. + /// + /// Returns: + /// This profit breakdown carrying the supplied entry spend. + pub(crate) fn with_entry_spend(mut self, spend: EntrySpend) -> Self { + self.entry_spend = spend; + self + } + /// Return a complete unified USDT total only when no row has unknown quote identity. /// /// Returns: @@ -886,6 +1048,115 @@ impl QuoteBreakdown { QuoteScope::Mixed } } + + /// State realized profit as a percentage of the average order over the counted rows, in + /// exactly the currency [`Self::scope`] would promote as the head figure. + /// + /// Driven by [`Self::scope`], [`Self::unified_usdt`] and [`EntrySpend::unified`] so the ratio + /// can never be denominated differently from the figure it qualifies. `u` below is + /// `self.unified_usdt()`, and `e` is `self.entry_spend`: + /// + /// ```text + /// scope() condition pct arm + /// Empty — absent + /// Single(c) e.totals has bucket c with orders > 0 native + /// Single(c) otherwise absent + /// Mixed u.is_some() AND e.unified() == Some((s, p)) unified, over USDT + /// Mixed otherwise absent + /// Unknown exactly one bucket in SELF.totals with orders>0 native (excluded incl. unknowns) + /// Unknown otherwise absent + /// ``` + /// + /// The `Unknown` row keys on `self.totals` — the PROFIT buckets — and not on `e.totals`, and + /// the difference is load-bearing rather than incidental: `self.totals` is what + /// `footer_facts` promotes into the never-clipped head, so keying on it is what guarantees + /// the percentage is denominated in the currency the row's own money is stated in. Reading + /// `e.totals` here instead would let the two disagree the moment a bucket has profit rows but + /// no COUNTED ones, or the reverse. + /// + /// The unified arm never reads [`UsdtTotal::spent`] — see [`EntrySpend`]'s own doc for why — + /// and it CAN still be partial: [`AverageOrderReturn::excluded`] can be positive there too, + /// because Funding, non-positive-spend, and unknown-quote rows shrink the counted scope below + /// [`Self::orders`] independently of whether every counted row was valued. + /// + /// The house average-order definition (`analytics::groups`, `analytics::profit_monitor`) is + /// over rows with a POSITIVE numeric settled spend; the SQL leg that fills + /// [`Self::entry_spend`] already applies that filter, together with the Funding exclusion the + /// traded-volume leg uses. `excluded` is always `Self::orders - counted`, so it aggregates + /// unknown-quote rows, non-numeric/non-positive spend, non-numeric profit, and Funding rows + /// without a second field that could disagree with the first. + /// + /// Worked example: profit 3496.52 over 5 rows with spend + /// 28742+34160+29884+28717+28717 = 150220 -> `avg_order == 30044.0`, + /// `pct == 100.0 * 3496.52 / 30044.0` (rounds to `+11.6%`), `counted == 5`, `excluded == 0`. + /// + /// Returns: + /// `None` for an empty, mixed-incomplete, mixed-unknown, or multi/zero-known unknown + /// scope, for zero counted rows, or for a non-finite ratio. + pub fn average_order_return(&self) -> Option { + match self.scope() { + QuoteScope::Empty => None, + QuoteScope::Single(currency) => self.native_average_order_return(currency), + QuoteScope::Mixed => { + self.unified_usdt()?; + let (spent, profit) = self.entry_spend.unified()?; + // `unified()` returns `Some` only over a positive counted scope, so the division + // below cannot divide by zero and needs no guard of its own here. + let counted = self.entry_spend.counted_orders; + let avg_order = spent / counted as f64; + if !usable_average(avg_order) { + return None; + } + let pct = 100.0 * profit / avg_order; + pct.is_finite().then_some(AverageOrderReturn { + pct, + avg_order, + currency: QuoteCurrency::usdt(), + counted, + excluded: self.orders - counted, + unified: true, + }) + } + QuoteScope::Unknown => { + if self.totals.len() != 1 { + return None; + } + self.native_average_order_return(self.totals[0].currency) + } + } + } + + /// Build the native-currency arm of [`Self::average_order_return`] for one exact currency. + /// + /// Args: + /// currency: Currency [`Self::scope`] promoted as the head figure. + /// + /// Returns: + /// `None` when no counted bucket exists for `currency`, when its spend sums to zero or + /// less, or when the resulting ratio is non-finite. + fn native_average_order_return(&self, currency: QuoteCurrency) -> Option { + let bucket = self + .entry_spend + .totals + .iter() + .find(|bucket| bucket.currency == currency)?; + if bucket.orders <= 0 { + return None; + } + let avg_order = bucket.spent / bucket.orders as f64; + if !usable_average(avg_order) { + return None; + } + let pct = 100.0 * bucket.profit / avg_order; + pct.is_finite().then_some(AverageOrderReturn { + pct, + avg_order, + currency, + counted: bucket.orders, + excluded: self.orders - bucket.orders, + unified: false, + }) + } } /// Comparability of raw quote-denominated money in one report scope. diff --git a/crates/moon-core/src/db/quote/tests.rs b/crates/moon-core/src/db/quote/tests.rs index 972ab0d5..de4882ac 100644 --- a/crates/moon-core/src/db/quote/tests.rs +++ b/crates/moon-core/src/db/quote/tests.rs @@ -77,6 +77,64 @@ fn breakdown_merges_only_identical_known_quotes() { assert_eq!(totals.traded_volume, TradedVolume::default()); } +/// `quote.rs::QuoteBreakdown::average_order_return` must gate an unknown scope on the PROFIT +/// buckets, not on the counted-spend buckets. Replacing `self.totals` with +/// `self.entry_spend.totals` would publish a USDT percentage while the Report head also contains +/// USDC profit, silently putting incomparable currencies on one footer line. +#[test] +fn unknown_scope_refuses_a_ratio_when_profit_and_counted_buckets_differ() { + let totals = + QuoteBreakdown::from_groups([(Some(1), 100.0, 1), (Some(8), 50.0, 1), (None, 0.0, 1)]) + .with_entry_spend(EntrySpend::from_groups([( + Some(1), + 1, + 1_000.0, + 100.0, + 0, + 0.0, + 0.0, + )])); + + assert_eq!(totals.scope(), QuoteScope::Unknown); + assert_eq!(totals.entry_spend.totals.len(), 1); + assert!( + totals.average_order_return().is_none(), + "two head-profit currencies cannot truthfully wear the one counted USDT denominator" + ); +} + +/// `quote.rs::usable_average` must retain its `is_finite()` leg. Reducing it to `avg_order > +/// 0.0` would turn an infinite replica spend into a confident finite +0.0% footer percentage. +#[test] +fn average_order_return_rejects_an_infinite_average() { + let totals = QuoteBreakdown::from_groups([(Some(1), 100.0, 1)]).with_entry_spend( + EntrySpend::from_groups([(Some(1), 1, f64::INFINITY, 100.0, 0, 0.0, 0.0)]), + ); + + assert!( + totals.average_order_return().is_none(), + "an infinite average is not a meaningful denominator even when its resulting percentage is finite" + ); +} + +/// `quote.rs::EntrySpend::unified` must require every counted row to have a valuation. Dropping +/// `valued_orders == counted_orders` would publish a partial USDT average as a complete one. +#[test] +fn unified_entry_spend_refuses_a_partially_valued_scope() { + let spend = EntrySpend::from_groups([ + (Some(1), 1, 100.0, 10.0, 1, 100.0, 10.0), + (Some(8), 1, 200.0, 20.0, 0, 0.0, 0.0), + ]); + + assert_eq!(spend.counted_orders, 2); + assert_eq!(spend.valued_orders, 1); + assert_eq!( + spend.unified(), + None, + "the independently seeded USDC row has no rate, so no complete unified denominator exists" + ); +} + /// Merging physical sources must keep each quote's reconstruction count beside its own subtotal. /// Collapsing that pair back into an optional amount blanks the USDT bucket over one unprovable /// row; reusing profit coverage instead would suppress the complete USDC bucket in this fixture. diff --git a/crates/moon-core/src/db/report_read.rs b/crates/moon-core/src/db/report_read.rs index 598ef9b8..716ee452 100644 --- a/crates/moon-core/src/db/report_read.rs +++ b/crates/moon-core/src/db/report_read.rs @@ -159,7 +159,8 @@ pub struct ReportTable { /// its own surfaces and would carry an eternally empty open tally. #[derive(Clone, Debug, Default, PartialEq)] pub struct ReportTotals { - /// Realized profit per known currency over CLOSED rows only, plus traded volume and coverage. + /// Realized profit per known currency over CLOSED rows only, plus traded volume, entry-spend + /// subtotals, and coverage. pub quotes: QuoteBreakdown, /// Unrealized money on the positions still running, counted apart from every figure above. pub open: super::OpenPositions, @@ -1337,7 +1338,8 @@ pub fn strategy_purge_rows( /// /// Returns: /// Exact known-currency profit buckets over CLOSED rows, unknown and complete row counts, -/// closed non-Funding traded volume with its per-quote reconstruction counts, optional complete +/// closed non-Funding traded volume with its per-quote reconstruction counts, counted +/// entry-spend subtotals for [`QuoteBreakdown::average_order_return`], optional complete /// active-mode USDT coverage, and the still-running positions counted separately beside them. /// /// Errors: @@ -1517,6 +1519,106 @@ fn traded_volume_sql(src: &ReadSource, rate: Option<&str>) -> TradedVolumeSql { } } +/// Per-source SQL for the counted spend/profit subtotal, plus its own unified USDT leg, behind +/// [`QuoteBreakdown::average_order_return`], independent of the Report's row/profit filter — +/// exactly like [`TradedVolumeSql`] beside it, and for the same reason: `rate` is taken so the +/// unified leg is this feature's OWN, never [`super::UsdtTotal::spent`], which carries no +/// positive-spend guard and no Funding exclusion. +struct EntrySpendSql { + /// Counted-row predicate: positive numeric settled spend, numeric settled profit, and + /// non-Funding. + counted: String, + /// The row's settled spend, or SQL NULL when the source cannot evidence it. + spent: String, + /// The row's settled profit, or `0.0` when the source cannot evidence it. + profit: String, + /// Active-mode USDT rate, or SQL NULL when no valuation projection exists. + rate: String, +} + +impl EntrySpendSql { + /// Build the six grouped columns consumed by [`super::EntrySpend::from_groups`]. + /// + /// Returns: + /// Counted-row count, summed settled spend, summed settled profit, valued-row count, and + /// the summed USDT spend and profit over counted rows carrying a rate, in that order. + fn aggregate_columns(&self) -> String { + format!( + "COALESCE(SUM(CASE WHEN {counted} THEN 1 ELSE 0 END),0), + COALESCE(SUM(CASE WHEN {counted} THEN {spent} ELSE 0.0 END),0.0), + COALESCE(SUM(CASE WHEN {counted} THEN {profit} ELSE 0.0 END),0.0), + COALESCE(SUM(CASE WHEN {counted} AND ({rate}) IS NOT NULL + THEN 1 ELSE 0 END),0), + COALESCE(SUM(CASE WHEN {counted} AND ({rate}) IS NOT NULL + THEN ({spent}) * ({rate}) ELSE 0.0 END),0.0), + COALESCE(SUM(CASE WHEN {counted} AND ({rate}) IS NOT NULL + THEN ({profit}) * ({rate}) ELSE 0.0 END),0.0)", + counted = self.counted, + spent = self.spent, + profit = self.profit, + rate = self.rate, + ) + } +} + +/// Build the entry-spend SQL for one source. +/// +/// A counted row is CLOSED (the caller already restricts the scope), non-Funding, with a positive +/// numeric settled spend and a numeric settled profit. It takes the POSITIVE-SPEND half from the +/// house average-order definition (`analytics::groups::avg_order`, +/// `analytics::profit_monitor::average_order`) and ADDS the Funding exclusion on top — the two are +/// deliberately NOT identical, because neither Analytics query filters `sellreason`, so a +/// positive-spend Funding row moves their averages and not this one. Do not "restore parity" in +/// either direction without deciding which surface is wrong. The numeric-profit and settled-spend +/// legs reuse [`super::valuation::source_predicates`] so this cannot silently disagree with the +/// valuation cache about which rows are eligible. +/// +/// On a source that cannot express `closedate` the realized pass widens to `ClosedAndOpen`, and +/// this leg inherits that exactly as the PROFIT total does rather than failing closed the way the +/// neighbouring volume leg does. That asymmetry is deliberate twice over: the percentage is a +/// ratio to the profit figure in the row's head, so a denominator that excluded rows the +/// numerator kept would state a ratio between two different scopes — the exact defect plan review +/// caught in the unified arm — and such a source is in practice a legacy ARCHIVE table of +/// already-closed trades, not a live one carrying open positions. +/// +/// Args: +/// src: Physical Report source and its discovered columns. +/// rate: Active-mode quote-to-USDT expression when a projection is available, taken the same +/// way [`traded_volume_sql`] takes it. +/// +/// Returns: +/// Fail-closed counted predicate, settled spend/profit expressions naming only columns the +/// source actually has, and the valuation rate expression. +fn entry_spend_sql(src: &ReadSource, rate: Option<&str>) -> EntrySpendSql { + let has = |column: &str| src.cols.contains(column); + let predicates = super::valuation::source_predicates("r", &src.cols); + // Without `sellreason` a closed row cannot be proven NOT Funding, so it counts nothing — the + // same fail-closed direction `traded_volume_sql` takes rather than risking a Funding row + // inflating the average. + let funding = if has("sellreason") { + "COALESCE(r.\"sellreason\", '') <> 'Funding'".to_string() + } else { + "0".to_string() + }; + // `> 0` on a NULL spend is NULL in SQLite, so a non-numeric or absent spend excludes itself + // without a second `typeof` test. + let counted = format!( + "(({spent}) > 0 AND {numeric_profit} AND {funding})", + spent = predicates.spent_value, + numeric_profit = predicates.numeric_profit, + ); + EntrySpendSql { + counted, + spent: predicates.spent_value, + profit: if has("profitbtc") { + super::quote::settled_amount_expr("r", &src.cols, "profitbtc") + } else { + "0.0".to_string() + }, + rate: rate.unwrap_or("NULL").to_string(), + } +} + /// Execute one complete Report totals pass with fresh accumulators. /// /// Args: @@ -1527,9 +1629,10 @@ fn traded_volume_sql(src: &ReadSource, rate: Option<&str>) -> TradedVolumeSql { /// current-rate mode does not depend on it. /// /// Returns: -/// Exact quote profit totals over closed rows and two-sided traded volume over the -/// reconstructed rows of each quote, optionally carrying active-mode USDT coverage for each -/// metric, plus the unrealized tally of the rows still open. +/// Exact quote profit totals over closed rows, two-sided traded volume over the reconstructed +/// rows of each quote, and counted entry-spend subtotals over the same closed rows, +/// optionally carrying active-mode USDT coverage for each metric, plus the unrealized tally of +/// the rows still open. /// /// Errors: /// Returns the underlying SQLite error from any physical-source aggregate. @@ -1542,6 +1645,7 @@ fn query_totals_attempt( let mut groups = Vec::new(); let mut open_groups = Vec::new(); let mut volume_groups = Vec::new(); + let mut spend_groups = Vec::new(); let mut coverage = super::valuation::CoverageAggregate::default(); let has_strategy_names = strategy_metadata_required(f) && super::analytics::strategies_attached(conn); @@ -1597,8 +1701,15 @@ fn query_totals_attempt( .map(|parts| parts.per_row.quote_rate.as_str()), ); let volume_columns = volume_sql.aggregate_columns(); + let spend_sql = entry_spend_sql( + src, + valuation + .as_ref() + .map(|parts| parts.per_row.quote_rate.as_str()), + ); + let spend_columns = spend_sql.aggregate_columns(); let sql = format!( - "SELECT {quote}, {profit}, COUNT(*){coverage_columns}, {volume_columns} + "SELECT {quote}, {profit}, COUNT(*){coverage_columns}, {volume_columns}, {spend_columns} FROM {} r{joins}{where_sql}{group_by}", src.table, ); @@ -1606,6 +1717,7 @@ fn query_totals_attempt( let mut stmt = conn.prepare(&sql)?; let mut rows = stmt.query(refs.as_slice())?; let volume_offset = 3 + usize::from(valuation.is_some()) * 6; + let spend_offset = volume_offset + 5; while let Some(row) = rows.next()? { let raw = row.get::<_, Value>(0)?; let ordinal = super::quote::report_ordinal_from_value(&raw); @@ -1623,6 +1735,15 @@ fn query_totals_attempt( row.get::<_, i64>(volume_offset + 3)?, row.get::<_, f64>(volume_offset + 4)?, )); + spend_groups.push(( + ordinal, + row.get::<_, i64>(spend_offset)?, + row.get::<_, f64>(spend_offset + 1)?, + row.get::<_, f64>(spend_offset + 2)?, + row.get::<_, i64>(spend_offset + 3)?, + row.get::<_, f64>(spend_offset + 4)?, + row.get::<_, f64>(spend_offset + 5)?, + )); } } // The open pass: a plain per-quote tally, with no window, no coverage and no volume — none of @@ -1653,7 +1774,8 @@ fn query_totals_attempt( } } let quotes = QuoteBreakdown::from_groups(groups) - .with_traded_volume(super::TradedVolume::from_groups(volume_groups)); + .with_traded_volume(super::TradedVolume::from_groups(volume_groups)) + .with_entry_spend(super::EntrySpend::from_groups(spend_groups)); // Publish coverage whenever the selected mode can build a projection: always for current rates, // and only with an attached cache for historical rates. Ok(ReportTotals { diff --git a/crates/moon-core/src/db/report_read/tests.rs b/crates/moon-core/src/db/report_read/tests.rs index 110a5b49..809706b4 100644 --- a/crates/moon-core/src/db/report_read/tests.rs +++ b/crates/moon-core/src/db/report_read/tests.rs @@ -475,6 +475,61 @@ fn traded_volume_report(quote: i64) -> Connection { conn } +/// `report_read.rs::entry_spend_sql` must retain its non-Funding predicate. Dropping +/// `AND {funding}` would count the deliberately positive-spend Funding row in the average-order +/// denominator, quietly moving every single-core Report percentage and understating exclusions. +#[test] +fn average_order_spend_excludes_positive_spend_funding_rows() { + let conn = traded_volume_report(1); + let totals = query_totals( + &conn, + &ReportFilter { + core_uids: vec![1], + ..ReportFilter::default() + }, + ) + .expect("query average-order spend") + .quotes; + + assert_eq!( + totals.orders, 3, + "two trades plus the Funding row are closed" + ); + assert_eq!(totals.entry_spend.counted_orders, 2); + assert_eq!(totals.entry_spend.totals.len(), 1); + let usdt = &totals.entry_spend.totals[0]; + assert_eq!(usdt.orders, 2); + assert_eq!(usdt.spent, 3.0, "only the 1.0 and 2.0 trade spends count"); + assert_eq!( + usdt.profit, 50.0, + "the denominator and profit share those two rows" + ); +} + +/// `report_read.rs::query_totals_attempt` must keep `spend_offset = volume_offset + 5`. Moving +/// it one column in either direction reads volume data as spend data or reads past the result, +/// producing a wrong or failed Report average-order footer. +#[test] +fn average_order_spend_columns_follow_the_complete_volume_block() { + let conn = traded_volume_report(1); + let totals = query_totals( + &conn, + &ReportFilter { + core_uids: vec![1], + ..ReportFilter::default() + }, + ) + .expect("query ordered aggregate columns") + .quotes; + + let usdt = totals + .entry_spend + .totals + .first() + .expect("the independently seeded USDT rows create one spend bucket"); + assert_eq!((usdt.orders, usdt.spent, usdt.profit), (2, 3.0, 50.0)); +} + /// Removing the exit leg, signing short quantity, reusing `spentbtc`, moving the closed/Funding /// predicates into `build_where`, or coupling volume completeness to profit coverage turns one of /// these exact assertions red and would misstate the current filtered Report footer. diff --git a/crates/moon-ui-gpui/src/analytics/summary/charts.rs b/crates/moon-ui-gpui/src/analytics/summary/charts.rs index 0fc8f616..f62455dd 100644 --- a/crates/moon-ui-gpui/src/analytics/summary/charts.rs +++ b/crates/moon-ui-gpui/src/analytics/summary/charts.rs @@ -16,6 +16,102 @@ use crate::design::{moon, moon_alpha}; use moon_core::db::analytics::{CoreSeries, DayPoint, KindStat}; pub(super) const CHART_H: f32 = 170.0; +/// Weight the chart value labels are drawn at — they set none, so it is GPUI's normal. +/// `mono_caption_text_width` needs it as a number. +pub(super) const LABEL_WEIGHT: f32 = 400.0; + +/// Width of the WIDEST of a set of already-formatted value labels, in pixels. +/// +/// MEASURED, never a constant. A guessed width is wrong in both directions and both are +/// expensive: too narrow and the collision passes believe two labels are clear when the digits +/// overlap — the exact defect they exist to prevent — too wide and they drop labels that would +/// have fitted. The old constants were guessed against example strings (`"-1268 USDT"`), and +/// that example was not even a string these charts produce: `pnl_suffix` is `"%"` or nothing, +/// never a ticker. +/// +/// The Analytics view draws in the MONO family (`analytics/render.rs`), so a glyph advance is +/// the same for every character and the widest label is simply the longest one — picking it by +/// `chars().count()` and measuring that one string is exact, not an approximation, and costs a +/// single measurement per frame rather than one per label. +/// +/// Args: +/// texts: The formatted labels this chart is about to consider drawing. +/// cx: Analytics view context. +/// +/// Returns: +/// Width of the longest label in pixels, or 0.0 when there are none. +pub(super) fn widest_label_w<'a>( + texts: impl Iterator, + cx: &Context, +) -> f32 { + texts + .max_by_key(|s| s.chars().count()) + .map(|s| design::mono_caption_text_width(cx, s, LABEL_WEIGHT)) + .unwrap_or(0.0) +} +/// Plot width both summary charts assume, in RAW pixels — deliberately neither font- nor +/// UI-scaled. +/// +/// The card's real width is a layout result these functions never learn: it is only known +/// inside the paint closure, while the labels are absolutely-positioned divs outside it, and +/// threading a measured width in would widen both chart signatures through the summary card. +/// So the pass measures against the narrowest the card realistically gets — the Analytics +/// window's own minimum (860 wide, `analytics/mod.rs`) less the page padding (2x10), the +/// inter-card gap (8) and the card's own padding (2x12), over two equal cards: +/// `(860 - 20 - 8) / 2 - 24`. At the default 1240-wide window the real plot is ~584, so +/// assuming the minimum only ever drops a label that would in fact have fitted — the safe +/// direction; assuming wide puts overlap back. +/// +/// **Raw, not scaled.** 860 is a fixed physical size and every padding it is reduced by goes +/// through `ui()`, which tracks the UI scale but NOT the Font slider — so a larger font never +/// widens this plot, and a larger UI scale only eats further INTO it. Passing this through +/// `font_w_px` would have grown the room the collision pass believes it has at exactly the +/// setting that makes the labels physically wider, which is the application's default (+3). +/// +/// ONE constant for BOTH charts on purpose: they are the two `flex_1` siblings of the same +/// row, with the same page padding, the same gap and the same card chrome. Two copies could +/// drift apart from a layout that cannot. +pub(super) const PLOT_W_NOMINAL: f32 = 392.0; + +/// Which buckets keep a value label once the bars are denser than the labels are wide. +/// +/// Greedy by descending |value|: the biggest day is always labelled, and every later candidate +/// is kept only if its column sits at least one label-width from every column already kept. +/// That is what replaced the old all-or-nothing `days.len() <= 45` cutoff, which drew a slab of +/// colliding digits just under the limit and NOTHING at all just over it — so on «Все» the +/// chart said nothing about its own extremes. +/// +/// Selecting by magnitude rather than by every N-th bucket is deliberate: the days worth naming +/// on a profit chart are the big ones, and an every-N-th rule names whichever days the stride +/// happens to land on. +/// +/// Args: +/// vals: Per-bucket profit, ordered, one entry per bar. +/// plot_w: Width the bars are laid out across, in pixels. +/// label_w: Width of one label, in pixels. +/// +/// Returns: +/// Indices into `vals` to label, ascending. +fn thinned_labels(vals: &[f64], plot_w: f32, label_w: f32) -> Vec { + let n = vals.len(); + if n == 0 { + return Vec::new(); + } + // Bars are equal flex cells, so bucket `i` is centred at (i+0.5)/n of the width — the same + // mapping the hover popup uses. + let x = |i: usize| (i as f32 + 0.5) / n as f32 * plot_w; + let mut order: Vec = (0..n).collect(); + // Index as the tie-break, so two equal days never swap between frames. + order.sort_by(|&a, &b| vals[b].abs().total_cmp(&vals[a].abs()).then(a.cmp(&b))); + let mut kept: Vec = Vec::new(); + for i in order { + if kept.iter().all(|&j| (x(i) - x(j)).abs() >= label_w) { + kept.push(i); + } + } + kept.sort_unstable(); + kept +} /// FALLBACK color for a core's series (cycled from the palette) — used when /// the server has no color in its settings (e.g. the core is already gone from @@ -130,15 +226,46 @@ pub(super) fn daily_bars( .min(0.0); let span = (vmax - vmin).max(1e-6); let up_frac = (vmax / span) as f32; // share of the height above the zero line - // Value labels stay readable only while the bars are few. - let labels_on = days.len() <= 45; + // Which bars keep a label: as many as fit side by side at the caption size, biggest + // days first. Never a font shrink — the label is at `t_caption` or it is not drawn. + // Format every candidate ONCE: the widest of them sizes the thinning pass, and the drawn + // ones are taken straight from here rather than formatted a second time below. + let texts: Vec = days + .iter() + .map(|d| { + format!( + "{}{}", + moon_core::util::fmt::compact(d.profit, 0), + crate::analytics::pnl_suffix() + ) + }) + .collect(); + // The label WIDTH is text and is measured at the caption size, so it already follows the + // Font slider; the plot width is layout and must not (see `PLOT_W_NOMINAL`). Scaling both + // together would let the room grow with the very setting that makes the labels wider. + let label_w = widest_label_w(texts.iter().map(String::as_str), cx); + let profits: Vec = days.iter().map(|d| d.profit).collect(); + let labelled: std::collections::BTreeSet = + thinned_labels(&profits, PLOT_W_NOMINAL, label_w) + .into_iter() + .collect(); + // A bar is labelled only if it also TRADED, so the reserved band must ask the same + // question: an all-quiet period has candidates but draws nothing, and reserving room for + // labels that never appear just shortens every bar. + let labels_on = labelled.iter().any(|&bi| days[bi].trades > 0); // Space reserved for the labels: always on top (above the tallest green // bar), on the bottom only when there is a negative value (the label goes // BELOW a red bar). Bars scale into the remaining height, so the numbers - // are never covered by a column. - let pad_top = if labels_on { 13.0f32 } else { 0.0 }; + // are never covered by a column. Derived from the caption size rather than a + // fixed 13px, which was sized for the 8px text this chart used to draw and + // stopped clearing the line box the moment the Font slider moved. + // Text-derived height plus a small fixed clearance, and the two take DIFFERENT scales on + // purpose: `t_caption` follows the Font slider, the clearance goes through `ui_px` like + // every other piece of chrome (`cumulative.rs` sizes its own band the same way). + let label_band = f32::from(design::t_caption(cx)) * 1.4 + f32::from(design::ui_px(cx, 2.0)); + let pad_top = if labels_on { label_band } else { 0.0 }; let pad_bottom = if labels_on && vmin < 0.0 { - 13.0f32 + label_band } else { 0.0 }; @@ -158,7 +285,7 @@ pub(super) fn daily_bars( let bottom = if d.profit >= 0.0 { zero_from_bottom } else { - (zero_from_bottom - bar_h).max(pad_bottom - 13.0) + (zero_from_bottom - bar_h).max(pad_bottom - label_band) }; let mut col = div() .id(SharedString::from(format!("an-db-{bi}"))) @@ -189,31 +316,35 @@ pub(super) fn daily_bars( if hover == Some(bi) { col = col.bg(moon_alpha(p.text_muted, 0.07)); } - if labels_on && d.trades > 0 { + if labelled.contains(&bi) && d.trades > 0 { // Label: above a green bar / below a red one (the space is // reserved by pad_top/pad_bottom, so no bar covers the number). let label_bottom = if d.profit >= 0.0 { bottom + bar_h + 2.0 } else { - (bottom - 12.0).max(0.0) + (bottom - label_band).max(0.0) }; - // The label is wider than its column (±24px on each side) and does - // not wrap — otherwise "333" got clipped to "33". Neighbouring - // labels may touch slightly, but every number stays fully readable. + // The label is wider than its column and does not wrap — otherwise + // "333" got clipped to "33". The overhang is half a label on each + // side, so it matches the width the thinning pass kept the columns + // apart by; neighbours can no longer touch. + let over = px(label_w / 2.0); col = col.child( div() .absolute() - .left(px(-24.0)) - .right(px(-24.0)) + .left(-over) + .right(-over) .bottom(px(label_bottom)) - .text_size(px(8.0)) + .text_size(design::t_caption(cx)) .whitespace_nowrap() .text_color(moon(super::sign_color(p, d.profit))) - .child(div().w_full().flex().justify_center().child(format!( - "{}{}", - moon_core::util::fmt::compact(d.profit, 0), - crate::analytics::pnl_suffix() - ))), + .child( + div() + .w_full() + .flex() + .justify_center() + .child(texts[bi].clone()), + ), ); } row = row.child(col); diff --git a/crates/moon-ui-gpui/src/analytics/summary/charts/tests.rs b/crates/moon-ui-gpui/src/analytics/summary/charts/tests.rs index a1b74713..c156d8ed 100644 --- a/crates/moon-ui-gpui/src/analytics/summary/charts/tests.rs +++ b/crates/moon-ui-gpui/src/analytics/summary/charts/tests.rs @@ -1,6 +1,6 @@ //! Unit tests for the pure selection and normalization rules behind the per-core ranking. -use super::{core_rank_rows, core_rank_stats, overview_ranges}; +use super::{core_rank_rows, core_rank_stats, overview_ranges, thinned_labels}; use moon_core::db::analytics::CoreSeries; /// Build the minimum core series needed by ranking helpers. @@ -71,3 +71,16 @@ fn ranking_bars_share_one_absolute_scale() { assert_eq!(rows[0].magnitude_pct, 50.0); assert_eq!(rows[1].magnitude_pct, 100.0); } + +/// `charts.rs:thinned_labels` must keep only separated bucket labels, prioritizing larger absolute profits. +/// Deleting its separation predicate labels adjacent bars together, while natural ordering hides the largest adjacent daily move. +#[test] +fn thinned_labels_keep_spaced_daily_extremes() { + let labels = thinned_labels(&[90.0, -100.0, 80.0, 0.0, 70.0], 100.0, 30.0); + + assert_eq!( + labels, + vec![1, 4], + "only the largest daily move in each overlapping label region may remain" + ); +} diff --git a/crates/moon-ui-gpui/src/analytics/summary/cumulative.rs b/crates/moon-ui-gpui/src/analytics/summary/cumulative.rs index 7af074db..b38f731d 100644 --- a/crates/moon-ui-gpui/src/analytics/summary/cumulative.rs +++ b/crates/moon-ui-gpui/src/analytics/summary/cumulative.rs @@ -12,7 +12,10 @@ use gpui::*; use moon_ui::{MoonPalette, h_flex, v_flex}; use super::super::AnalyticsView; -use super::charts::{CHART_H, PopupMode, bucket_label, bucket_popup, core_color, muted_caption}; +use super::charts::{ + CHART_H, PLOT_W_NOMINAL, PopupMode, bucket_label, bucket_popup, core_color, muted_caption, + widest_label_w, +}; use crate::design; use crate::design::{moon, moon_alpha}; use moon_core::db::analytics::{CoreSeries, DayPoint}; @@ -23,8 +26,6 @@ use moon_core::db::analytics::{CoreSeries, DayPoint}; /// apart. The hover popup has its own, larger limit (`POPUP_ROWS`), so a core with no line /// can still be read there. pub(super) const MAX_CORE_LINES: usize = 12; -/// Width of one swing label ("+12345.67"), in font units. -const LABEL_W: f32 = 62.0; /// A swing is confirmed once the curve retraces this share of the TOTAL's own range. /// Smaller → every wiggle becomes a label; larger → only the biggest moves survive. const SWING_FRAC: f32 = 0.07; @@ -114,6 +115,72 @@ fn swing_labels(pts: &[f32]) -> Vec { out } +/// Second placement pass: drop the swings whose label RECTANGLE would land on an already-kept +/// one. `swing_labels` only bounds the label COUNT — a curve with fourteen genuine turns still +/// puts fourteen 62-unit labels across a ~390px plot, which is where "-2911.63" ended up on top +/// of "-2511.95". A count cannot see that; a rectangle can. +/// +/// Both axes are tested, because either alone is wrong here: two labels at the same X are fine +/// when the curve moved far vertically between them, and two labels at the same height are fine +/// when they sit weeks apart. Only an overlap in BOTH is unreadable. +/// +/// The survivor of a collision is always the BIGGER move — the boxes are considered in +/// descending |value| order, so the label that gets dropped is the one that says less. A label +/// is never nudged off its point: a number drawn away from the point it belongs to lies about +/// when the swing happened, which is worse than not drawing it. +/// +/// Args: +/// boxes: One `(x_centre, y_centre, value)` per candidate label, in plot pixels. +/// w: Label width in pixels. +/// h: Label height in pixels. +/// +/// Returns: +/// Indices INTO `boxes` of the labels to draw, ascending. +fn place_labels(boxes: &[(f32, f32, f64)], w: f32, h: f32) -> Vec { + // Descending |value|, with the index as the tie-break so the result never depends on sort + // stability — two swings of the exact same magnitude must not swap between frames. + let mut order: Vec = (0..boxes.len()).collect(); + order.sort_by(|&a, &b| { + boxes[b] + .2 + .abs() + .total_cmp(&boxes[a].2.abs()) + .then(a.cmp(&b)) + }); + let mut kept: Vec = Vec::new(); + for i in order { + let (x, y, _) = boxes[i]; + let clear = kept.iter().all(|&j| { + let (kx, ky, _) = boxes[j]; + (x - kx).abs() >= w || (y - ky).abs() >= h + }); + if clear { + kept.push(i); + } + } + kept.sort_unstable(); + kept +} + +/// How far a label is pulled left of its point, as a multiple of the label width. +/// +/// A label is centred on its point, except at the very ends: there it is pulled fully inside, +/// or half of it would hang past the card's edge and nothing clips it. +/// +/// This is a function rather than two copies of the same `if` because BOTH the layout and the +/// collision pass need it: the pass compares label CENTRES, and a centre computed as `frac * w` +/// is off by half a label at `frac == 0` — exactly where the first swing sits — which is enough +/// to call a real overlap clear. +fn label_shift(frac: f32) -> f32 { + if frac <= 0.0 { + 0.0 + } else if frac >= 1.0 { + -1.0 + } else { + -0.5 + } +} + /// Cumulative profit: the total as an area + line, the per-core curves inside it, swing /// labels carrying the running total, and a per-date hover popup. /// @@ -212,13 +279,21 @@ pub(super) fn cumulative_area( // moment the UI font-size slider grows it. 1.7× covers gpui's own line height (≈1.618×). // Clamped: at an extreme font scale the band could otherwise eat the whole plot. let lift = f32::from(design::ui_px(cx, 3.0)); - let band = f32::from(design::t_caption(cx)) * 1.7 + lift; + let label_h = f32::from(design::t_caption(cx)) * 1.7; + // The swings decide whether there is a band at all: a single-bucket period has no turn to + // label, and reserving the strip anyway would shorten the curve for text that never comes. + let swings = swing_labels(&pts); + let band = if swings.is_empty() { + 0.0 + } else { + label_h + lift + }; let plot_h = (CHART_H - band).max(20.0); // Labels: resolved HERE against the same vmin/span the canvas paints with, so the // points can move into the paint closure instead of being cloned for it. - let labels: Vec<(f32, f32, String, u32)> = swing_labels(&pts) - .into_iter() - .map(|k| { + let cands: Vec<(f32, f32, String, u32)> = swings + .iter() + .map(|&k| { let y_up = (pts[k] - vmin) / span * (plot_h - 2.0) + 1.0; let frac = if n > 1 { k as f32 / (n - 1) as f32 @@ -233,6 +308,27 @@ pub(super) fn cumulative_area( ) }) .collect(); + // Collision pass over the rectangles as they will actually be laid out — so the X here is + // the label's true CENTRE, `label_shift` included, not the bare point it hangs from. The + // X positions are fractions of a width this function never learns, so they resolve against + // `PLOT_W_NOMINAL`; the Y positions are already the real pixel offsets. Only the label + // WIDTH tracks the font — it is MEASURED off the longest label actually formatted, so the + // box the pass tests is the box that lands, at any font size; the plot width is layout and + // must not grow with the Font slider. + let lwf = widest_label_w(cands.iter().map(|(_, _, t, _)| t.as_str()), cx); + let lw = px(lwf); + let boxes: Vec<(f32, f32, f64)> = swings + .iter() + .zip(&cands) + .map(|(&k, (frac, y_up, _, _))| { + let left = frac * PLOT_W_NOMINAL + label_shift(*frac) * lwf; + (left + lwf / 2.0, *y_up, cum[k]) + }) + .collect(); + let labels: Vec<(f32, f32, String, u32)> = place_labels(&boxes, lwf, label_h) + .into_iter() + .map(|i| cands[i].clone()) + .collect(); let pts_paint = pts; let canvas_el = canvas( @@ -334,17 +430,8 @@ pub(super) fn cumulative_area( .child(canvas_el), ) .child(hover_row(n, hover, p, cx)); - let lw = design::font_w_px(cx, LABEL_W); for (frac, y_up, text, col) in labels { - // Centred on its point, except at the very ends: there the label is pulled fully - // inside, or half of it would hang past the card's edge (nothing clips it). - let shift = if frac <= 0.0 { - px(0.0) - } else if frac >= 1.0 { - -lw - } else { - -lw / 2.0 - }; + let shift = lw * label_shift(frac); stack = stack.child( div() .absolute() @@ -355,6 +442,16 @@ pub(super) fn cumulative_area( .flex() .justify_center() .whitespace_nowrap() + // A chip behind the digits. The twelve per-core curves run straight through + // this band, and a bare number drawn over three of them is unreadable however + // well it is placed. The label is NOT moved off its point to dodge them — + // a number drawn away from its point lies about when the swing happened — so + // the lines are dimmed behind it instead. Panel-toned and rounded so it reads + // as chrome rather than as data. No padding, deliberately: the chip is exactly + // the box `place_labels` measured, and padding it would make the drawn + // rectangle wider than the one the collision pass cleared. + .rounded(design::ui_px(cx, 3.0)) + .bg(moon_alpha(p.panel, 0.82)) .text_size(design::t_caption(cx)) .text_color(moon(col)) .child(text), diff --git a/crates/moon-ui-gpui/src/analytics/summary/cumulative/tests.rs b/crates/moon-ui-gpui/src/analytics/summary/cumulative/tests.rs index 08a4ca0d..84baf3e4 100644 --- a/crates/moon-ui-gpui/src/analytics/summary/cumulative/tests.rs +++ b/crates/moon-ui-gpui/src/analytics/summary/cumulative/tests.rs @@ -1,4 +1,30 @@ -use super::{MAX_SWING_LABELS, swing_labels, swing_points}; +use super::{MAX_SWING_LABELS, place_labels, swing_labels, swing_points}; + +/// `cumulative.rs:place_labels` must retain labels separated vertically by one full label height. +/// Changing its clear-on-either-axis `||` to `&&` drops a readable swing label, hiding a period move. +#[test] +fn place_labels_keeps_labels_clear_on_one_axis() { + let labels = place_labels(&[(0.0, 0.0, 100.0), (0.0, 10.0, 50.0)], 10.0, 10.0); + + assert_eq!( + labels, + vec![0, 1], + "vertical clearance must keep both labels" + ); +} + +/// `cumulative.rs:place_labels` must prefer the larger absolute move when label rectangles overlap. +/// Replacing its descending-magnitude ordering with natural order keeps the smaller swing and hides the period's largest move. +#[test] +fn place_labels_keeps_the_larger_colliding_move() { + let labels = place_labels(&[(0.0, 0.0, 12.0), (2.0, 1.0, -30.0)], 10.0, 10.0); + + assert_eq!( + labels, + vec![1], + "the larger absolute move must survive the collision" + ); +} /// A saw that turns at EVERY bucket: the threshold must be backed off until the labels /// fit, or the chart becomes a wall of overlapping numbers. diff --git a/crates/moon-ui-gpui/src/analytics/summary/mod.rs b/crates/moon-ui-gpui/src/analytics/summary/mod.rs index edcf8002..f24aec2a 100644 --- a/crates/moon-ui-gpui/src/analytics/summary/mod.rs +++ b/crates/moon-ui-gpui/src/analytics/summary/mod.rs @@ -416,12 +416,6 @@ impl AnalyticsView { /// Row of KPI tiles with deltas against the previous period. fn kpi_row(&self, d: &Summary, p: MoonPalette, cx: &Context) -> impl IntoElement { let (cur, prev) = (&d.cur, &d.prev); - // A missing or zero comparison value has no meaningful percentage - // delta, so the KPI tile renders an em dash. - let delta = |c: f64, pr: Option| -> Option { - let pr = pr?; - (pr.abs() > f64::EPSILON).then(|| (c - pr) / pr.abs() * 100.0) - }; let profit_el = colored_value(p, cur.profit, format!("{}", fmt_signed(cur.profit))); let dd_el = div() .text_color(moon(p.orange)) @@ -444,48 +438,48 @@ impl AnalyticsView { unit = crate::analytics::pnl_unit_label() ), profit_el, - delta(cur.profit, prev.as_ref().map(|v| v.profit)), - false, + pct_delta(cur.profit, prev.as_ref().map(|v| v.profit)), + DeltaGood::Up, )) .child(kpi( p, cx, t!("analytics.kpi.trades"), plain_value(p, cur.n.to_string()), - delta(cur.n as f64, prev.as_ref().map(|v| v.n as f64)), - false, + pct_delta(cur.n as f64, prev.as_ref().map(|v| v.n as f64)), + DeltaGood::Up, )) .child(kpi( p, cx, t!("analytics.kpi.winrate"), plain_value(p, format!("{:.1}%", cur.winrate())), - delta(cur.winrate(), prev.as_ref().map(|v| v.winrate())), - false, + pct_delta(cur.winrate(), prev.as_ref().map(|v| v.winrate())), + DeltaGood::Up, )) .child(kpi( p, cx, t!("analytics.kpi.pf"), plain_value(p, format!("{:.2}", cur.pf)), - delta(cur.pf, prev.as_ref().map(|v| v.pf)), - false, + pct_delta(cur.pf, prev.as_ref().map(|v| v.pf)), + DeltaGood::Up, )) .child(kpi( p, cx, t!("analytics.kpi.maxdd"), dd_el, - delta(cur.max_dd, prev.as_ref().map(|v| v.max_dd)), - true, + pct_delta(cur.max_dd, prev.as_ref().map(|v| v.max_dd)), + DeltaGood::Down, )) .child(kpi( p, cx, t!("analytics.kpi.avg"), avg_el, - delta(cur.avg, prev.as_ref().map(|v| v.avg)), - false, + pct_delta(cur.avg, prev.as_ref().map(|v| v.avg)), + DeltaGood::Up, )) .child(kpi( p, @@ -495,8 +489,8 @@ impl AnalyticsView { p, format!("{:.0} {}", cur.avg_dur_min, t!("analytics.minutes")), ), - delta(cur.avg_dur_min, prev.as_ref().map(|v| v.avg_dur_min)), - true, + pct_delta(cur.avg_dur_min, prev.as_ref().map(|v| v.avg_dur_min)), + DeltaGood::Neither, )) } } @@ -515,41 +509,128 @@ fn colored_value(p: MoonPalette, v: f64, text: String) -> AnyElement { .into_any_element() } +/// Which direction of change is GOOD for a metric, and therefore which colour its delta takes. +/// +/// Two of the seven tiles are not "up is good", and each is a different case. Max Drawdown is +/// stored as a MAGNITUDE — `Summary::max_dd` is `max(peak - cum)` and never negative — so ▲ means +/// a DEEPER drawdown and is the bad direction, even though the value printed beside it carries a +/// leading minus. Average duration has no good direction at all: a longer hold is neither a win +/// nor a loss, so its delta is toned down rather than claiming one. +/// +/// That neutral tone is `text_soft`, not `text_muted`, and the difference is not cosmetic: +/// `text_muted` on `panel` measures 3.50:1 on the Terminal palette and 3.70:1 on the Light one, +/// under the 4.5:1 floor for normal text at caption size. It is the right token for the em dash +/// and for the "vs previous period" suffix — chrome, and an ABSENCE of a figure — but the neutral +/// delta is a FIGURE the user reads. `text_soft` clears the floor in both themes (5.09:1 and +/// 6.96:1) and is already this tile's own label colour, so nothing new enters the palette. +enum DeltaGood { + /// Growth is good: profit, trades, winrate, profit factor, average trade. + Up, + /// Shrinkage is good: drawdown. + Down, + /// Neither direction is good: duration. + Neither, +} + +impl DeltaGood { + /// Pick the palette token for a delta whose ROUNDED sign is `sign`. + /// + /// Args: + /// p: Active MoonUI palette. + /// sign: Sign of the delta, classified from the rounded percentage. + /// + /// Returns: + /// `0xRRGGBB` token for the delta text. + fn tone(self, p: MoonPalette, sign: fmt::DeltaSign) -> u32 { + match self { + Self::Up => sign.pick(p.green, p.orange, p.text_soft), + Self::Down => sign.pick(p.orange, p.green, p.text_soft), + Self::Neither => p.text_soft, + } + } +} + +/// Percentage change of `cur` against the previous period's `prev`. +/// +/// A missing or zero comparison value has no meaningful percentage delta, so the KPI tile renders +/// an em dash. The denominator is `|prev|`, so the sign of the result is the sign of the CHANGE and +/// never of the baseline. +/// +/// Args: +/// cur: Current-period value. +/// prev: Previous-period value, when one was loaded. +/// +/// Returns: +/// Change in percent, or `None` when there is nothing to compare against. +fn pct_delta(cur: f64, prev: Option) -> Option { + let prev = prev?; + (prev.abs() > f64::EPSILON).then(|| (cur - prev) / prev.abs() * 100.0) +} + +/// Arrow text and palette token for one KPI delta — the whole colour rule of the row, in one place. +/// +/// [`fmt::pct`] classifies the sign from the ROUNDED percentage, so the arrow and the colour can +/// never disagree with the digits rendered beside them, and a delta that rounds away to zero — or +/// was never comparable — returns `None` and leaves the tile its em dash. `pct` prints that sign +/// into the text too, which the arrow already carries, hence the trim. +/// +/// Routing the digits through `pct` also puts this row on the module's ONE rounding rule — +/// [`fmt::round_to`]'s half-away-from-zero, the rule [`fmt::signed_amount`] documents at length — +/// instead of `{:.1}`'s half-to-even. Every exactly representable midpoint therefore moves by a +/// tenth of a point: `0.25%` renders `0.3%` where it used to render `0.2%`, and a delta of exactly +/// `0.05%` now shows `0.1%` where it used to fall under the old `> 0.05` guard into the em dash. +/// That is the point of the change, not a side effect of it — the alternative is a KPI row that +/// rounds differently from every other figure on the window. +/// +/// Args: +/// delta: Percentage change against the previous period, when comparable. +/// good: Which direction of change this metric calls good. +/// p: Active MoonUI palette. +/// +/// Returns: +/// The `"▲ 168.7%"`-shaped text with its `0xRRGGBB` token, or `None` for the em dash. +fn delta_parts(delta: Option, good: DeltaGood, p: MoonPalette) -> Option<(String, u32)> { + let (text, sign) = delta.and_then(|d| fmt::pct(d, 1))?; + if sign == fmt::DeltaSign::Zero { + return None; + } + Some(( + format!( + "{} {}", + sign.pick("▲", "▼", ""), + text.trim_start_matches('-') + ), + good.tone(p, sign), + )) +} + /// KPI tile: label (caption, muted) + large value + a ▲▼ delta. -/// `invert` — growth in this metric is bad (drawdown, duration). +/// `good` — which direction of change this metric calls good; see [`DeltaGood`]. fn kpi( p: MoonPalette, cx: &Context, label: impl std::fmt::Display, value: AnyElement, delta: Option, - invert: bool, + good: DeltaGood, ) -> impl IntoElement { - let delta_el = match delta { - Some(d) if d.is_finite() && d.abs() > 0.05 => { - let good = if invert { d < 0.0 } else { d > 0.0 }; - let col = if good { p.green } else { p.orange }; - h_flex() - .gap(design::ui_px(cx, 4.0)) - .items_center() - .child( - div() - .text_size(design::t_caption(cx)) - .text_color(moon(col)) - .child(format!( - "{} {:.1}%", - if d > 0.0 { "▲" } else { "▼" }, - d.abs() - )), - ) - .child( - div() - .text_size(design::t_caption(cx)) - .text_color(moon(p.text_muted)) - .child(t!("analytics.vs_prev").to_string()), - ) - .into_any_element() - } + let delta_el = match delta_parts(delta, good, p) { + Some((text, col)) => h_flex() + .gap(design::ui_px(cx, 4.0)) + .items_center() + .child( + div() + .text_size(design::t_caption(cx)) + .text_color(moon(col)) + .child(text), + ) + .child( + div() + .text_size(design::t_caption(cx)) + .text_color(moon(p.text_muted)) + .child(t!("analytics.vs_prev").to_string()), + ) + .into_any_element(), _ => div() .text_size(design::t_caption(cx)) .text_color(moon(p.text_muted)) diff --git a/crates/moon-ui-gpui/src/analytics/summary/tests.rs b/crates/moon-ui-gpui/src/analytics/summary/tests.rs index 6211b315..5f81d5fa 100644 --- a/crates/moon-ui-gpui/src/analytics/summary/tests.rs +++ b/crates/moon-ui-gpui/src/analytics/summary/tests.rs @@ -2,9 +2,10 @@ //! //! Explicit imports, never `use super::*`: the parent re-exports `gpui::*`, whose own `test` //! would shadow the built-in `#[test]` attribute and make it expand recursively (CONTRIBUTING.md). -use super::fmt_signed_unit; +use super::{DeltaGood, delta_parts, fmt_signed_unit, pct_delta}; use crate::analytics::{pnl_unit_label, set_pnl_unit}; use moon_core::db::{ProfitUnit, QuoteCurrency}; +use moon_ui::MoonPalette; use rust_i18n::t; /// The unit word must track both the active metric and exact persisted quote currency. @@ -62,3 +63,77 @@ fn insight_sentence_unit_follows_the_metric() { let usdt = render(); assert!(usdt.contains("USDT"), "usdt mode lost the unit: {usdt}"); } + +/// Swapping the positive and negative palette arguments in `DeltaGood::tone`'s `Down` arm makes +/// a deeper maximum drawdown render green, telling the user that a worsening period improved. +#[test] +fn kpi_delta_tones_follow_each_metrics_good_direction() { + let p = MoonPalette::LIGHT; + let cases = [ + ( + "deeper max drawdown", + DeltaGood::Down, + 2690.0, + 7228.21, + p.orange, + ), + ( + "shallower max drawdown", + DeltaGood::Down, + 7228.21, + 2690.0, + p.green, + ), + ( + "longer duration", + DeltaGood::Neither, + 30.0, + 60.0, + p.text_soft, + ), + ( + "shorter duration", + DeltaGood::Neither, + 60.0, + 30.0, + p.text_soft, + ), + ("improving up metric", DeltaGood::Up, 100.0, 150.0, p.green), + ("worsening up metric", DeltaGood::Up, 150.0, 100.0, p.orange), + ]; + + for (name, good, prev, cur, expected_tone) in cases { + let expected_arrow = if cur > prev { "▲" } else { "▼" }; + let (text, tone) = delta_parts(pct_delta(cur, Some(prev)), good, p) + .unwrap_or_else(|| panic!("{name} must have a visible delta")); + assert!( + text.starts_with(expected_arrow), + "{name} must point {expected_arrow}: {text}" + ); + assert_eq!( + tone, expected_tone, + "{name} must use its independently chosen palette token" + ); + } + + assert_eq!( + delta_parts(Some(0.05), DeltaGood::Up, p), + Some(("▲ 0.1%".to_string(), p.green)), + "a 0.05% increase rounds to the visible 0.1% boundary" + ); + assert_eq!( + pct_delta(100.0, None), + None, + "no previous period has no delta" + ); + assert_eq!( + pct_delta(100.0, Some(0.0)), + None, + "a zero previous period has no meaningful percentage" + ); + assert_eq!( + delta_parts(pct_delta(100.02, Some(100.0)), DeltaGood::Up, p), + None, + "a delta that rounds away leaves the tile's muted em dash" + ); +} diff --git a/crates/moon-ui-gpui/src/analytics/toolbar/tests.rs b/crates/moon-ui-gpui/src/analytics/toolbar/tests.rs index e74677a7..7247d4b4 100644 --- a/crates/moon-ui-gpui/src/analytics/toolbar/tests.rs +++ b/crates/moon-ui-gpui/src/analytics/toolbar/tests.rs @@ -104,6 +104,7 @@ fn found(n: i64) -> Option { orders: n, valuation: None, traded_volume: Default::default(), + entry_spend: Default::default(), }, }) } diff --git a/crates/moon-ui-gpui/src/analytics/tuner/list/mod.rs b/crates/moon-ui-gpui/src/analytics/tuner/list/mod.rs index b96e8d47..5f7f06f1 100644 --- a/crates/moon-ui-gpui/src/analytics/tuner/list/mod.rs +++ b/crates/moon-ui-gpui/src/analytics/tuner/list/mod.rs @@ -16,9 +16,8 @@ mod tests; use gpui::*; use moon_ui::{ - MoonButtonSegment, MoonButtonSize, MoonButtonVariant, MoonCheckbox, MoonCheckboxSize, - MoonDropdown, MoonInput, MoonInputEvent, MoonInputState, MoonMenuItem, MoonMenuSize, - MoonPalette, h_flex, + MoonButtonSize, MoonButtonVariant, MoonCheckbox, MoonCheckboxSize, MoonDropdown, MoonInput, + MoonInputEvent, MoonInputState, MoonMenuItem, MoonMenuSize, MoonPalette, h_flex, }; use rust_i18n::t; use std::cmp::Ordering; @@ -498,14 +497,23 @@ impl AnalyticsView { menu } - /// Visible-column selector (glyph "▦"); each item toggles a column bit, "All" toggles all. + /// Visible-column selector ([`design::COLUMN_SELECTOR_ICON`]); each item toggles a column bit, + /// "All" toggles all. /// The menu stays open across clicks; the name column is always shown, so hiding every /// toggleable column is allowed (unlike Orders, which locks the last one). + /// + /// The trigger carries no label, so it takes a tooltip like the four other pickers do — it + /// reuses `report.columns_menu`, the same borrowing this menu already does for + /// `report.filter.all`, rather than adding a fifth dictionary key that says "Columns". fn strat_column_menu(&self, cx: &Context) -> impl IntoElement + use<> { let view = cx.entity(); let cur = self.strat_cols(); let mut menu = MoonDropdown::new("an-strat-cols") - .segment(MoonButtonSegment::new("▦")) + // The shared column-selector asset; the choice and the childless trigger are + // `design::COLUMN_SELECTOR_ICON`'s contract. This one alone stays MICRO and keeps its + // own pill width: it sits in the tuner's compact strip, not in a chrome toolbar, so + // `glyph_btn_w` (the Action preset's 26px height) would not fit the row. + .trigger_icon(design::COLUMN_SELECTOR_ICON) .trigger_variant(MoonButtonVariant::Soft) .trigger_size(MoonButtonSize::Micro) .trigger_width_scaled(30.0) @@ -550,6 +558,12 @@ impl AnalyticsView { COL_BIT_LASTEDIT, t!("analytics.col.lastedit").to_string(), ); - menu + div() + .id("an-strat-cols-tip") + .tooltip(|_window, cx| { + cx.new(|_| moon_ui::MoonTooltipView::new(t!("report.columns_menu").to_string())) + .into() + }) + .child(menu) } } diff --git a/crates/moon-ui-gpui/src/analytics/tuner/strat_columns.rs b/crates/moon-ui-gpui/src/analytics/tuner/strat_columns.rs index 882147f5..d9df8b18 100644 --- a/crates/moon-ui-gpui/src/analytics/tuner/strat_columns.rs +++ b/crates/moon-ui-gpui/src/analytics/tuner/strat_columns.rs @@ -111,7 +111,7 @@ const STRAT_MIN_PANEL_W: f32 = 740.0; /// /// Identity (kind, core) plus the headline numbers (trades, profit, Profit %, Avg order, winrate, /// PF) — the set that fits beside a readable name. The tails (avg / best / worst) and the edit -/// stamp stay one click away in the ▦ selector rather than squeezing the name out of the row. +/// stamp stay one click away in the column selector rather than squeezing the name out of the row. /// `strat_columns_default_fits` holds this honest. pub(in crate::analytics) const STRAT_COLS_DEFAULT: u16 = COL_BIT_KIND | COL_BIT_CORE diff --git a/crates/moon-ui-gpui/src/chart_tabs/strip.rs b/crates/moon-ui-gpui/src/chart_tabs/strip.rs index b93b77b5..361f61c1 100644 --- a/crates/moon-ui-gpui/src/chart_tabs/strip.rs +++ b/crates/moon-ui-gpui/src/chart_tabs/strip.rs @@ -8,7 +8,7 @@ use gpui::prelude::FluentBuilder; use gpui::*; use moon_ui::{ MoonButton, MoonButtonIconSlot, MoonButtonSize, MoonButtonVariant, MoonInput, MoonPalette, - MoonTabItem, MoonTabStrip, h_flex, v_flex, + MoonTabItem, MoonTabStrip, h_flex, rgba_from, v_flex, }; use rust_i18n::t; @@ -422,11 +422,37 @@ impl Render for ChartTabs { .child( // Tabs yield (`flex_1 min_w_0`); the right chrome cluster is a real flex sibling, // not an overlay. This row does not clip: hanging coin/figstyle layers are lifted. + // + // The ROW paints the surface, not the strip alone. `MoonTabStrip`'s root fills + // `shell_high` across its OWN width, and since it became an in-flow `w_full` + // sibling of the cluster that width is only the `flex_1` slot — so behind the + // toolbar the unpainted ancestors showed through, which in the dark theme reads as + // a black band around the figure combo and the gear. The same token here, once, on + // the container both of them sit in, so the seam is invisible in every theme. h_flex() .h(px(strip_h)) .w_full() .min_w_0() + .relative() .items_center() + .bg(rgb(p_strip.shell_high)) + // ...and the same for the hairline that closes the row against the chart: + // `MoonTabStrip` draws its own, `w_full` of ITS width, so it stopped at the + // seam too. Continued here in the strip's own idiom and values. It is the + // FIRST child on purpose — a `Div` paints its background, then its children in + // order, so the strip's opaque fill covers this one on the left and redraws it + // identically, while the cluster (no background of its own, and every widget in + // it centred well above the last pixel) simply lets it through on the right. + // Nothing stacks, and no alpha is drawn twice. + .child( + div() + .absolute() + .left(px(0.0)) + .bottom(px(0.0)) + .w_full() + .h(px(1.0)) + .bg(rgba_from(p_strip.border, 0.78)), + ) .child(div().flex_1().min_w_0().h_full().child(strip)) .child(right_cluster), ) diff --git a/crates/moon-ui-gpui/src/design.rs b/crates/moon-ui-gpui/src/design.rs index 0e24209f..f2cdadd9 100644 --- a/crates/moon-ui-gpui/src/design.rs +++ b/crates/moon-ui-gpui/src/design.rs @@ -113,8 +113,27 @@ pub fn chrome_section(cx: &App) -> Div { .gap(ui_px(cx, CHROME_GAP)) } -/// Rendered width that makes a glyph button SQUARE — the column selector (`▦`) and the report -/// export (`⇩`). +/// Icon standing for "which columns does this table show", on every column selector in the app. +/// +/// All six pickers — Orders, Figures, Assets, the Screener, the Analytics tuner list and the +/// Report toolbar — draw THIS asset, for the reason [`CORE_COMPACT_ICON`] gives for the compact +/// core selector: the trigger carries no label, so the icon IS the name, and a second glyph for +/// the same concept would read as a different control. +/// +/// It replaced `▦` (U+25A6), which is absent from the default Windows font stack and rendered as +/// two hollow squares. The embedded MoonUI set carries no `filter`, `columns` or `sliders` icon; +/// `layout-dashboard` is its nearest reading — a table's own layout is exactly what these menus +/// change — and it is the only candidate that does NOT collide with `icons/settings-2.svg`, which +/// the Orders toolbar already draws on the sort/settings dropdown standing right beside its +/// column picker. `eye.svg` was rejected: it reads as row visibility, not column layout. +/// +/// The trigger is left CHILDLESS at every site so `MoonButton` takes its square icon-only layout; +/// [`glyph_btn_w`] then keeps the cell square on the UI slider while `MoonButton::render` sizes +/// the icon from the size preset's own font metrics, on the Font slider. +pub const COLUMN_SELECTOR_ICON: &str = "icons/layout-dashboard.svg"; + +/// Rendered width that makes a SQUARE one-symbol button — the report export (`⇩`) and the +/// Action-sized column selectors, which draw [`COLUMN_SELECTOR_ICON`] rather than a glyph. /// /// It returns the button's own drawn height, so the caller must pass it to a RENDERED width /// (`MoonDropdown::trigger_width`, `MoonButton::width`), never to a `*_scaled` variant: MoonUI diff --git a/crates/moon-ui-gpui/src/panels/alerts/controls.rs b/crates/moon-ui-gpui/src/panels/alerts/controls.rs index 4925ace4..fe93b4d6 100644 --- a/crates/moon-ui-gpui/src/panels/alerts/controls.rs +++ b/crates/moon-ui-gpui/src/panels/alerts/controls.rs @@ -198,7 +198,7 @@ impl AlertsPanel { .items(items) } - /// Builds the persisted visible-column menu, matching the other tables' glyph selector. + /// Builds the persisted visible-column menu, matching the other tables' icon selector. /// /// `close_on_select(false)` keeps the menu open for several edits. The action column is not /// offered at all — it holds the only delete and settings buttons — and the last remaining @@ -207,7 +207,9 @@ impl AlertsPanel { let view = cx.entity(); let cur = self.view; let mut menu = MoonDropdown::new("alerts-columns") - .segment(moon_ui::MoonButtonSegment::new("▦")) + // The shared column-selector asset; the choice and the childless trigger are + // `design::COLUMN_SELECTOR_ICON`'s contract. + .trigger_icon(design::COLUMN_SELECTOR_ICON) .trigger_variant(MoonButtonVariant::Soft) .trigger_size(MoonButtonSize::Action) .trigger_width(design::glyph_btn_w(cx)) diff --git a/crates/moon-ui-gpui/src/panels/assets/balances.rs b/crates/moon-ui-gpui/src/panels/assets/balances.rs index 2e7ed425..c1e1d8c1 100644 --- a/crates/moon-ui-gpui/src/panels/assets/balances.rs +++ b/crates/moon-ui-gpui/src/panels/assets/balances.rs @@ -149,6 +149,32 @@ pub(super) fn figure(a: Option<&CoreAgg>, p: MoonPalette, cx: &App) -> impl Into }) } +/// Rendered width of the figure [`figure`] draws for `a`, in font-scaled pixels. +/// +/// Lives here rather than at the caller so the MEASURED string stays the string this module +/// actually renders: `agg_text` and `state_marker` are private, and a hand-copied format in the +/// roster's width arithmetic would drift silently the day either of them changes — the same +/// argument `analytics::tuner::list::table::core_col_w` makes for measuring `core_label(g)` +/// rather than the raw name. +/// +/// Mirrors [`figure`]'s own composition exactly: the amount at body size, plus — only when a +/// trust marker is drawn — the row's gap and that marker at caption size. +/// +/// Args: +/// a: Core aggregate the figure is drawn for, or `None` for the dash. +/// cx: Application context used to measure text. +/// +/// Returns: +/// The figure's width in rendered pixels, already font-scaled. +pub(super) fn figure_width(a: Option<&CoreAgg>, cx: &App) -> f32 { + let mut width = design::mono_body_text_width(cx, &agg_text(a), FontWeight::NORMAL.0); + if let Some(marker) = state_marker(a) { + width += f32::from(design::ui_px(cx, 4.0)) + + design::mono_caption_text_width(cx, &marker, FontWeight::NORMAL.0); + } + width +} + /// Scope total with trust metadata. /// /// Cores without a usable figure are NOT summed: their balance is unknown, and a silent zero diff --git a/crates/moon-ui-gpui/src/panels/assets/cache.rs b/crates/moon-ui-gpui/src/panels/assets/cache.rs index f84c9287..a61e6e88 100644 --- a/crates/moon-ui-gpui/src/panels/assets/cache.rs +++ b/crates/moon-ui-gpui/src/panels/assets/cache.rs @@ -73,6 +73,10 @@ impl AssetsView { self.sort_entries(&mut entries); self.cached_entries = Rc::new(entries); self.cached_aggs = Rc::new(self.per_core(b)); + // The roster's measured width is a property of these aggregates, so it dies with them. + // Clearing rather than recomputing: measuring needs an `App` this method does not have, + // and the first `render` that actually shows the wallets section refills it. + self.cached_roster_auto_w = None; self.cached_all_futures = self.all_scope_cores_futures(b); self.rebuild_wallet_cache(b); // Skip non-finite row values so one bad price cannot turn the whole Σ into `NaN`, but diff --git a/crates/moon-ui-gpui/src/panels/assets/columns.rs b/crates/moon-ui-gpui/src/panels/assets/columns.rs index c620f060..6f6b2223 100644 --- a/crates/moon-ui-gpui/src/panels/assets/columns.rs +++ b/crates/moon-ui-gpui/src/panels/assets/columns.rs @@ -429,8 +429,9 @@ impl AssetsView { let all_on = self.hidden_cols.is_empty(); let all_view = view.clone(); let mut menu = MoonDropdown::new("assets-columns") - // The shared glyph trigger used by every column selector (Orders, Report, Screener). - .segment(moon_ui::MoonButtonSegment::new("▦")) + // The shared icon trigger used by every column selector (Orders, Report, Screener); + // the choice and the childless trigger are `design::COLUMN_SELECTOR_ICON`'s contract. + .trigger_icon(design::COLUMN_SELECTOR_ICON) .trigger_variant(MoonButtonVariant::Soft) .trigger_size(MoonButtonSize::Action) .trigger_width(design::glyph_btn_w(cx)) diff --git a/crates/moon-ui-gpui/src/panels/assets/mod.rs b/crates/moon-ui-gpui/src/panels/assets/mod.rs index 1e6b821e..d3b3460a 100644 --- a/crates/moon-ui-gpui/src/panels/assets/mod.rs +++ b/crates/moon-ui-gpui/src/panels/assets/mod.rs @@ -125,6 +125,19 @@ pub struct AssetsView { pub(super) sell_marked: Rc>, /// Per-core balance figures and their trust classifications for the current scope. cached_aggs: Rc>, + /// Content-measured BASE width of the wallets roster column ([`roster_width::auto_base`]), + /// paired with the presentation it was measured under, or `None` while it needs re-measuring. + /// + /// Retained rather than measured per frame: the widest row is found by measuring every core + /// name and figure, and `design::ui_text_width` resolves a font and walks every character on + /// each call — a 200-core roster would pay that at repaint rate. + /// + /// TWO things invalidate it, and both are needed. `rebuild_cache` clears the slot when + /// `cached_aggs`, the aggregates it is derived from, are replaced. The stored + /// [`table::RosterWidthEnv`] covers the other half — a live language or scale change moves + /// the correct width while touching no data revision at all, and on a quiet account the next + /// data change may never come. + cached_roster_auto_w: Option<(f32, table::RosterWidthEnv)>, /// Every in-scope core (after the filter) is a futures core. An empty table then means "no /// open positions" rather than "no assets": futures balances are quote-denominated and never /// reach the table. Computed in `rebuild_cache` to keep the store out of `render`. @@ -347,6 +360,7 @@ impl AssetsView { cached_entries: Rc::new(Vec::new()), sell_marked: Rc::new(std::collections::HashSet::new()), cached_aggs: Rc::new(Vec::new()), + cached_roster_auto_w: None, cached_all_futures: false, cached_wallet_key: None, cached_wallets: Rc::new(Vec::new()), diff --git a/crates/moon-ui-gpui/src/panels/assets/render.rs b/crates/moon-ui-gpui/src/panels/assets/render.rs index 07eec366..98c8e671 100644 --- a/crates/moon-ui-gpui/src/panels/assets/render.rs +++ b/crates/moon-ui-gpui/src/panels/assets/render.rs @@ -102,6 +102,14 @@ impl Render for AssetsView { let p = MoonPalette::active(cx); let windowed = self.windowed; let show_wallets = self.wallets_visible(cx); + // The roster's content width is measured HERE and nowhere else: this is the only place + // holding both `&mut self` and an `App`, while `rebuild_cache`, which invalidates it, has + // no context to measure with. Behind the same flag as the section that reads it, so a + // table-only Classic tab pays nothing — and BEFORE the first builder below, each of which + // borrows `self` for the rest of this function. + if show_wallets { + self.ensure_roster_auto_w(cx); + } let count = entries.len(); // Natural table height is its header plus rows, or zero when empty. This lets the table grow diff --git a/crates/moon-ui-gpui/src/panels/assets/roster_width.rs b/crates/moon-ui-gpui/src/panels/assets/roster_width.rs index 39f61eb5..136079ec 100644 --- a/crates/moon-ui-gpui/src/panels/assets/roster_width.rs +++ b/crates/moon-ui-gpui/src/panels/assets/roster_width.rs @@ -32,23 +32,74 @@ pub(super) const MIN_BASE_W: f32 = 203.1; /// `core_status::by_ip_widths::MAX_COL_W`). pub(super) const MAX_BASE_W: f32 = 720.0; -/// Resolve the roster's BASE width: the default, overridden by a finite stored value clamped to +/// Resolve the roster's BASE width: `auto`, overridden by a finite stored value clamped to /// `[MIN_BASE_W, MAX_BASE_W]`. /// -/// A non-finite stored value is SKIPPED rather than clamped — `f32::clamp` panics on NaN, and -/// `layout.toml` is hand-editable untrusted input rather than something only the drag handler -/// ever writes (precedent: `core_status::by_ip_widths::ByIpWidths::resolved`). +/// A non-finite stored value is SKIPPED rather than clamped: `f32::clamp` does NOT reject a NaN +/// `self` — it returns the NaN unchanged (it panics only on a NaN or inverted BOUND) — so +/// clamping would launder a corrupt entry into `design::font_w_px` and on into GPUI layout as a +/// NaN width. The guard is what stops that, not the clamp. `layout.toml` is hand-editable +/// untrusted input rather than something only the drag handler ever writes (precedent: +/// `core_status::by_ip_widths::ByIpWidths::resolved`). +/// +/// The user's drag still WINS over the content measurement, and the double-click that removes the +/// entry drops the column back onto `auto` — which is what makes the content width the "default" +/// in the sense that gesture already promises, rather than a second, competing mechanism. /// /// Args: /// user: Stored per-column widths, keyed by [`WIDTH_KEY`]. +/// auto: Content-measured width from [`auto_base`], used when nothing is stored. /// /// Returns: -/// The default width, or the clamped stored override. -pub(super) fn resolved(user: &HashMap) -> f32 { +/// The auto width, or the clamped stored override. +pub(super) fn resolved(user: &HashMap, auto: f32) -> f32 { match user.get(WIDTH_KEY) { Some(&value) if value.is_finite() => value.clamp(MIN_BASE_W, MAX_BASE_W), - _ => DEFAULT_BASE_W, + _ => auto, + } +} + +/// BASE width the roster needs to draw its widest core name in full beside the free/total pair. +/// +/// Content-measured because a core name is the user's own free text: `AWS$22 ~ F-BN / SHOT_FUT +/// (SUB_09)` and `VLTR$18 ~ F-BN / SUB ACC No 38 L` are real names, and against the fixed +/// [`DEFAULT_BASE_W`] every one of them ellipsized at the sub-account — the part that says WHICH +/// row this is — while the three `flex_1` wallet columns beside it sat near-empty. Same shape as +/// `analytics::tuner::list::table::core_col_w`, for the same reason: any fixed width is wrong for +/// somebody's names. +/// +/// The FLOOR is [`DEFAULT_BASE_W`] rather than [`MIN_BASE_W`]: a short-named roster keeps exactly +/// the width it ships with today, so this widens the column and never narrows it. The CEILING is +/// [`MAX_BASE_W`], the same cap the drag honours — past it a pathological name would starve the +/// wallet columns, and the name truncates with its hover tooltip instead. Between them the column +/// stays draggable and the drag still overrides ([`resolved`]). +/// +/// The input is the widest ROW — one core's name plus its own figure — not the widest name plus +/// the widest figure: those two maxima usually belong to different cores, and a width reserving +/// both at once would starve the wallet columns for a row that does not exist. The caller owns +/// that measurement; this function owns the units and the bounds. +/// +/// Args: +/// widest_row_px: Widest name-plus-figure pair on any one row, in font-scaled pixels. +/// chrome_px: Everything else on the row — its padding and the cell gap — in font-scaled +/// pixels. +/// scale: Current font-width scale ([`crate::design::font_scale`]). +/// +/// Returns: +/// [`DEFAULT_BASE_W`] when `scale` or the measured total is not usable; otherwise the ceiled +/// base width clamped to `[DEFAULT_BASE_W, MAX_BASE_W]`. +pub(super) fn auto_base(widest_row_px: f32, chrome_px: f32, scale: f32) -> f32 { + if !scale.is_finite() || scale <= 0.0 { + return DEFAULT_BASE_W; + } + // Back to BASE units, because the caller renders through `design::font_w_px`, which scales + // again — the same round trip `core_col_w` documents. Ceil so a fractional shortfall cannot + // ellipsize the very name the column was sized for. + let needed = ((widest_row_px + chrome_px) / scale).ceil(); + if !needed.is_finite() { + return DEFAULT_BASE_W; } + needed.clamp(DEFAULT_BASE_W, MAX_BASE_W) } /// Compute the roster's next BASE width from a live divider drag. diff --git a/crates/moon-ui-gpui/src/panels/assets/table.rs b/crates/moon-ui-gpui/src/panels/assets/table.rs index 410a9326..4ef59826 100644 --- a/crates/moon-ui-gpui/src/panels/assets/table.rs +++ b/crates/moon-ui-gpui/src/panels/assets/table.rs @@ -98,6 +98,31 @@ pub(super) struct RosterDragAnchor { pub(super) base_w: f32, } +/// Presentation inputs the roster's measured width depends on, beside the aggregates themselves. +/// +/// Mirrors `panels::report::widths`'s `NaturalWidthsEnvironment`, which exists for exactly this +/// reason: a content-measured width is valid only for the typography and the LANGUAGE it was +/// measured in, and neither of those bumps a DATA revision — `assets_sig` hashes core ids and +/// their `*_rev` counters and nothing else. Settings applies a language or scale change LIVE to +/// open windows, so keying the width on `cached_aggs` alone would leave the column sized for the +/// previous locale until some balance happened to move, which on a quiet account is never. +/// +/// The localized part is real rather than theoretical: `balances::figure_width` measures the +/// trust markers `assets.balance_stale` / `assets.balance_unpriced`, whose length differs per +/// language. +#[derive(Clone, PartialEq)] +pub(super) struct RosterWidthEnv { + /// Rendered body text size, which moves with the theme base and the Font slider. Covers the + /// caption size too — it is derived from the same base. + body_bits: u32, + /// UI geometry scale, which moves the row's padding and the figure's internal gap. + ui_bits: u32, + /// Font-width scale: the divisor that turns a rendered measurement back into base units. + scale_bits: u32, + /// Active locale, because the measured trust markers are localized. + locale: String, +} + /// GPUI drag payload identifying the panel whose roster divider is being dragged. Carries no /// visual. /// @@ -350,6 +375,17 @@ impl AssetsView { )) } + /// Unscaled horizontal padding on each side of a roster row. SHARED by the row that draws it + /// and by [`AssetsView::ensure_roster_auto_w`], which sizes the column around it — the two + /// numbers must be the same number, not two copies free to drift. + const ROSTER_ROW_PAD: f32 = 8.0; + + /// Rendered gap between a roster row's name cell and its figure — the `gap_2` that + /// [`AssetsView::wallet_core_row`] applies, restated so the width arithmetic can account for + /// it. Chrome, not text: `gap_2` is a fixed rem step and is deliberately NOT run through the + /// Font slider. + const ROSTER_ROW_GAP_PX: f32 = 8.0; + /// Build one selectable core row of the wallet section's left list. /// /// Shared by both shapes of that list — grouped under exchange headings and flat — so the two @@ -378,16 +414,35 @@ impl AssetsView { .id(SharedString::from(format!("asset-core-{cid}"))) .w_full() .h(design::fit_h_px(cx, 24.0, 13.0, 5.0)) - .px(design::ui_px(cx, 8.0)) + .px(design::ui_px(cx, Self::ROSTER_ROW_PAD)) .items_center() .justify_between() .gap_2() .cursor_pointer() .text_color(rgb(p.text)) - .child(div().flex_1().min_w_0().truncate().child(core_name)) + // The name yields FIRST and the figure never does: the roster is sized to show both + // (`roster_width::auto_base`), but a user drag or a genuinely narrow dock can still + // undercut that, and a half-drawn balance is worse than a shortened name — the name + // survives on hover, a clipped number reads as a different number. Hence `truncate` + // here and `flex_none` on the figure below. + .child( + div() + .id(SharedString::from(format!("asset-core-name-{cid}"))) + .flex_1() + .min_w_0() + .truncate() + // No `occlude`, unlike the resize handle: the click belongs to the row, and + // this cell only needs a hover target for the tooltip. + .tooltip(crate::panels::common::text_tooltip(core_name.clone())) + .child(core_name), + ) // Per-core trust, rendered by the module that owns the vocabulary — so a core // shown as current here cannot be one the footer total counts as stale. - .child(super::balances::figure(Some(agg), p, cx)) + .child( + div() + .flex_none() + .child(super::balances::figure(Some(agg), p, cx)), + ) .on_click(cx.listener(move |this, _, window, cx| { if let AssetsScope::Group(_) = &this.scope { if let Some(scope) = this.effective_scope(this.backend.read(cx)) { @@ -506,6 +561,60 @@ impl AssetsView { }); } + /// Refill [`AssetsView::cached_roster_auto_w`] when the aggregates it is derived from moved. + /// + /// Measures the WIDEST ROW rather than the widest name plus the widest figure: the two cells + /// belong to one aggregate, and the longest name rarely sits on the row carrying the longest + /// balance — combining maxima that never coexist would reserve width no row asks for and + /// starve all three wallet columns for nothing. Every row is measured, not only distinct + /// names, because the figure differs per core even where the name repeats. + /// + /// Called from `render`, which is the only place holding both `&mut self` and an `App`; + /// `rebuild_cache` owns the invalidation but has no context to measure with. + /// + /// Args: + /// cx: Application context used to measure text and read the active font scale. + /// + /// Returns: + /// Nothing; a slot that is already filled is left alone. + pub(super) fn ensure_roster_auto_w(&mut self, cx: &App) { + // Re-measure when the aggregates moved (`rebuild_cache` cleared the slot) OR when the + // presentation the measurement was taken under moved — see [`RosterWidthEnv`]. Both are + // needed: a language switch changes the width without touching the data, and new data + // changes it without touching the language. + let env = RosterWidthEnv { + body_bits: f32::from(design::t_body(cx)).to_bits(), + ui_bits: f32::from(design::ui_px(cx, 1.0)).to_bits(), + scale_bits: design::font_scale(cx).to_bits(), + locale: rust_i18n::locale().to_string(), + }; + if self + .cached_roster_auto_w + .as_ref() + .is_some_and(|(_, measured_under)| *measured_under == env) + { + return; + } + // Everything on the row that is neither the name nor the figure: its horizontal padding, + // twice, and the `gap_2` between the two cells. The resize strip is deliberately NOT + // added — it is absolutely positioned over the column's right edge, so it consumes no + // flex space and already sits inside that right padding. + let chrome = + 2.0 * f32::from(design::ui_px(cx, Self::ROSTER_ROW_PAD)) + Self::ROSTER_ROW_GAP_PX; + let widest_row = self + .cached_aggs + .iter() + .map(|agg| { + design::mono_body_text_width(cx, &agg.name, FontWeight::NORMAL.0) + + super::balances::figure_width(Some(agg), cx) + }) + .fold(0.0f32, f32::max); + self.cached_roster_auto_w = Some(( + roster_width::auto_base(widest_row, chrome, design::font_scale(cx)), + env, + )); + } + /// Collapsible Wallets section: a selectable balance-aware core list plus the Spot, /// Futures, and Quarterly transfer containers. Expanded content shares the available height. /// @@ -675,7 +784,23 @@ impl AssetsView { // unscaled units — see that module) rather than as the fixed law above: the column tracks // the Font slider like every other metric in the panel, and the user can drag it wider // himself via the resize handle at its right edge, with the result persisted. - let base_w = roster_width::resolved(&self.roster_widths.read(cx).column_widths); + // + // And the DEFAULT is now measured rather than fixed. 420 px was chosen against the names + // above, but a name is the user's own free text: at the shipped width every row read + // `AWS$22 ~ F-BN / SHOT_FUT (SU…`, ellipsized exactly at the sub-account that says which + // row it is, while the three wallet columns beside it held one short entry each. The + // column is sized to its widest name plus the figure it must not overlap, floored at the + // shipped width and capped at `MAX_BASE_W` (`roster_width::auto_base`); a stored drag + // still wins, and double-click drops back onto this measurement. + // + // Measured once per aggregate rebuild by `ensure_roster_auto_w`, which `render` calls + // before it reaches this section; an empty slot here means the section is being drawn + // outside that path, and the shipped default is the honest answer rather than a guess. + let auto_w = self + .cached_roster_auto_w + .as_ref() + .map_or(roster_width::DEFAULT_BASE_W, |(width, _)| *width); + let base_w = roster_width::resolved(&self.roster_widths.read(cx).column_widths, auto_w); let left = v_flex() .relative() .w(design::font_w_px(cx, base_w)) diff --git a/crates/moon-ui-gpui/src/panels/assets/tests.rs b/crates/moon-ui-gpui/src/panels/assets/tests.rs index 67578a85..2f2d0dee 100644 --- a/crates/moon-ui-gpui/src/panels/assets/tests.rs +++ b/crates/moon-ui-gpui/src/panels/assets/tests.rs @@ -118,7 +118,8 @@ fn group_assets_shortcuts_cannot_bypass_the_auto_rail() { /// the roster would render near 496 px instead of 420 px, silently taking width from all wallets. #[test] fn roster_default_renders_at_the_shipped_width_scale() { - let rendered = roster_width::resolved(&HashMap::new()) * (13.0 / 11.0); + let rendered = + roster_width::resolved(&HashMap::new(), roster_width::DEFAULT_BASE_W) * (13.0 / 11.0); assert!( (rendered - 420.0).abs() < 0.05, @@ -126,6 +127,88 @@ fn roster_default_renders_at_the_shipped_width_scale() { ); } +/// `roster_width.rs::auto_base` must convert rendered row pixels back to base-width units. +/// +/// Mutation: drop `/ scale` before `ceil`. At a raised Font slider the roster would be scaled +/// twice, consuming the wallet columns even though the measured row already fits exactly once. +#[test] +fn roster_auto_width_round_trips_rendered_pixels_through_font_scale() { + let base = roster_width::auto_base(600.0, 20.0, 1.25); + + assert_eq!(base, 496.0); + assert_eq!(base * 1.25, 620.0); +} + +/// `roster_width.rs::auto_base` must retain the shipped rendered floor for short core names. +/// +/// Mutation: clamp the automatic result to `MIN_BASE_W` instead of `DEFAULT_BASE_W`. A fresh +/// install with short names would narrow the roster below the historical 420-pixel width. +#[test] +fn roster_auto_width_keeps_the_shipped_floor_for_short_rows() { + let rendered = roster_width::auto_base(20.0, 20.0, 13.0 / 11.0) * (13.0 / 11.0); + + assert!( + (rendered - 420.0).abs() < 0.05, + "short rows must retain the shipped 420 px roster width, got {rendered}" + ); +} + +/// `roster_width.rs::auto_base` must cap one pathological name before it starves wallet columns. +/// +/// Mutation: drop the upper clamp. A single very long core name would take nearly all horizontal +/// space and make the Spot, Futures, and Quarterly figures unreadable. +#[test] +fn roster_auto_width_caps_pathological_rows_before_the_wallet_columns_starve() { + let rendered = roster_width::auto_base(2_000.0, 20.0, 13.0 / 11.0) * (13.0 / 11.0); + + assert!( + rendered <= 852.0, + "the roster cap must leave room for the three wallet columns, got {rendered} px" + ); +} + +/// `roster_width.rs::auto_base` must reject unusable scale and measurement inputs. +/// +/// Mutation: remove the non-finite guards. A hand-edited Font scale or invalid text measurement +/// would reach `f32::clamp`, panic on NaN, or replace the normal roster width with a bogus value. +#[test] +fn roster_auto_width_rejects_non_finite_scale_and_measurements() { + for (case, widest_row_px, chrome_px, scale) in [ + ("zero scale", 600.0, 20.0, 0.0), + ("NaN scale", 600.0, 20.0, f32::NAN), + ("infinite measurement", f32::INFINITY, 20.0, 1.25), + ] { + assert_eq!( + roster_width::auto_base(widest_row_px, chrome_px, scale), + roster_width::DEFAULT_BASE_W, + "{case} must fall back to the shipped base width" + ); + } +} + +/// `roster_width.rs::resolved` must prefer a finite user drag and skip a NaN one. +/// +/// Mutation: let `auto` win before the stored width, or clamp a NaN value. A user's resized roster +/// would be overridden on every rebuild, or a hand-edited layout would panic instead of using auto. +#[test] +fn roster_resolved_preserves_finite_user_widths_and_skips_nan() { + let auto = 460.0; + let mut widths = HashMap::from([(roster_width::WIDTH_KEY.to_string(), 1_000.0)]); + + assert_eq!( + roster_width::resolved(&widths, auto), + roster_width::MAX_BASE_W, + "a finite stored drag must win and still honour the upper drag boundary" + ); + + widths.insert(roster_width::WIDTH_KEY.to_string(), f32::NAN); + assert_eq!( + roster_width::resolved(&widths, auto), + auto, + "a NaN layout value must be ignored in favour of the measured roster width" + ); +} + /// `roster_width.rs::dragged` must convert pointer pixels back into persisted base-width units. /// /// Mutation: drop the division by `scale`. At a raised Font slider the divider would trail the diff --git a/crates/moon-ui-gpui/src/panels/detects/mod.rs b/crates/moon-ui-gpui/src/panels/detects/mod.rs index 7f14d945..8d65ce3a 100644 --- a/crates/moon-ui-gpui/src/panels/detects/mod.rs +++ b/crates/moon-ui-gpui/src/panels/detects/mod.rs @@ -27,6 +27,9 @@ use moon_core::session::CoreId; use moon_ui::{ MoonPalette, MoonSliderEvent, MoonSliderState, Panel, PanelEvent, PanelState, h_flex, v_flex, }; +use rust_i18n::t; + +use crate::workspace::scope_marker::{self, ScopeMarker}; /// Number of latest five-minute OHLC buckets retained for a card's candle chart, approximately two /// hours. At a typical 75-130 px chart width, 24 buckets leave roughly 3-5 px per outlined candle; @@ -416,9 +419,9 @@ impl DetectsPanel { } /// Arms a one-second refresh timer while the queue is nonempty. On wake it clears the armed - /// flag, prunes expired cards against wall-clock time, notifies if cards remain so their - /// countdowns update, and rearms itself. If pruning empties the queue, the timer stops and this - /// callback does not issue a final notification. + /// flag, prunes expired cards against wall-clock time, notifies so their countdowns update, + /// and rearms itself. Pruning to zero stops the timer but still issues its FINAL + /// notification: that render is the one that replaces the last card with the empty state. fn arm_prune_timer(&mut self, cx: &mut Context) { if self.prune_timer_armed || self.items.is_empty() { return; @@ -430,9 +433,13 @@ impl DetectsPanel { let alive = cx.update(|cx| { this.update(cx, |this, cx| { this.prune_timer_armed = false; - this.prune(now_unix_ms()); - // Refresh countdowns and reflect removals while at least one card remains. - if !this.items.is_empty() { + let pruned = this.prune(now_unix_ms()); + // Refresh countdowns while cards remain, and paint the transition to empty + // exactly once. Gating this on a non-empty queue alone left the LAST expired + // card painted until some unrelated notification arrived — harmless while an + // empty feed drew nothing, but it is now the render that puts the + // empty-state sentence on screen. + if pruned || !this.items.is_empty() { cx.notify(); } this.arm_prune_timer(cx); @@ -510,6 +517,53 @@ impl DetectsPanel { } } +/// Pick the sentence an EMPTY detection feed states, in the house precedence. +/// +/// A feed with no visible card is several different facts, and they must not share a string: +/// only the no-cores one asks the user to go and connect something. +/// +/// Two orderings here are load-bearing, and both were wrong in this function's first draft. +/// +/// **The empty UNIVERSE is checked first.** A card outlives the session that produced it — it +/// stays in the queue for its whole `KeepAlert` and [`Self::ingest`] never evicts one whose core +/// disappeared — so a disconnect mid-`KeepAlert` leaves cards retained, nothing visible, and no +/// core available. Asking about the retained cards first would blame the scope for hiding +/// detects when the cores behind them are simply gone. The PARTIAL version of that state — one +/// core of several goes away while its siblings sit idle — is settled by the caller instead, +/// which counts only cards whose core is still available (see `retained` below); the count and +/// this ordering are two halves of one rule. `available` counts +/// `WorkspaceCoreAvailability::is_available` (`workspace.rs:251`), which is group and core +/// activation plus a live session and a live window — hence "available", never "connected". +/// +/// **The shared hidden-by-preset sentence applies ONLY when data really was excluded.** +/// [`scope_marker::scope_empty_text`] switches on membership alone, and its contract is that the +/// data exists and the preset is what withholds it. A Detects feed can be empty with a full +/// scope-hiding preset simply because nothing ever fired, and there the shared sentence would +/// send the user to widen a preset that is hiding nothing. So it is reached only under +/// `retained > 0`. +/// +/// Args: +/// marker: This group's scope marker, built from the same membership counts presentation +/// filters on. +/// retained: Cards this panel holds whose core is STILL AVAILABLE — the ones a preset change +/// could actually bring back. Nonzero with nothing visible means detects DID arrive and +/// presentation is what hides them. A card whose core has gone away is deliberately NOT +/// counted: it is unreachable rather than hidden, and no scope change reveals it. +/// available: Group cores that survived availability — the universe the feed could ever +/// show. Zero means nothing here can detect at all. +/// +/// Returns: +/// One localized sentence, already resolved; never empty. +fn empty_feed_text(marker: &ScopeMarker, retained: usize, available: usize) -> String { + if available == 0 { + t!("detects.empty_no_cores").to_string() + } else if retained > 0 { + scope_marker::scope_empty_text(Some(marker), t!("detects.empty_filtered").to_string()) + } else { + t!("detects.empty").to_string() + } +} + /// Signature that makes the panel re-ingest: a hash of every group core's detect revision, PLUS the /// AddToChart setting as its own value. The setting belongs here because flipping it changes which /// rows the feed accepts, and this is what wakes EVERY panel of the group — the one whose checkbox @@ -560,7 +614,8 @@ impl Render for DetectsPanel { /// cx: Panel context providing workspace scope, theme, and configuration. /// /// Returns: - /// Toolbar and filtered card grid without discarding out-of-scope retained cards. + /// Toolbar and filtered card grid without discarding out-of-scope retained cards, or the + /// toolbar above one centred sentence naming why the feed is empty. fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { crate::diag::bump(&crate::diag::DETECTS_RENDER); let _render_us = crate::diag::scope(&crate::diag::DETECTS_RENDER_US); @@ -574,12 +629,35 @@ impl Render for DetectsPanel { // The chart theme supplies colors for card candle and line vectors. let theme = self.backend.read(cx).config.chart_theme().clone(); let now = now_unix_ms(); - let visible_cores = self - .backend - .read(cx) - .effective_workspace_scope(&self.group, crate::workspace::RetainedCoreScope::All) - .ids() - .to_vec(); + // The scope value is kept, not consumed straight into its ids: its membership counts are + // what tells an empty feed whether a preset is hiding a full one. + let (marker, visible_cores, available_cores, retained_reachable) = { + let b = self.backend.read(cx); + let scope = + b.effective_workspace_scope(&self.group, crate::workspace::RetainedCoreScope::All); + let available = scope.membership_total(); + let marker = ScopeMarker::new( + b.display_preset(crate::workspace::DisplayOwner::Group(&self.group)), + scope.membership_shown(), + available, + ); + // Count only the retained cards a preset change could actually bring BACK. A card + // whose core has gone unavailable is unreachable, not hidden: deactivating a server + // makes `SessionManager::reconcile` drop the core outright + // (`moon-core/src/session/lifecycle.rs:304`), and nothing evicts the card it left + // behind until its `KeepAlert` expires. Counting it as retained would tell a + // multi-core group whose other cores are merely idle that the scope is hiding + // detects, and send the user widening a preset that can never reveal them. + let retained_reachable = self + .items + .iter() + .filter(|it| { + b.workspace_core_availability(&self.group, it.core) + .is_available() + }) + .count(); + (marker, scope.ids().to_vec(), available, retained_reachable) + }; // Place the gear-triggered configuration toolbar and divider above the feed. let toolbar = popup::toolbar(self, &cfg, p, cx); @@ -588,6 +666,7 @@ impl Render for DetectsPanel { // Render fixed-size cards in reverse insertion order in a wrapping grid. Newly inserted // markets appear first; a repeated core-market detection refreshes its existing position. let mut container = h_flex().flex_wrap().gap_1p5().content_start(); + let mut shown = 0usize; for it in self.items.iter().rev().filter(|item| { detection_core_visible(item.core, &visible_cores) && detection_route_visible(item.add_to_chart, cfg.show_add_to_chart) @@ -614,16 +693,47 @@ impl Render for DetectsPanel { }), ); container = container.child(card); + shown += 1; } - let scroll = div() - .id("detects-scroll") - .flex_1() - .w_full() - .min_h(px(0.0)) - .overflow_y_scroll() - .p_2() - .child(container); + // An empty feed states WHY it is empty instead of painting a blank pane under the gear. + // Same element the Log panel uses for `log.empty_filtered` (`panels/log/view.rs`): one + // centred, muted line filling the space the card grid would have taken. It replaces the + // scroll box rather than sitting inside it — an empty scroll container would still own the + // `flex_1` slot and leave the sentence pinned to the top-left corner. + let body: AnyElement = if shown == 0 { + div() + .flex_1() + .w_full() + .min_h(px(0.0)) + .flex() + .items_center() + .justify_center() + // `items_center` + `justify_center` centre the text BOX, not the lines inside it. + // The Log panel needs no more than that because its empty copy is two words; these + // sentences wrap at any realistic dock width, and without this the centred empty + // state reads left-aligned exactly where it is most cramped. + .text_center() + .p_2() + .text_size(crate::design::t_body(cx)) + .text_color(rgb(p.text_soft)) + .child(empty_feed_text( + &marker, + retained_reachable, + available_cores, + )) + .into_any_element() + } else { + div() + .id("detects-scroll") + .flex_1() + .w_full() + .min_h(px(0.0)) + .overflow_y_scroll() + .p_2() + .child(container) + .into_any_element() + }; v_flex() .id("detects") @@ -634,7 +744,7 @@ impl Render for DetectsPanel { .bg(rgb(p.table_body)) .child(toolbar) .child(divider) - .child(scroll) + .child(body) } } diff --git a/crates/moon-ui-gpui/src/panels/detects/tests.rs b/crates/moon-ui-gpui/src/panels/detects/tests.rs index 2a85e0fb..57780011 100644 --- a/crates/moon-ui-gpui/src/panels/detects/tests.rs +++ b/crates/moon-ui-gpui/src/panels/detects/tests.rs @@ -1,7 +1,10 @@ //! Regression tests for detection presentation scoping. use super::cards::{self, strategy_chip_text}; -use super::{detect_expired, detection_core_visible, detection_route_visible}; +use super::{detect_expired, detection_core_visible, detection_route_visible, empty_feed_text}; +use crate::workspace::scope_marker::ScopeMarker; +use moon_core::config::WorkspaceMode; +use rust_i18n::t; /// Body of `DetectsPanel::ingest`, the subject of the source-shape assertions below. Its closing /// brace is the first at method indentation. @@ -235,3 +238,42 @@ fn name_budget_follows_the_space_a_card_actually_has() { // Never below the floor, however narrow the card is configured. assert!(cards::side_name_w(1.0, true) >= 24.0); } + +/// `detects/mod.rs:empty_feed_text` must check `available == 0` before `retained > 0`. +/// +/// Mutation: swap the first two branches. A card retained across a disconnected core would claim a +/// preset hid it instead of explaining that no core in the group can currently detect. +#[test] +fn empty_feed_no_available_cores_outrank_retained_cards() { + let hidden_marker = ScopeMarker::new(Some(WorkspaceMode::Classic), 0, 3); + + assert_eq!( + empty_feed_text(&hidden_marker, 1, 0), + t!("detects.empty_no_cores") + ); +} + +/// `detects/mod.rs:empty_feed_text` must apply `scope_empty_text` only when cards are retained. +/// +/// Mutation: wrap all branches in `scope_empty_text`. An empty feed that never received a detect +/// would incorrectly say that the preset hid every core instead of saying no detects have fired. +#[test] +fn empty_feed_all_hidden_preset_does_not_rewrite_an_unretained_feed() { + let hidden_marker = ScopeMarker::new(Some(WorkspaceMode::Classic), 0, 3); + + assert_eq!(empty_feed_text(&hidden_marker, 0, 3), t!("detects.empty")); +} + +/// `detects/mod.rs:empty_feed_text` must preserve the filtered sentence for a partial preset. +/// +/// Mutation: replace the retained branch with `detects.empty`. A retained detect hidden by only +/// part of the preset would lose the explanation that the active scope is withholding it. +#[test] +fn empty_feed_partial_preset_with_retained_cards_reports_filtered_state() { + let partial_marker = ScopeMarker::new(Some(WorkspaceMode::Classic), 1, 3); + + assert_eq!( + empty_feed_text(&partial_marker, 1, 3), + t!("detects.empty_filtered") + ); +} diff --git a/crates/moon-ui-gpui/src/panels/orders/controls.rs b/crates/moon-ui-gpui/src/panels/orders/controls.rs index a63f1478..93912e57 100644 --- a/crates/moon-ui-gpui/src/panels/orders/controls.rs +++ b/crates/moon-ui-gpui/src/panels/orders/controls.rs @@ -142,8 +142,10 @@ impl OrdersPanel { let view = cx.entity(); let cur = self.view; let mut menu = MoonDropdown::new("orders-columns") - // Use a glyph button instead of a text field, matching the other column selectors. - .segment(moon_ui::MoonButtonSegment::new("▦")) + // An icon button instead of a text field, matching the other column selectors; the + // asset and the childless trigger are `design::COLUMN_SELECTOR_ICON`'s contract. It is + // deliberately NOT `sort_menu`'s gear below: the two sit side by side in this bar. + .trigger_icon(design::COLUMN_SELECTOR_ICON) .trigger_variant(MoonButtonVariant::Soft) .trigger_size(MoonButtonSize::Action) .trigger_width(design::glyph_btn_w(cx)) diff --git a/crates/moon-ui-gpui/src/panels/orders/render.rs b/crates/moon-ui-gpui/src/panels/orders/render.rs index 75c7a234..02513159 100644 --- a/crates/moon-ui-gpui/src/panels/orders/render.rs +++ b/crates/moon-ui-gpui/src/panels/orders/render.rs @@ -186,6 +186,18 @@ impl Render for OrdersPanel { emu = self.count_emu ) .to_string(); + // The same three numbers, spelled out with what each one counts. Built from the counts the + // row is about to render rather than from the rendered string, so the two cannot drift — + // the idiom `panels/report/totals.rs` uses for its own footer. It is unconditional: the + // head states the figures on every render, so there is never a state in which it has + // nothing to explain. + let head_tip = t!( + "orders.footer_total_tip", + total = total, + real = self.count_real, + emu = self.count_emu + ) + .to_string(); let footer_split = scope_marker::scope_footer(head, Some(&marker)); let footer_tip = scope_marker::scope_footer_tooltip(&footer_split, Some(&marker)); let footer = h_flex() @@ -197,12 +209,18 @@ impl Render for OrdersPanel { .py_1() .child( div() + // The id is what lets the head carry a tooltip at all — GPUI hangs one off an + // interactive element only. It changes nothing about the layout. + .id("orders-footer-head") // `flex_none` only while a tail exists to yield in its place. With nothing // hidden there is no tail, and pinning the head then would change how this row // behaves at a narrow width for a marker that is not on screen. .when(!footer_split.tail.is_empty(), |el| el.flex_none()) .text_size(design::t_body(cx)) .text_color(rgb(p.text_soft)) + .tooltip(crate::panels::common::text_tooltip(SharedString::from( + head_tip, + ))) .child(footer_split.head), ) .children(scope_marker::scope_footer_tail( diff --git a/crates/moon-ui-gpui/src/panels/report/columns.rs b/crates/moon-ui-gpui/src/panels/report/columns.rs index b62c81b0..98f3eed2 100644 --- a/crates/moon-ui-gpui/src/panels/report/columns.rs +++ b/crates/moon-ui-gpui/src/panels/report/columns.rs @@ -629,8 +629,9 @@ fn cell_tooltip(col: &str, text: &str) -> Option { /// Return the font weight a GENERIC Report data cell's text is drawn — and MEASURED — with. /// /// The two profit columns carry the number the table exists to be read for, so they are weighted -/// above the rest of the row. Their sibling `profitbtc`/`gainedbtc` share the same sign colouring -/// but not the weight: weighting every numeric column would flatten the emphasis back out. +/// above the rest of the row. Their sibling `profitbtc` shares the same sign colouring but not the +/// weight: weighting every numeric column would flatten the emphasis back out. `spentbtc` and +/// `gainedbtc` carry neither — they are amounts, not results. /// /// The coin column is deliberately not part of this rule. It renders through the specialized /// `coin_cell` element at `BOLD`; this predicate governs generic Report cells and the corresponding @@ -675,6 +676,21 @@ fn is_numeric_report_column(col: &str) -> bool { || col.ends_with("ratio") } +/// Decimals one money cell prints, resolved from the ROW's own quote currency. +/// +/// Stated once because two money arms read it: a second copy would let `spent` and `profit` drift +/// apart on the same row, which is exactly the mismatch this column's formatting exists to remove. +/// A row naming no quote falls back to two decimals, matching the totals row's own fallback. +/// +/// Args: +/// quote: The row's quote currency, or `None` when the schema carries none. +/// +/// Returns: +/// Fractional digit count for that row's money cells. +fn money_decimals(quote: Option) -> usize { + quote.map_or(2, |currency| currency.display_decimals()) +} + /// Formats a database value and optional text color for display in a report cell. /// /// Generic text values are trimmed and folded to one line. Report exports bypass this @@ -736,7 +752,20 @@ pub(super) fn cell( // `valuation_rate` deliberately has no arm: applied rates span roughly 1e5 (BTC) down to // 1e-2 (IDR), and the generic numeric path already answers that with eight significant // digits, where the two decimals the profit cells use would flatten most rates to `0.00`. - "profitbtc" | "gainedbtc" | "profitpct" | "valuation_profit_usdt" => { + // `spent` and `gained` are AMOUNTS, not results: they carry no sign and no profit/loss + // colour, but they are money and need the same per-quote precision the profit cells + // resolve. Without an arm of their own they fall through to the generic path, where + // `value_to_string` prints eight raw decimals — `98.9518728` beside `profit USDT`'s + // `+2.12`. No thousands grouping, because no other money cell in this table groups and one + // pretty column reads as an inconsistency rather than as polish. + "spentbtc" | "gainedbtc" => { + let decimals = money_decimals(quote); + let text = as_f64(v) + .map(|x| format!("{x:.decimals$}")) + .unwrap_or_default(); + (text, None) + } + "profitbtc" | "profitpct" | "valuation_profit_usdt" => { let n = as_f64(v); let color = match n { Some(x) if x > 0.0 => Some(p.green), @@ -751,7 +780,7 @@ pub(super) fn cell( // one is a ratio, the other is always USDT whatever the row is denominated in. let decimals = match col { "profitpct" | db::VALUATION_PROFIT_COLUMN => 2, - _ => quote.map_or(2, |currency| currency.display_decimals()), + _ => money_decimals(quote), }; let text = n .map(|x| { diff --git a/crates/moon-ui-gpui/src/panels/report/controls.rs b/crates/moon-ui-gpui/src/panels/report/controls.rs index 6394a90b..ea6332b5 100644 --- a/crates/moon-ui-gpui/src/panels/report/controls.rs +++ b/crates/moon-ui-gpui/src/panels/report/controls.rs @@ -976,8 +976,9 @@ impl ReportPanel { }) }), ); - // Use a glyph button instead of a list field, matching other column selectors. The - // tooltip is localized; glyphs remain outside the locale dictionary per locales/README. + // An icon button instead of a list field, matching every other column selector; the asset + // and the childless trigger are `design::COLUMN_SELECTOR_ICON`'s contract. The tooltip is + // localized; the icon stays outside the locale dictionary per locales/README. div() .id("rep-cols-tip") .tooltip(crate::panels::common::text_tooltip( @@ -985,7 +986,7 @@ impl ReportPanel { )) .child( MoonDropdown::new("rep-cols") - .segment(moon_ui::MoonButtonSegment::new("▦")) + .trigger_icon(design::COLUMN_SELECTOR_ICON) .trigger_variant(MoonButtonVariant::Soft) .trigger_size(MoonButtonSize::Action) .trigger_width(design::glyph_btn_w(cx)) diff --git a/crates/moon-ui-gpui/src/panels/report/totals.rs b/crates/moon-ui-gpui/src/panels/report/totals.rs index c20f0cab..fe4e0e20 100644 --- a/crates/moon-ui-gpui/src/panels/report/totals.rs +++ b/crates/moon-ui-gpui/src/panels/report/totals.rs @@ -121,6 +121,26 @@ struct VolumeGap { currencies: String, } +/// Format an unsigned amount at full quote precision, followed by its ticker. +/// +/// Shared by every footer fact that states an exact native amount in its tooltip — currently the +/// traded-volume amount here and the average order size in [`average_order_fact`] — so the two +/// can never round or ticker-format differently. +/// +/// Args: +/// amount: Unsigned amount in `currency`. +/// currency: Exact persisted quote identity. +/// +/// Returns: +/// Full quote-precision text followed by the ticker. +fn exact_native(amount: f64, currency: db::QuoteCurrency) -> String { + format!( + "{} {}", + fmt::compact(amount, currency.display_decimals()), + currency.ticker() + ) +} + /// Format an unsigned volume amount in compact and exact native quote forms. /// /// Args: @@ -133,11 +153,7 @@ struct VolumeGap { fn native_volume(amount: f64, currency: db::QuoteCurrency) -> (String, String) { ( format!("{} {}", fmt::compact_si(amount), currency.ticker()), - format!( - "{} {}", - fmt::compact(amount, currency.display_decimals()), - currency.ticker() - ), + exact_native(amount, currency), ) } @@ -323,6 +339,82 @@ fn open_positions_fact(open: &db::OpenPositions) -> Option { Some(open_fact) } +/// Whether the loaded snapshot's own filter names exactly one real core. +/// +/// The predicate is CARDINALITY over `data.filter.core_uids`, deliberately not selector +/// provenance. An All or Overview selector that happens to resolve to one core in the connected +/// universe is exactly as honest a scope to average over as an explicit single-core pick: either +/// way there is one core's order sizes in the sums and no second core to mix them with, so the +/// figure is correct regardless of how the selector arrived at that one core. +/// +/// Args: +/// data: Current report snapshot. +/// +/// Returns: +/// Whether the snapshot's own filter names exactly one core, excluding the +/// [`moon_core::config::NO_MATCH_CORE_UID`] sentinel a preset that hides every core emits. +fn single_core_scope(data: &ReportData) -> bool { + data.filter.core_uids.len() == 1 + && data.filter.core_uids[0] != moon_core::config::NO_MATCH_CORE_UID +} + +/// State realized profit as a percentage of the average order over a single-core scope, or +/// nothing when the scope names more than one core or no ratio exists. +/// +/// Denominated exactly as [`db::QuoteBreakdown::average_order_return`] chose, so this fact can +/// never disagree with the currency the row's own promoted money is stated in. Wording mirrors +/// [`traded_volume_amount`]'s partial-vs-complete split: an incomplete counted scope takes the +/// warn tone and the partial wording ahead of the current-rate wording, whatever the valuation +/// mode, because the shortfall is the more important signal. +/// +/// Args: +/// data: Current report snapshot. +/// mode: Valuation mode the snapshot's figures were computed under. +/// +/// Returns: +/// The assembled fact, or `None` outside a single-core scope or without a computable ratio. +fn average_order_fact(data: &ReportData, mode: db::valuation::ValuationMode) -> Option { + if !single_core_scope(data) { + return None; + } + let ret = data.totals.average_order_return()?; + let (pct, sign) = fmt::signed_pct(ret.pct, 1)?; + let avg = exact_native(ret.avg_order, ret.currency); + if ret.excluded > 0 { + let mut partial = fact( + t!("report.avg_order_pct_partial", pct = pct.clone()).to_string(), + FactTone::Warn, + false, + ); + partial.spelled = Some( + t!( + "report.avg_order_pct_partial_tip", + pct = pct, + avg = avg, + n = ret.counted, + excluded = ret.excluded + ) + .to_string(), + ); + return Some(partial); + } + // A current-rate figure is not historical P&L, so it never borrows that sentence — the same + // gate the traded-volume fact uses for the identical wording pair. + let current = ret.unified && mode == db::valuation::ValuationMode::Current; + let tip_key = if current { + "report.avg_order_pct_current_tip" + } else { + "report.avg_order_pct_tip" + }; + let mut complete = fact( + t!("report.avg_order_pct", pct = pct.clone()).to_string(), + money_tone(sign), + false, + ); + complete.spelled = Some(t!(tip_key, pct = pct, avg = avg, n = ret.counted).to_string()); + Some(complete) +} + /// Assemble every fact the totals row states, split by whether it may be clipped. /// /// The tail order is the priority order and is deliberate. A valuation stall leads it because it @@ -330,9 +422,12 @@ fn open_positions_fact(open: &db::OpenPositions) -> Option { /// quote totals come next because a missing currency total silently changes what the row appears to /// sum, whereas a missing row count only withholds a tally the table itself shows. Everything ahead /// of the traded volume QUALIFIES the realized figure in the never-clipped head, so losing one of -/// them changes what that head appears to mean. The open-positions fact closes the tail because it -/// is the one entry that names itself completely — it can never be misread as part of the head, and -/// the grid above already shows those rows — so it is the cheapest thing to lose to a narrow dock. +/// them changes what that head appears to mean. The average-order-return fact sits right after the +/// traded volume it shares a section with — it too qualifies the head's money, over the narrower +/// single-core scope the volume figure does not itself require. The open-positions fact closes the +/// tail because it is the one entry that names itself completely — it can never be misread as part +/// of the head, and the grid above already shows those rows — so it is the cheapest thing to lose +/// to a narrow dock. /// The shown-rows count is not in the tail at all; it is pinned right, and the ORDER count rides in /// the never-clipped caption. /// @@ -548,6 +643,10 @@ pub(super) fn footer_facts( volume.section_start = true; tail.push(volume); } + if let Some(average) = average_order_fact(data, mode) { + // Qualifies the volume section it immediately follows rather than opening its own. + tail.push(average); + } if let Some(mut open) = open_positions_fact(&data.open) { open.section_start = true; tail.push(open); diff --git a/crates/moon-ui-gpui/src/panels/report/totals/tests.rs b/crates/moon-ui-gpui/src/panels/report/totals/tests.rs index 214616f2..81ebf13e 100644 --- a/crates/moon-ui-gpui/src/panels/report/totals/tests.rs +++ b/crates/moon-ui-gpui/src/panels/report/totals/tests.rs @@ -6,7 +6,10 @@ use moon_core::db::valuation::{ FailureKind, ValuationFault, ValuationMode, ValuationStage, ValuationStatus, }; -use moon_core::db::{QuoteBreakdown, QuoteCurrency, QuoteVolume, TradedVolume, ValuationCoverage}; +use moon_core::db::{ + EntrySpend, QuoteBreakdown, QuoteCurrency, QuoteSpend, QuoteVolume, ReportFilter, TradedVolume, + ValuationCoverage, +}; use super::{FactTone, FooterFacts, footer_facts, footer_tooltip}; use crate::panels::report::query::ReportData; @@ -44,6 +47,24 @@ fn with_volume(mut snapshot: ReportData, volume: TradedVolume) -> ReportData { snapshot } +/// Attach a complete native average-order carrier that is independent of the Report profit groups. +fn with_average_order(mut snapshot: ReportData) -> ReportData { + let usdt = QuoteCurrency::from_report_ordinal(1).expect("USDT ordinal"); + snapshot.totals.entry_spend = EntrySpend { + totals: vec![QuoteSpend { + currency: usdt, + spent: 100.0, + profit: 10.0, + orders: 1, + }], + counted_orders: 1, + valued_orders: 0, + usdt_spent: 0.0, + usdt_profit: 0.0, + }; + snapshot +} + /// A worker that has been failing long enough and often enough to report as stuck. /// /// The fault is built from its public fields rather than through `FaultCause`, which stays @@ -873,3 +894,68 @@ fn open_positions_fact_clips_before_traded_volume() { tally away first, got tail = {texts:?}" ); } + +/// `totals.rs::single_core_scope` must exclude `NO_MATCH_CORE_UID`. Dropping that conjunct would +/// render an average-order percentage for a workspace preset that intentionally names no core. +#[test] +fn no_match_scope_has_no_average_order_fact() { + let mut snapshot = with_average_order(data(vec![(Some(1), 10.0, 1)], 1)); + snapshot.filter = std::sync::Arc::new(ReportFilter { + core_uids: vec![moon_core::config::NO_MATCH_CORE_UID], + ..ReportFilter::default() + }); + + assert!( + super::average_order_fact(&snapshot, ValuationMode::Historical).is_none(), + "the no-match sentinel represents an empty visible scope, not one selected core" + ); +} + +/// `totals.rs::footer_facts` must push the average-order fact after traded volume and before open +/// positions. Moving it after open positions would make it survive longer than the more important +/// open tally or disrupt the footer's documented narrow-dock clipping priority. +#[test] +fn average_order_fact_clips_between_volume_and_open_positions() { + let usdt = QuoteCurrency::from_report_ordinal(1).expect("USDT ordinal"); + let mut snapshot = with_volume( + with_average_order(data(vec![(Some(1), 10.0, 1)], 1)), + TradedVolume { + totals: vec![QuoteVolume { + currency: usdt, + amount: 420.0, + orders: 1, + reconstructed: 1, + }], + eligible_orders: 1, + reconstructed_orders: 1, + valued_orders: 1, + usdt: Some(420.0), + ..Default::default() + }, + ); + snapshot.filter = std::sync::Arc::new(ReportFilter { + core_uids: vec![7], + ..ReportFilter::default() + }); + snapshot.open = moon_core::db::OpenPositions::from_groups(vec![(Some(1), 50.0, 1)]); + + let facts = historical_facts(Some(&snapshot), false, &ValuationStatus::default(), T0); + let texts: Vec<&str> = facts.tail.iter().map(|fact| fact.text.as_str()).collect(); + let volume_ix = texts + .iter() + .position(|text| text.starts_with("Volume")) + .expect("the volume fixture must make its fact visible"); + let average_ix = texts + .iter() + .position(|text| text.contains("avg order")) + .expect("the complete one-core fixture must make its average fact visible"); + let open_ix = texts + .iter() + .position(|text| text.starts_with("open:")) + .expect("the open-position fixture must make its fact visible"); + + assert!( + volume_ix < average_ix && average_ix < open_ix, + "the average-order fact must remain between volume and open positions, got tail = {texts:?}" + ); +} diff --git a/crates/moon-ui-gpui/src/screener/view.rs b/crates/moon-ui-gpui/src/screener/view.rs index 35714e88..04daf8f2 100644 --- a/crates/moon-ui-gpui/src/screener/view.rs +++ b/crates/moon-ui-gpui/src/screener/view.rs @@ -539,8 +539,9 @@ impl ScreenerView { let view = cx.entity(); let visible_count = self.visible_cols.len(); let mut menu = MoonDropdown::new("screener-columns") - // Use the shared column-selector glyph button instead of a text-field trigger. - .segment(moon_ui::MoonButtonSegment::new("▦")) + // The shared column-selector icon button instead of a text-field trigger; the asset + // and the childless trigger are `design::COLUMN_SELECTOR_ICON`'s contract. + .trigger_icon(design::COLUMN_SELECTOR_ICON) .trigger_variant(MoonButtonVariant::Soft) .trigger_size(MoonButtonSize::Action) .trigger_width(design::glyph_btn_w(cx)) diff --git a/crates/moon-ui-gpui/src/settings/connections/columns.rs b/crates/moon-ui-gpui/src/settings/connections/columns.rs index dd55a868..211327ea 100644 --- a/crates/moon-ui-gpui/src/settings/connections/columns.rs +++ b/crates/moon-ui-gpui/src/settings/connections/columns.rs @@ -7,7 +7,7 @@ //! min-content width, and the row's children are live controls that render WIDER than the basis //! (`MoonDropdown::trigger_width_scaled` multiplies by the Font-slider ratio; `MoonColorPicker` //! draws a hard 128px trigger). The header's plain text labels never inflate, so only the header -//! had slack left for its two growing columns to absorb -- and every column after them drifted. +//! had slack left for its growing columns to absorb -- and every column after them drifted. //! //! The fix is structural rather than a nudged constant: one list; `min_w_0()` on every cell so the //! declared basis is the whole truth (Taffy resolves an AUTO minimum from min-content, an explicit @@ -74,7 +74,9 @@ pub(super) struct ConnCol { /// Flex basis in rendered pixels. Authoritative: every cell also carries `min_w_0()`, so a /// wider child paints over its neighbour instead of pushing it. pub(super) basis: f32, - /// Whether the column absorbs free space. Exactly the two text columns do. + /// Whether the column absorbs free space. Only columns holding user-typed text that + /// TRUNCATES do -- `h-name` and `h-group`. `h-key` deliberately does not: its content is + /// masked, so extra width buys more dots and nothing readable. pub(super) grow: bool, /// How [`ConnCol::basis`] becomes a rendered width. pub(super) width: ConnColWidth, @@ -122,12 +124,19 @@ const CONN_COLS: [ConnCol; 13] = [ align: ConnColAlign::Left, head_pad: 8.0, }, + // FIXED, not growing, at the basis it always had. The key field is MASKED and MoonUI draws + // one bullet per character of a variable-length key, so there is no "width of the masked + // value" to size to -- growth simply handed the column every spare pixel, ~480px of a 1791px + // window spent on identical dots while `Имя` and `Группа` truncated real text beside them. + // 200 keeps a usable text viewport next to the input's mask-toggle and clear affixes and the + // sibling Paste glyph (`table.rs::paste_key_affix`); a longer key scrolls inside the field, + // which is what it did before and what a masked field can afford. ConnCol { id: "h-key", label: Some("conn.col.key"), tip: Some("conn.tip.key"), basis: 200.0, - grow: true, + grow: false, width: ConnColWidth::Raw, align: ConnColAlign::Left, head_pad: 8.0, @@ -155,12 +164,14 @@ const CONN_COLS: [ConnCol; 13] = [ align: ConnColAlign::Center, head_pad: 0.0, }, + // Grows with `h-name`: both hold user-typed text that truncates, and the width the masked + // key gave up is exactly what they were missing. ConnCol { id: "h-group", label: Some("conn.col.group"), tip: Some("conn.tip.group"), basis: 110.0, - grow: false, + grow: true, width: ConnColWidth::Raw, align: ConnColAlign::Left, head_pad: 8.0, diff --git a/crates/moon-ui-gpui/src/settings/connections/columns/tests.rs b/crates/moon-ui-gpui/src/settings/connections/columns/tests.rs index c38d1f9b..efddf812 100644 --- a/crates/moon-ui-gpui/src/settings/connections/columns/tests.rs +++ b/crates/moon-ui-gpui/src/settings/connections/columns/tests.rs @@ -82,16 +82,17 @@ fn widths_follow_the_frozen_per_column_policy() { } } -/// Only Name and Key may absorb spare width, and every visible header label needs help text; -/// widening another column or dropping a tooltip would misplace controls or leave a heading -/// unexplained to the user. +/// `columns.rs:CONN_COLS` must let only Name and Group absorb spare width: Key is excluded because +/// its masked content has no readable length to reward with width. Turning `h-key.grow` back on +/// wastes space on dots, while removing growth from Name or Group truncates user-entered text; +/// every visible header label also needs help text. #[test] fn growth_and_tooltips_match_the_text_column_contract() { let growing: Vec<_> = ConnColId::ALL .into_iter() .filter(|column| column.spec().grow) .collect(); - assert_eq!(growing, [ConnColId::Name, ConnColId::Key]); + assert_eq!(growing, [ConnColId::Name, ConnColId::Group]); for column in ConnColId::ALL { let spec = column.spec(); diff --git a/crates/moon-ui-gpui/src/settings/connections/tab.rs b/crates/moon-ui-gpui/src/settings/connections/tab.rs index bf9d1071..2591cc1e 100644 --- a/crates/moon-ui-gpui/src/settings/connections/tab.rs +++ b/crates/moon-ui-gpui/src/settings/connections/tab.rs @@ -9,9 +9,10 @@ use std::sync::Arc; use gpui::*; use moon_ui::{ - MoonButton, MoonButtonSize, MoonButtonVariant, MoonCheckbox, MoonCheckboxSize, MoonDropdown, - MoonMenuSize, MoonPalette, MoonPopover, MoonPopoverPlacement, MoonScrollbarVisibility, - MoonSelect, MoonTooltipView, MoonVirtualList, StyledExt, h_flex, v_flex, + MoonButton, MoonButtonIconSlot, MoonButtonSize, MoonButtonVariant, MoonCheckbox, + MoonCheckboxSize, MoonDropdown, MoonMenuSize, MoonPalette, MoonPopover, MoonPopoverPlacement, + MoonScrollbarVisibility, MoonSelect, MoonTooltipView, MoonVirtualList, StyledExt, h_flex, + v_flex, }; use rust_i18n::t; @@ -115,7 +116,9 @@ fn subsection_header_row( .gap_2() .items_center() .pl(px(CONN_TABLE_INSET)) - .pr_1() + // A row of the same virtual list as the core rows, so it clears the overlay scrollbar the + // same way; `pr_1` alone left the member count under the track once the list overflowed. + .pr(design::ui_px(cx, design::MOON_SCROLLBAR_OVERLAY_W)) .py_0p5() .child( div() @@ -316,6 +319,11 @@ fn group_header_row( .gap_1() .items_center() .px_1() + // Overrides the `px_1` right inset only: this row ends in live controls -- the proto + // dropdown, the icon popover and the "+ core" button -- and it is a row of the same + // virtual list, whose overlay scrollbar would otherwise paint over and swallow clicks on + // the right edge of that button. + .pr(design::ui_px(cx, design::MOON_SCROLLBAR_OVERLAY_W)) .py_0p5() .rounded(design::r_button(cx)) .bg(rgb(p.panel_high)) @@ -343,9 +351,12 @@ fn group_header_row( }), ) .child(ico_el) + // The name SHRINKS but never GROWS: `flex_1` here handed it every spare pixel of a + // 1791px row, which pushed the count away from the name it counts and left the controls + // scattered across the gap instead of reading as one cluster. `min_w_0` keeps the + // truncation, so a long name still ellipsises rather than pushing the cluster off-row. .child( div() - .flex_1() .min_w_0() .truncate() .font_bold() @@ -353,23 +364,34 @@ fn group_header_row( ) .child( div() + .flex_shrink_0() .text_size(design::t_body(cx)) .text_color(rgb(p.text_soft)) .child(t!("conn.member_count", n = member_count).to_string()), ) .child( - div() - .id(SharedString::from(format!("eye-tip-{name}"))) - .tooltip(|_window, cx| { - cx.new(|_| MoonTooltipView::new(t!("conn.show_group").to_string())) - .into() - }) + // One right-aligned cluster: `ml_auto` absorbs the free space the name gave up, so + // the four controls sit together at the row's end instead of spread along it. + h_flex() + .ml_auto() + .flex_shrink_0() + .gap_1() + .items_center() .child( MoonButton::new(SharedString::from(format!("eye-{name}"))) .ghost() .size(MoonButtonSize::Micro) .width(34.0) - .label("win") + // The label read "win" -- an untranslated abbreviation of an action this + // button does NOT perform: it opens the group's window + // (`show_group_request`), while `win` is the per-core headless toggle in + // `table.rs`. The glyph names the action and the tooltip carries the + // sentence. Same icon, size and variant as the chart strip's + // gather-windows button (`chart_tabs/strip.rs`), tooltip included -- + // `MoonButton` takes one natively, so an icon-only button needs no + // wrapping `div` to host it. + .leading_icon(MoonButtonIconSlot::new("icons/window-restore.svg")) + .tooltip(t!("conn.show_group").to_string()) .on_click({ let weak = weak.clone(); move |_, _, cx| { @@ -383,25 +405,26 @@ fn group_header_row( } }) .render(), + ) + .child(bulk_proto_dropdown(weak, name)) + .child(popover) + .child( + MoonButton::new(SharedString::from(format!("addgrp-{name}"))) + .outline() + .size(MoonButtonSize::Micro) + .width(56.0) + .label(format!("+ {}", t!("conn.add_core_short"))) + .on_click({ + let weak = weak.clone(); + move |_, window, cx| { + let n = nm_add.clone(); + let _ = + weak.update(cx, |this, ctx| this.add_server(n, window, ctx)); + } + }) + .render(), ), ) - .child(bulk_proto_dropdown(weak, name)) - .child(popover) - .child( - MoonButton::new(SharedString::from(format!("addgrp-{name}"))) - .outline() - .size(MoonButtonSize::Micro) - .width(56.0) - .label(format!("+ {}", t!("conn.add_core_short"))) - .on_click({ - let weak = weak.clone(); - move |_, window, cx| { - let n = nm_add.clone(); - let _ = weak.update(cx, |this, ctx| this.add_server(n, window, ctx)); - } - }) - .render(), - ) } /// Build one group's icon-picker grid, as the content of its anchored [`MoonPopover`]. diff --git a/crates/moon-ui-gpui/src/settings/connections/table.rs b/crates/moon-ui-gpui/src/settings/connections/table.rs index ece9b7fd..f0d113da 100644 --- a/crates/moon-ui-gpui/src/settings/connections/table.rs +++ b/crates/moon-ui-gpui/src/settings/connections/table.rs @@ -967,9 +967,13 @@ pub(super) fn server_row( .state(&row.group) .small() .into_any_element(), + // An empty bundle field is the DEFAULT, not an omission, so the placeholder names what + // the field would hold rather than nudging: it is a bundle NAME (`ChartBucket::Bundle` + // in `moon-core/src/config/servers.rs`), which an empty white cell said nothing about. MoonInput::new(ids.bundle.clone()) .state(&row.bundle) .small() + .placeholder(t!("conn.bundle_ph").to_string()) .into_any_element(), feed_popover(view, weak, i, row_key, ids, cx).into_any_element(), MoonColorPicker::new(&row.color).into_any_element(), @@ -994,6 +998,10 @@ pub(super) fn server_row( .gap_1() .items_center() .py_0p5() + // The list's scrollbar is an overlay that reserves no width of its own, so the status dot, + // the reconnect glyph and the delete button rendered beneath it. The header subtracts the + // same gutter, or the two stop lining up. + .pr(design::ui_px(cx, design::MOON_SCROLLBAR_OVERLAY_W)) .children( ConnColId::ALL .into_iter() @@ -1012,7 +1020,7 @@ impl SettingsView { /// `min_w_0()` is the load-bearing part: gpui's default `min_size: auto` is the CONTENT-based /// automatic minimum, which clamps a flex item UP to its child's min-content width. A control /// that renders wider than its column would then eat free space in the rows that the header - /// still had -- and since only the header can hand that space to its two growing columns, + /// still had -- and since only the header can hand that space to its growing columns, /// every column after them drifted, further with each one. Pinned to the basis, an oversized /// child overlaps instead of shifting the grid. /// @@ -1130,6 +1138,9 @@ impl SettingsView { .gap_1() .items_center() .pl(px(CONN_TABLE_INSET)) + // The same gutter every row of the list below reserves for its overlay scrollbar, so + // the headings stay over their own columns. + .pr(design::ui_px(cx, design::MOON_SCROLLBAR_OVERLAY_W)) .pb(px(3.0)) .border_b_1() .border_color(rgb(p.border)) diff --git a/crates/moon-ui-gpui/src/strategies/mod.rs b/crates/moon-ui-gpui/src/strategies/mod.rs index bee97f7b..d3b7bd16 100644 --- a/crates/moon-ui-gpui/src/strategies/mod.rs +++ b/crates/moon-ui-gpui/src/strategies/mod.rs @@ -155,8 +155,24 @@ pub struct StrategiesView { field_colors: HashMap)>, /// Field whose contextual helper or autocomplete is open. focused_field: Option, - /// Expanded cores in the strategy tree. + /// Cores the user expanded by hand in the strategy tree — the only expansion that persists. expanded_cores: HashSet, + /// Core opened because the Auto rail selected it, or `None` under Classic and Auto Overview. + /// + /// An OVERLAY over [`Self::expanded_cores`], never a member of it: the union is what renders and + /// what the Expand/Collapse-all caret reads, but only `expanded_cores` reaches + /// `StrategiesSessionState`, so a rail seed is never inherited by a later window or another scope. + /// `Option` rather than a set is deliberate — the rail names at most one core, and the type is what + /// makes a rail move REPLACE the previous seed instead of accumulating it. + rail_expanded_core: Option, + /// Auto rail selection last resolved for this view. + /// + /// Distinct from the overlay because the overlay is the USER'S to clear: collapsing the seeded + /// core's caret empties the overlay and deliberately leaves this field alone, so a later + /// workspace revision that resolves the SAME rail selection recognises it as unchanged and does + /// not reopen the row. Only a rail selection that actually MOVED — including moving to `None` + /// under Auto Overview — re-seeds. + rail_seen_core: Option, /// Expanded tree folders keyed by core and path. expanded_folders: HashSet<(CoreId, String)>, /// Field-dependency rules from `param_deps.toml`, hot-reloaded only with the opt-in environment flag. diff --git a/crates/moon-ui-gpui/src/strategies/selection.rs b/crates/moon-ui-gpui/src/strategies/selection.rs index 128cc415..528e85cb 100644 --- a/crates/moon-ui-gpui/src/strategies/selection.rs +++ b/crates/moon-ui-gpui/src/strategies/selection.rs @@ -299,6 +299,9 @@ impl StrategiesView { if !collapsed { self.expanded_cores.clear(); self.expanded_folders.clear(); + // Otherwise the rail-seeded core is the one row that survives "collapse everything" — + // it lives in the overlay, not in `expanded_cores`. + self.rail_expanded_core = None; return; } for (c, _) in cores { diff --git a/crates/moon-ui-gpui/src/strategies/session.rs b/crates/moon-ui-gpui/src/strategies/session.rs index 47f57b3a..c2ebad6d 100644 --- a/crates/moon-ui-gpui/src/strategies/session.rs +++ b/crates/moon-ui-gpui/src/strategies/session.rs @@ -8,7 +8,11 @@ use super::*; /// Restorable Strategies browsing state for the current process only. #[derive(Clone, Default)] pub(crate) struct StrategiesSessionState { - /// Cores the user left expanded in the tree. + /// Cores the user left expanded in the tree, by hand. + /// + /// `StrategiesView::rail_expanded_core` — the Auto rail's live seed — is deliberately absent + /// from this snapshot: capturing it would let a rail seed outlive the window that received it + /// and reappear as if the user had expanded that core themselves, in another scope or window. pub(crate) expanded_cores: HashSet, /// Folders the user left expanded, keyed by core and slash-separated path. pub(crate) expanded_folders: HashSet<(CoreId, String)>, diff --git a/crates/moon-ui-gpui/src/strategies/state.rs b/crates/moon-ui-gpui/src/strategies/state.rs index 1666d915..f59941f4 100644 --- a/crates/moon-ui-gpui/src/strategies/state.rs +++ b/crates/moon-ui-gpui/src/strategies/state.rs @@ -4,14 +4,17 @@ use super::*; use crate::workspace::scope_marker::ScopeMarker; -/// Seed the tree's expanded cores from the Auto rail selection. +/// Resolve the Auto rail's seeded core, the overlay `StrategiesView::rail_expanded_core` holds. /// /// The window is opened from a rail that already says which server the user is working on, so -/// opening it fully collapsed makes them re-find that server by hand every time. +/// opening it fully collapsed makes them re-find that server by hand every time. The result +/// REPLACES any previous seed rather than accumulating it: the rail names at most one core, and a +/// stale seed for a core the rail left must not linger as if the user had expanded it by hand. /// /// The intersection with the visible scope keeps two validity levels distinct: `selected_core` is /// validated against live cores, while the tree is built from the effective workspace scope. The -/// guard prevents any narrower scope from seeding a node that never renders. +/// guard prevents any narrower scope from seeding a node that never renders. A `None` workspace +/// slice (not scope-bound) still seeds. /// /// A core whose strategies are all filtered out does not enter the tree at all; the seeded /// expansion is harmless there and takes effect as soon as the filter admits it. @@ -21,45 +24,60 @@ use crate::workspace::scope_marker::ScopeMarker; /// workspace_cores: Cores the window may show, or `None` when it is not scope-bound. /// /// Returns: -/// The set of cores to open with; empty whenever there is nothing unambiguous to expand. -fn initial_expanded_cores( +/// The core to open with, or `None` whenever there is nothing unambiguous to expand. +fn rail_seed_core( selected_core: Option, workspace_cores: Option<&[CoreId]>, -) -> HashSet { - let Some(core) = selected_core else { - return HashSet::new(); - }; +) -> Option { + let core = selected_core?; if workspace_cores.is_some_and(|cores| !cores.contains(&core)) { - return HashSet::new(); + return None; } - HashSet::from([core]) + Some(core) } -/// Insert the Auto-selected core into an existing expansion set without replacing it. -/// -/// Opening or focusing Strategies from a rail that already names a server must show that -/// server's strategies without an extra click. Other cores the user expanded stay expanded; -/// an empty seed (Classic, Auto Overview, or a core outside the visible scope) is a no-op. +/// Whether the retained expansion state considers `core` open: hand-expanded, or the current +/// Auto rail seed. /// /// Args: -/// expanded: Live or restored set of expanded cores. -/// selected_core: Concrete Auto rail selection, or `None` for Classic and Auto Overview. -/// workspace_cores: Cores the window may show, or `None` when it is not scope-bound. +/// expanded: Cores the user expanded by hand. +/// rail: The current rail overlay, if any. +/// core: Core being tested. /// /// Returns: -/// Whether at least one core was newly inserted. -fn seed_selected_core_into( +/// `true` when either source counts `core` as open. +pub(super) fn core_is_open(expanded: &HashSet, rail: Option, core: CoreId) -> bool { + expanded.contains(&core) || rail == Some(core) +} + +/// Toggle one core's expansion across both the persisted set and the rail overlay. +/// +/// Collapsing clears the rail seed too when it names this core: otherwise a click meant to close +/// the row would leave it reopened by the overlay on the very next frame. Expanding writes only +/// the persisted set, matching every other hand-expansion site — the overlay is exclusively an +/// Auto rail concern. +/// +/// Deliberately leaves `StrategiesView::rail_seen_core` untouched: that field tracks what the rail +/// last resolved to, not what the user is currently showing, so a later unrelated revision that +/// resolves the same rail selection recognises it as unchanged instead of reopening this row. +/// +/// Args: +/// expanded: Cores the user expanded by hand, mutated in place. +/// rail: The current rail overlay, mutated in place. +/// core: Core being toggled. +pub(super) fn toggle_core_expansion( expanded: &mut HashSet, - selected_core: Option, - workspace_cores: Option<&[CoreId]>, -) -> bool { - let seed = initial_expanded_cores(selected_core, workspace_cores); - if seed.is_empty() { - return false; + rail: &mut Option, + core: CoreId, +) { + if core_is_open(expanded, *rail, core) { + expanded.remove(&core); + if *rail == Some(core) { + *rail = None; + } + } else { + expanded.insert(core); } - let added = seed.iter().any(|core| !expanded.contains(core)); - expanded.extend(seed); - added } /// Resolve the cores a Classic-focused singleton window may DISPLAY. @@ -200,8 +218,10 @@ impl StrategiesView { /// Create the Strategies view and subscribe it to search, tree, backend, and window events. /// /// A process-lifetime snapshot restores browsing state after the window is closed and - /// reopened. Construction then additively seeds the Auto rail's selected core when it belongs - /// to the visible workspace scope, so a collapsed snapshot still opens that server's list. + /// reopened. Construction then seeds the Auto rail's selected core into the `rail_expanded_core` + /// overlay when it belongs to the visible workspace scope, so a collapsed snapshot still opens + /// that server's list — the overlay, never the restored snapshot itself, which is why a rail + /// seed never survives into another window or scope. /// /// Args: /// backend: Shared state supplying strategy data and workspace scope. @@ -254,15 +274,13 @@ impl StrategiesView { .fold(0u64, |acc, s| acc.wrapping_mul(31).wrapping_add(s.id)); let selected_core = scope.and_then(|(_, selected)| selected); let initial_sig = strategies_sig(backend.read(cx), workspace_cores.as_deref()); - let mut expanded_cores = match &session { + let expanded_cores = match &session { Some(s) => s.expanded_cores.clone(), None => HashSet::new(), }; - seed_selected_core_into( - &mut expanded_cores, - selected_core, - workspace_cores.as_deref(), - ); + let rail_seed = rail_seed_core(selected_core, workspace_cores.as_deref()); + let rail_expanded_core = rail_seed; + let rail_seen_core = rail_seed; let tree_state = cx.new(|cx| MoonTreeState::new(cx)); // MoonTree can mutate expansion from keyboard input, but `expanded_cores` and @@ -357,22 +375,32 @@ impl StrategiesView { this.scope_marker = marker; let scope = singleton_strategy_scope(this.backend.read(cx)); let next = scope.as_ref().map(|(cores, _)| cores.clone()); + // Recomputed BEFORE the shape-equality return below: a single-core group's Overview and + // AutoCore id vectors are equal, so a live rail move between them would otherwise never + // reach this observer's body at all. + let selected_core = scope.as_ref().and_then(|(_, selected)| *selected); + let rail = rail_seed_core(selected_core, next.as_deref()); + // Compared against `rail_seen_core`, never the overlay: the overlay is the user's to + // clear (`toggle_core_expansion`), and comparing against it would resurrect a + // hand-collapsed core on the next unrelated revision. `rail_seen_core` tracks what the + // rail last resolved to regardless of what the user did with the overlay afterwards, so + // only a rail selection that actually MOVED re-seeds. + let rail_moved = rail != this.rail_seen_core; + if rail_moved { + // REPLACE, never extend: the overlay is what the rail says NOW. `None` under Auto + // Overview is the live-window twin of "a seed is never carried into another scope". + this.rail_seen_core = rail; + this.rail_expanded_core = rail; + this.tree_cache = None; + this.last_tree_shape = None; + } if next == this.workspace_cores { - if marker_moved { + if marker_moved || rail_moved { cx.notify(); } return; } this.workspace_cores = next; - // Re-seed additively when an Auto rail move changes the scope because the singleton - // window outlives the selection. Existing expansions remain intact; returning to a - // manually collapsed core re-opens it. - let selected_core = scope.and_then(|(_, selected)| selected); - seed_selected_core_into( - &mut this.expanded_cores, - selected_core, - this.workspace_cores.as_deref(), - ); this.last_sig = strategies_sig(this.backend.read(cx), this.workspace_cores.as_deref()); this.tree_cache = None; this.last_tree_shape = None; @@ -504,6 +532,8 @@ impl StrategiesView { field_colors: HashMap::new(), focused_field: None, expanded_cores, + rail_expanded_core, + rail_seen_core, expanded_folders: session .as_ref() .map(|s| s.expanded_folders.clone()) @@ -550,30 +580,41 @@ impl StrategiesView { }); } - /// Expand the Auto-selected core in a live Strategies view without collapsing anything else. + /// Reseed a live Strategies view from the Auto-selected core while preserving hand expansions. /// - /// Used when focusing an already-open window. Construction seeds the same way before the + /// Used when focusing an already-open window. Construction seeds the same overlay before the /// first paint; this path must also drop the tree cache so the next frame rebuilds the - /// selected core's subtree. + /// selected core's subtree. Nothing is persisted here: the rail overlay never reaches + /// `StrategiesSessionState`, so there is nothing for `persist_session` to save. + /// + /// Compared against the OVERLAY, not `rail_seen_core`: focusing an already-open window + /// deliberately REOPENS the seeded core even if the user had collapsed it by hand, which is + /// why this path disagrees with the `workspace_revision` observer's comparison. /// /// Args: - /// cx: View context used to persist the session and notify. + /// cx: View context used to read the current scope and notify. pub(super) fn ensure_auto_selected_core_expanded(&mut self, cx: &mut Context) { let selected_core = singleton_strategy_scope(self.backend.read(cx)).and_then(|(_, selected)| selected); - if !seed_selected_core_into( - &mut self.expanded_cores, - selected_core, - self.workspace_cores.as_deref(), - ) { + let rail = rail_seed_core(selected_core, self.workspace_cores.as_deref()); + if rail == self.rail_expanded_core && rail == self.rail_seen_core { return; } + self.rail_seen_core = rail; + self.rail_expanded_core = rail; self.tree_cache = None; self.last_tree_shape = None; - self.persist_session(cx); cx.notify(); } + /// Toggle one core's expansion from a tree click, over both expansion fields. + /// + /// Args: + /// core: Core whose row was clicked. + pub(super) fn toggle_core_expanded(&mut self, core: CoreId) { + toggle_core_expansion(&mut self.expanded_cores, &mut self.rail_expanded_core, core); + } + // ── Selection ─────────────────────────────────────────────────────────── } diff --git a/crates/moon-ui-gpui/src/strategies/state/tests.rs b/crates/moon-ui-gpui/src/strategies/state/tests.rs index 614b7ef7..5a816a28 100644 --- a/crates/moon-ui-gpui/src/strategies/state/tests.rs +++ b/crates/moon-ui-gpui/src/strategies/state/tests.rs @@ -1,5 +1,5 @@ -//! Unit tests for Auto-rail tree-expansion seeding (`initial_expanded_cores` and -//! `seed_selected_core_into`). +//! Unit tests for the Auto-rail overlay (`rail_seed_core`, `core_is_open`, +//! `toggle_core_expansion`). // NOT `use super::*`: the grandparent `strategies` module imports `gpui::*`, whose `test` macro // shadows `#[test]`, and `state.rs` re-exposes that glob via its own `use super::*`. Reach the @@ -8,102 +8,191 @@ use std::collections::HashSet; use moon_core::session::CoreId; -use super::{initial_expanded_cores, seed_selected_core_into}; +use super::{core_is_open, rail_seed_core, toggle_core_expansion}; -/// `strategies/state.rs::initial_expanded_cores` seeds the tree with the Auto rail's selected -/// core, not an unconditionally empty set. +/// `strategies/state.rs::rail_seed_core` resolves nothing when no Auto core is selected. /// -/// Mutation: replacing the `HashSet::from([core])` result with `HashSet::new()`. The window would -/// then always open fully collapsed even with a concrete Auto core selected on the rail, so the -/// user has to re-find that server by hand every time. -/// -/// `initial_expanded_cores` is a pure function of `(Option, Option<&[CoreId]>) -> -/// HashSet` with no GPUI dependency, so it is exercised directly rather than through a -/// headlessly-constructed `StrategiesView`. +/// Mutation: seeding unconditionally regardless of `selected_core`. Classic and Auto Overview +/// would then force-expand whatever core last happened to resolve. #[test] -fn no_selected_core_seeds_nothing() { - assert_eq!(initial_expanded_cores(None, Some(&[1, 2])), HashSet::new()); +fn no_selected_core_resolves_no_seed() { + assert_eq!(rail_seed_core(None, Some(&[1, 2])), None); } #[test] -fn a_core_outside_the_workspace_scope_seeds_nothing() { +fn a_core_outside_the_workspace_scope_resolves_no_seed() { let core: CoreId = 7; - assert_eq!( - initial_expanded_cores(Some(core), Some(&[1, 2])), - HashSet::new() - ); + assert_eq!(rail_seed_core(Some(core), Some(&[1, 2])), None); } #[test] -fn a_core_inside_the_workspace_scope_seeds_itself() { +fn a_core_inside_the_workspace_scope_resolves_itself() { let core: CoreId = 2; - assert_eq!( - initial_expanded_cores(Some(core), Some(&[1, 2])), - HashSet::from([core]) - ); + assert_eq!(rail_seed_core(Some(core), Some(&[1, 2])), Some(core)); } -/// A window with no scope-bound workspace (`workspace_cores: None`, e.g. Classic or an -/// unscoped Auto owner) still seeds the selected core — the scope guard only rejects a core it -/// can positively prove is out of bounds. +/// A window with no scope-bound workspace (`workspace_cores: None`, e.g. Classic or an unscoped +/// Auto owner) still resolves the selected core — the scope guard only rejects a core it can +/// positively prove is out of bounds. #[test] -fn an_unscoped_window_still_seeds_the_selected_core() { +fn an_unscoped_window_still_resolves_the_selected_core() { let core: CoreId = 5; + assert_eq!(rail_seed_core(Some(core), None), Some(core)); +} + +/// (a) Seed A into an empty persisted set: A shows open through the overlay alone, the persisted +/// set — what a "close the window" snapshot copies — stays empty, and reopening under Auto +/// Overview (`rail_seed_core(None, ...)`) leaves A closed with the persisted set still empty. +/// +/// Names the item-1 contract: the seed must never enter the persisted set. The constructor +/// mutation that violates it (`expanded_cores.extend`/`.insert` the seed) cannot be exercised +/// through these pure functions alone, which never call `StrategiesView::new` — the load-bearing +/// half of this proof is the static constructor assertion in `theme_contract`, which reads the +/// mutated source directly. +#[test] +fn seed_a_into_an_empty_set_then_reopen_under_overview() { + let workspace = [1, 2]; + let expanded: HashSet = HashSet::new(); + let rail = rail_seed_core(Some(1), Some(&workspace)); + assert!( + core_is_open(&expanded, rail, 1), + "the seeded core must show open through the overlay" + ); + let snapshot = expanded.clone(); assert_eq!( - initial_expanded_cores(Some(core), None), - HashSet::from([core]) + snapshot, + HashSet::new(), + "a 'close the window' snapshot must copy the persisted set alone, never the seed" ); + let rail = rail_seed_core(None, Some(&workspace)); + assert!( + !core_is_open(&expanded, rail, 1), + "reopening under Auto Overview must not keep the seed open" + ); + assert_eq!(expanded, HashSet::new()); } -/// `strategies/state.rs::seed_selected_core_into` inserts the Auto-selected core into an -/// already-populated expansion set without replacing it. -/// -/// Mutation: replace `expanded.extend(seed)` with `*expanded = seed`. Focusing Strategies -/// would then collapse every other core the user had open, leaving only the rail selection. -/// -/// Oracle: the product contract is additive — a concrete Auto rail selection must appear in -/// `expanded_cores`, and any other cores the user already expanded must stay. The expected set -/// is that pair, not a value this helper computed. +/// (b) As (a), then the user hand-expands B: the persisted set becomes exactly `{B}`, and under +/// Auto Overview B is open while the never-persisted seed A is not. #[test] -fn seeds_additively_inserts_the_selected_core() { - let selected: CoreId = 2; - let mut expanded = HashSet::from([1]); - seed_selected_core_into(&mut expanded, Some(selected), Some(&[1, 2])); - assert_eq!(expanded, HashSet::from([1, selected])); +fn seed_a_then_hand_expand_b() { + let workspace = [1, 2]; + let mut expanded: HashSet = HashSet::new(); + let mut rail = rail_seed_core(Some(1), Some(&workspace)); + toggle_core_expansion(&mut expanded, &mut rail, 2); + let snapshot = expanded.clone(); + assert_eq!(snapshot, HashSet::from([2])); + let rail = rail_seed_core(None, Some(&workspace)); + assert!(core_is_open(&expanded, rail, 2), "B must stay open"); + assert!(!core_is_open(&expanded, rail, 1), "A was never persisted"); } -/// `strategies/state.rs::seed_selected_core_into` still inserts into an empty stored set. -/// -/// Mutation: return without inserting when `expanded` is already empty (treating a stored -/// empty set as "the user collapsed everything"). After collapsing the selected Auto core, -/// close and reopen Strategies; the core stays collapsed and the user re-finds the server -/// by hand. -/// -/// Oracle: Auto mode with rail-selected core 4 in a visible workspace of `[4, 5]` must leave -/// `{4}` in `expanded_cores` even when the snapshot stored nothing. +/// (c) Seed A, the user collapses A (which clears the overlay too), then the user expands A by +/// hand: the persisted set becomes `{A}`, A is open, and the snapshot carries A. #[test] -fn seeds_an_empty_stored_set_with_the_selected_core() { - let selected: CoreId = 4; - let mut expanded = HashSet::new(); - seed_selected_core_into(&mut expanded, Some(selected), Some(&[4, 5])); - assert_eq!(expanded, HashSet::from([selected])); +fn seed_a_collapse_then_hand_expand_again() { + let workspace = [1, 2]; + let mut expanded: HashSet = HashSet::new(); + let mut rail = rail_seed_core(Some(1), Some(&workspace)); + toggle_core_expansion(&mut expanded, &mut rail, 1); + assert!( + !core_is_open(&expanded, rail, 1), + "collapsing the seeded core must close it" + ); + assert_eq!(rail, None, "collapsing it must clear the overlay too"); + toggle_core_expansion(&mut expanded, &mut rail, 1); + assert!(core_is_open(&expanded, rail, 1)); + assert_eq!( + expanded, + HashSet::from([1]), + "a hand re-expand must land in the persisted set" + ); + let snapshot = expanded.clone(); + assert_eq!(snapshot, HashSet::from([1])); } -/// Classic / Auto Overview (`selected_core: None`) must not force-expand any core. -/// -/// Companion to `initial_expanded_cores(None, ...)` returning empty: applying that empty seed -/// to a live set is a no-op, so already-expanded cores stay and none are added. +/// (d) A core open through BOTH the persisted set and the overlay collapses on one call: both +/// clear together. #[test] -fn seeds_nothing_into_an_existing_set_when_unselected() { - let mut expanded = HashSet::from([1]); - seed_selected_core_into(&mut expanded, None, Some(&[1, 2])); - assert_eq!(expanded, HashSet::from([1])); +fn collapsing_a_core_open_via_both_sources_clears_both() { + let mut expanded: HashSet = HashSet::from([1]); + let mut rail = Some(1); + toggle_core_expansion(&mut expanded, &mut rail, 1); + assert!(!core_is_open(&expanded, rail, 1)); + assert_eq!( + expanded, + HashSet::new(), + "the persisted membership must clear" + ); + assert_eq!(rail, None, "the overlay must clear in the same call"); } -/// A selected core outside the visible workspace scope must not enter `expanded_cores`. +/// (e) Seed A, the user collapses A, then an unrelated workspace revision resolves the SAME rail +/// selection. Comparing the fresh resolve against the stored `rail_seen_core` (still `Some(A)`) +/// says nothing moved, so A stays collapsed. Contrasted against comparing the overlay instead, +/// which would wrongly say it moved — that contrast is the item-4 regression this catches. #[test] -fn seeds_nothing_when_selected_core_is_out_of_scope() { - let mut expanded = HashSet::from([1]); - seed_selected_core_into(&mut expanded, Some(7), Some(&[1, 2])); - assert_eq!(expanded, HashSet::from([1])); +fn unrelated_revision_after_a_hand_collapse_does_not_reopen_it() { + let workspace = [1, 2]; + let mut expanded: HashSet = HashSet::new(); + let mut rail = rail_seed_core(Some(1), Some(&workspace)); + let mut rail_seen = rail; + toggle_core_expansion(&mut expanded, &mut rail, 1); + assert_eq!(rail, None, "collapsing A must clear the overlay"); + assert_eq!( + rail_seen, + Some(1), + "rail_seen_core is untouched by a hand collapse" + ); + + let resolved = rail_seed_core(Some(1), Some(&workspace)); + + let rail_moved = resolved != rail_seen; + assert!( + !rail_moved, + "an unchanged rail selection compared against rail_seen_core must not look like it moved" + ); + if rail_moved { + rail_seen = resolved; + rail = resolved; + } + assert_eq!( + rail_seen, + Some(1), + "an unmoved rail must leave rail_seen_core untouched" + ); + assert!(!core_is_open(&expanded, rail, 1), "A must stay collapsed"); + + let would_move_against_overlay = resolved != rail; + assert!( + would_move_against_overlay, + "comparing against the overlay instead of rail_seen_core is exactly the regression item 4 guards against" + ); +} + +/// (f) A stale overlay (`Some(A)`) with a rail that now resolves to `None`: assigning +/// unconditionally clears it, while an `is_none()` early-return guard would leave it stale. +#[test] +fn focus_clears_a_stale_overlay_unconditionally() { + let workspace = [1, 2]; + let stale_overlay = Some(1); + let resolved = rail_seed_core(None, Some(&workspace)); + assert_eq!(resolved, None); + + let unconditional_assign = resolved; + assert_eq!( + unconditional_assign, None, + "assigning unconditionally must clear a stale overlay" + ); + + let mut guarded = stale_overlay; + if resolved.is_none() { + // The rejected shape: an early return leaves the stale overlay untouched. + } else { + guarded = resolved; + } + assert_eq!( + guarded, stale_overlay, + "an is_none() guard would wrongly keep the stale overlay, which is why new() assigns unconditionally" + ); } diff --git a/crates/moon-ui-gpui/src/strategies/tree/cache.rs b/crates/moon-ui-gpui/src/strategies/tree/cache.rs index d453e892..6dee9a9d 100644 --- a/crates/moon-ui-gpui/src/strategies/tree/cache.rs +++ b/crates/moon-ui-gpui/src/strategies/tree/cache.rs @@ -98,8 +98,9 @@ impl TreeCache { /// identity/caption fields, `strategies_rev` (the strategy snapshot), 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, the UI-only folders, the selection, the staged -/// checkboxes, the selected folder, and the deleted-strategy revision. Folder/core bulk +/// active-only — the three expansion sets plus the Auto rail overlay, the UI-only +/// folders, the selection, the staged checkboxes, the selected folder, and the +/// deleted-strategy revision. Folder/core bulk /// boxes are derived from those staged values plus the strategy snapshot, not hashed as /// their own set. /// @@ -153,6 +154,7 @@ pub(crate) fn data_sig( view.filter.active_only.hash(&mut h); unordered(view.expanded_cores.iter()).hash(&mut h); + view.rail_expanded_core.hash(&mut h); unordered(view.expanded_folders.iter()).hash(&mut h); unordered(view.expanded_deleted.iter()).hash(&mut h); unordered(view.ui_folders.iter()).hash(&mut h); diff --git a/crates/moon-ui-gpui/src/strategies/tree/mod.rs b/crates/moon-ui-gpui/src/strategies/tree/mod.rs index de529d9d..37919f56 100644 --- a/crates/moon-ui-gpui/src/strategies/tree/mod.rs +++ b/crates/moon-ui-gpui/src/strategies/tree/mod.rs @@ -48,13 +48,20 @@ fn constrain_folder_drag_to_tree(tree: Div) -> Div { /// A process snapshot can hold expansion for cores that left the Auto rail while the window was /// closed. Counting those leftover ids as "expanded" would make a visibly collapsed tree run the /// collapse branch on the first click. +/// +/// The rail overlay counts as expanded too, or a freshly Auto-seeded window renders with a core +/// already open while the caret still points "expand" — the first click would then run the EXPAND +/// branch over an already-open tree instead of collapsing it. fn visible_tree_collapsed( expanded_cores: &HashSet, expanded_folders: &HashSet<(CoreId, String)>, + rail: Option, cores: &[(CoreId, String)], ) -> bool { cores.iter().all(|(core, _)| { - !expanded_cores.contains(core) && expanded_folders.iter().all(|(c, _)| c != core) + !expanded_cores.contains(core) + && rail != Some(*core) + && expanded_folders.iter().all(|(c, _)| c != core) }) } @@ -147,7 +154,12 @@ impl StrategiesView { }), }; - let collapsed = visible_tree_collapsed(&self.expanded_cores, &self.expanded_folders, cores); + let collapsed = visible_tree_collapsed( + &self.expanded_cores, + &self.expanded_folders, + self.rail_expanded_core, + cores, + ); let settings = self.settings_popover(super::settings::settings_trigger(self.settings_open), p, cx); @@ -227,6 +239,7 @@ impl StrategiesView { let coll = visible_tree_collapsed( &this.expanded_cores, &this.expanded_folders, + this.rail_expanded_core, &cores, ); this.expand_collapse_toggle(&cores, store, coll); diff --git a/crates/moon-ui-gpui/src/strategies/tree/moon.rs b/crates/moon-ui-gpui/src/strategies/tree/moon.rs index 559f0b5e..bbd3c5f3 100644 --- a/crates/moon-ui-gpui/src/strategies/tree/moon.rs +++ b/crates/moon-ui-gpui/src/strategies/tree/moon.rs @@ -100,6 +100,22 @@ const BADGE_TINY_MIN_W: f32 = 16.0; /// heading spends. const HEADING_GAP: f32 = 5.0; +/// Width a heading row reserves for its `active/total` counter, in design units. +/// +/// A MINIMUM rather than a fixed box: at the shipped text step this fits seven mono glyphs, which +/// covers every count a real account produces, so the column lines up across every row at every +/// tree depth. A count wide enough to exceed it grows its own slot and truncates nothing — a +/// counter that lies is worse than a column that bulges on one row. +const COUNTS_SLOT_W: f32 = 50.0; +/// Width a heading row reserves for the open-orders `(N)`, in design units. +/// +/// ALWAYS reserved, including on a row that currently has no open orders, so a core gaining or +/// losing them never shifts the `active/total` column left of it. +const ORDERS_SLOT_W: f32 = 34.0; +/// Gap between the two counter slots, in design units. Tighter than [`HEADING_GAP`] so the two +/// numbers read as one cluster rather than as two unrelated columns. +const COUNTS_GAP: f32 = 4.0; + /// Row height at the tree's local text step, in `Pixels`. /// /// The only caller of [`design::fit_h_px`] for a tree row — see [`ROW_H_BASE`] for why every row @@ -373,7 +389,12 @@ fn build_core_root( let cd = store.core(core)?; // Nothing below a collapsed core can render, so it needs only the totals in its own caption. // Search and reveal paths force their required core/folder chain open before this build runs. - let core_open = searching || view.expanded_cores.contains(&core); + // Direct field reads, not `state::core_is_open(...)`: the contract scanner + // (`the_tree_cache_signature_covers_every_input_the_build_reads`, in + // `tests/theme_contract/strategies.rs`) walks this function for `view.` reads and + // requires each one hashed in the tree signature, and an accessor would hide the second field. + let core_open = + searching || view.expanded_cores.contains(&core) || view.rail_expanded_core == Some(core); // One pass feeds both the visible set and every folder count. let mut counts = if core_open { @@ -902,11 +923,6 @@ fn render_row( checked, } => { let core = *core; - let txt = if *open_orders > 0 { - format!("{label} {active}/{total} ({open_orders})") - } else { - format!("{label} {active}/{total}") - }; core_folder_row( view, node_id, @@ -914,7 +930,8 @@ fn render_row( *selected, *checked, indent, - txt, + label.clone(), + RowCounts::subtree(*active, *total, *open_orders), p.blue, 600.0, ToggleTarget::Core(core), @@ -933,7 +950,6 @@ fn render_row( } => { let core = *core; let path = path.clone(); - let txt = format!("{label} {active}/{total}"); core_folder_row( view, node_id, @@ -941,7 +957,9 @@ fn render_row( *selected, *checked, indent, - txt, + 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, 400.0, ToggleTarget::Folder(core, path), @@ -985,7 +1003,8 @@ fn render_row( // Deleted addresses no folder, so `core_folder_row` draws it no checkbox at all. false, indent, - format!("{} {count}", rust_i18n::t!("strat.deleted_folder")), + rust_i18n::t!("strat.deleted_folder").to_string(), + RowCounts::deleted(*count), p.text_muted, 400.0, ToggleTarget::Deleted(core), @@ -1068,6 +1087,89 @@ fn exchange_row( .into_any_element() } +/// The trailing counter column of one heading row, already rendered to strings. +/// +/// The counters used to be concatenated onto the end of the caption, which put them at a different +/// x on every row and made a column of fifty cores read as noise. They are their own element now, +/// so the caption keeps the flexible truncating slot and the numbers keep a fixed one. +struct RowCounts { + /// Left slot: `active/total` for a core or folder, the bare count for the Deleted heading. + primary: String, + /// Right slot: the open-orders `(N)`, empty when the row has none. The slot is reserved either + /// way — see [`ORDERS_SLOT_W`]. + orders: String, + /// Localized tooltip naming exactly the numbers this row actually shows. + tip: SharedString, +} + +impl RowCounts { + /// Counters for a core or folder heading, whose numbers cover its whole subtree. + /// + /// Args: + /// active: Checked strategies under this heading, after the kind and side filters. + /// total: All strategies under it, after the same filters. + /// open_orders: Open orders of the whole core; always zero for a folder, which does not + /// carry an order count of its own. + /// + /// Returns: + /// The two slot strings plus the tooltip that names whichever of them is populated. + fn subtree(active: usize, total: usize, open_orders: usize) -> Self { + let counts_tip = rust_i18n::t!("strat.tree_counts_tip").to_string(); + Self { + primary: format!("{active}/{total}"), + orders: if open_orders > 0 { + format!("({open_orders})") + } else { + String::new() + }, + tip: SharedString::from(if open_orders > 0 { + format!( + "{counts_tip} · {}", + rust_i18n::t!("strat.tree_open_orders_tip") + ) + } else { + counts_tip + }), + } + } + + /// Counters for a core's Deleted heading, which carries one number and no orders. + fn deleted(count: usize) -> Self { + Self { + primary: count.to_string(), + orders: String::new(), + tip: SharedString::from(rust_i18n::t!("strat.tree_deleted_count_tip").to_string()), + } + } +} + +/// Render one right-aligned counter slot of a heading row's trailing column. +/// +/// Args: +/// text: The slot's number, or empty to reserve the width without drawing anything. +/// width: Minimum slot width in design units — [`COUNTS_SLOT_W`] or [`ORDERS_SLOT_W`]. +/// step: Local unscaled text-size step, so the number rides the row's own text size. +/// app: Application context used for palette and scaled geometry. +/// +/// Returns: +/// A `flex_none` slot whose content sits on its right edge. +fn counts_slot(text: String, width: f32, step: f32, app: &App) -> impl IntoElement { + let p = MoonPalette::active(app); + h_flex() + .flex_none() + .min_w(design::ui_px(app, width)) + .justify_end() + .child( + MoonText::new(text) + .mono(true) + .uppercase(false) + .color(p.text_muted) + .font_size(design::moon_text_base(app, step)) + .line_height(ROW_LINE_BASE + step) + .render(), + ) +} + enum ToggleTarget { Core(CoreId), Folder(CoreId, Vec), @@ -1102,7 +1204,8 @@ impl ToggleTarget { /// selected: Whether to draw the selected-folder highlight. /// checked: Summary of covered strategies; ignored by a row that addresses no folder. /// indent: Leading indentation for the tree depth. -/// text: Heading label and count summary. +/// label: Heading caption, counters excluded — they render in their own trailing column. +/// counts: The row's trailing counter column and its tooltip. /// color: Heading text color. /// weight: Heading font weight. /// target: Core, folder, or Deleted collection toggled by the row. @@ -1118,7 +1221,8 @@ fn core_folder_row( selected: bool, checked: bool, indent: Pixels, - text: String, + label: String, + counts: RowCounts, color: u32, weight: f32, target: ToggleTarget, @@ -1140,6 +1244,9 @@ fn core_folder_row( // Taken before the row consumes `row_id`: the checkbox derives its own element id from this // node's id for the same reason the row does — see the note on `.id(row_id)` below. let check_row_id = row_id.clone(); + // Same rule for the counter column, which needs an id of its own to carry a tooltip. Derived + // from the NODE id, never from the numbers it draws. + let counts_row_id = SharedString::from(format!("cnt:{row_id}")); let view_click = view.clone(); let view_menu = view.clone(); h_flex() @@ -1181,7 +1288,7 @@ fn core_folder_row( }) .child( div().flex_1().min_w_0().truncate().child( - MoonText::new(text) + MoonText::new(label) .mono(true) .uppercase(false) .color(color) @@ -1191,12 +1298,29 @@ fn core_folder_row( .render(), ), ) + // The counters, muted and right-aligned in fixed slots after the flexible caption, so they + // land on one column across every row instead of wherever each name happened to end. + // + // A tooltip gives this element a hitbox (fork `elements/div.rs`, `should_insert_hitbox`), + // but it keeps the default `HitboxBehavior::Normal`, which by contract "doesn't affect + // mouse handling for other hitboxes" — so unlike an interactive `MoonDisclosure::button` + // it cannot swallow the click that expands this row. + .child( + h_flex() + .id(counts_row_id) + .flex_none() + .items_center() + .gap(design::ui_px(app, COUNTS_GAP)) + .child(counts_slot(counts.primary, COUNTS_SLOT_W, step, app)) + .child(counts_slot(counts.orders, ORDERS_SLOT_W, step, app)) + .tooltip(crate::panels::common::text_tooltip(counts.tip)), + ) .on_click(move |_e, window, app| { view_click.update(app, |this, cx| { window.focus(&this.focus, cx); match &target { ToggleTarget::Core(c) => { - toggle(&mut this.expanded_cores, *c); + this.toggle_core_expanded(*c); this.selected_folder = Some((*c, String::new())); } ToggleTarget::Folder(c, path) => { diff --git a/crates/moon-ui-gpui/tests/theme_contract/analytics.rs b/crates/moon-ui-gpui/tests/theme_contract/analytics.rs index ae0c1c8f..6764eab1 100644 --- a/crates/moon-ui-gpui/tests/theme_contract/analytics.rs +++ b/crates/moon-ui-gpui/tests/theme_contract/analytics.rs @@ -2560,3 +2560,34 @@ fn the_run_column_stays_wired_and_marks_what_the_core_has_not_confirmed() { "an unconfirmed state must be drawn faded rather than hidden or recoloured" ); } + +/// Changing the maximum-drawdown policy to `Up` makes a deeper drawdown look green, while giving +/// duration either directional policy falsely presents a longer or shorter hold as good or bad. +#[test] +fn kpi_row_keeps_its_non_up_metric_policies() { + let summary = read_src("analytics/summary/mod.rs"); + let row = code_only(braced_body( + &summary, + "fn kpi_row(&self, d: &Summary, p: MoonPalette, cx: &Context)", + )); + + for (label, policy, consequence) in [ + ( + "t!(\"analytics.kpi.maxdd\")", + "DeltaGood::Down,", + "a deeper maximum drawdown would render as an improvement", + ), + ( + "t!(\"analytics.kpi.duration\")", + "DeltaGood::Neither,", + "a neutral duration change would claim a good or bad direction", + ), + ] { + chain_between(&row, label, policy, "KPI policy"); + assert!( + row.split_once(label) + .is_some_and(|(_, after_label)| after_label.contains(policy)), + "{label} must be followed by {policy} so {consequence}" + ); + } +} diff --git a/crates/moon-ui-gpui/tests/theme_contract/shell.rs b/crates/moon-ui-gpui/tests/theme_contract/shell.rs index b01b2245..4185a192 100644 --- a/crates/moon-ui-gpui/tests/theme_contract/shell.rs +++ b/crates/moon-ui-gpui/tests/theme_contract/shell.rs @@ -497,6 +497,11 @@ fn the_assets_wallets_header_caret_stays_passive() { /// The Assets wallet roster groups core rows by venue identity and resolves logos only after an /// off-thread prewarm, while every core keeps the trust-aware balance figure and click behavior. /// +/// Mutation: remove the cached automatic width from `bottom`, its render refill, its cache +/// invalidation, the name tooltip, or the non-shrinking figure wrapper. The pure width helper +/// would remain green while live Wallets either used a stale/default width or clipped a balance +/// into a misleading number beside a name the user could no longer recover on hover. +/// /// The roster has two shapes — grouped under exchange headings and flat, chosen by the section's /// persisted preference — and both must reach the SAME row builder: a second copy of the row would /// be the one that forgets the transfer-asset refresh a click owes the core it selects. So the @@ -546,9 +551,14 @@ fn assets_wallet_roster_reuses_canonical_exchange_sections_and_logos() { "crate::media::exchange_logos::exchange_logo", "img(logo)", "super::balances::figure(Some(agg), p, cx)", + ".id(SharedString::from(format!(\"asset-core-name-{cid}\")))", + ".tooltip(crate::panels::common::text_tooltip(core_name.clone()))", + ".flex_none()", ".on_click(cx.listener(move |this", "this.overview_wallet_pick = Some(cid)", "this.selected_core = Some(cid)", + ".cached_roster_auto_w", + "roster_width::resolved(&self.roster_widths.read(cx).column_widths, auto_w)", ".min_w(design::font_w_px(cx, roster_width::MIN_BASE_W))", ".flex_shrink_1()", ] { @@ -557,10 +567,32 @@ fn assets_wallet_roster_reuses_canonical_exchange_sections_and_logos() { "grouped Assets roster must retain {needle:?}" ); } + let figure_child = row + .find(".child(super::balances::figure(Some(agg), p, cx))") + .expect("wallet core rows must render a balance figure"); + let figure_cell = row[..figure_child] + .rsplit_once(".child(") + .expect("the balance figure must remain inside its own wrapper") + .1; + assert!( + figure_cell.contains(".flex_none()"), + "the wallet balance figure must stay in a non-shrinking wrapper" + ); assert!( bottom.contains("asset-exchange-unknown") && !bottom.contains("status_dot"), "unknown exchange headings stay explicit without a fake logo or status dot" ); + + let render = code_only(&read_src("panels/assets/render.rs")); + assert!( + render.contains("self.ensure_roster_auto_w(cx);"), + "Assets render must refill the automatic roster width before building Wallets" + ); + let cache = code_only(&read_src("panels/assets/cache.rs")); + assert!( + cache.contains("self.cached_roster_auto_w = None;"), + "Assets cache rebuild must invalidate the automatic roster width with its aggregates" + ); } /// The wallet roster's grouping preference must own exactly one optional layout key and one write @@ -1483,6 +1515,9 @@ fn the_main_tab_row_is_gated_and_addresses_charts_by_identity() { /// /// The contract is: both call sites opt into the overflow menu, the strip yields with /// `flex_1`/`min_w_0`, and neighbouring chrome is a flex sibling rather than an overlay. +/// The shared row also paints `shell_high` across that sibling and carries the strip's full-width +/// one-pixel bottom rule: deleting either builder during a layout refactor restores the dark +/// toolbar band or the visibly broken hairline at the strip/chrome seam. #[test] fn chart_tab_strips_are_in_flow_and_yield_to_chrome() { let strip = code_only(braced_body( @@ -1513,6 +1548,28 @@ fn chart_tab_strips_are_in_flow_and_yield_to_chrome() { strip.contains(".children(coin_dismiss)") && strip.contains(".children(fig_dismiss)"), "dismiss layers must stay on the chart body so they cannot cover the in-row cluster" ); + let tab_row = chain_between( + &strip, + "h_flex()\n .h(px(strip_h))", + ".child(right_cluster),", + "the shared chart tab row", + ); + assert!( + tab_row.contains(".bg(rgb(p_strip.shell_high))"), + "the shared tab row must paint shell_high behind the in-flow chrome cluster" + ); + let continued_rule = chain_between( + tab_row, + ".bg(rgb(p_strip.shell_high))", + ".child(div().flex_1().min_w_0().h_full().child(strip))", + "the chart tab row surface and strip slot", + ); + assert!( + continued_rule.contains( + ".child(\n div()\n .absolute()\n .left(px(0.0))\n .bottom(px(0.0))\n .w_full()\n .h(px(1.0))\n .bg(rgba_from(p_strip.border, 0.78))," + ), + "the row must continue the strip's full-width one-pixel bottom rule behind the chrome" + ); let row = code_only(braced_body( &read_src("chart_tabs/main_stack.rs"), diff --git a/crates/moon-ui-gpui/tests/theme_contract/strategies.rs b/crates/moon-ui-gpui/tests/theme_contract/strategies.rs index 4829f167..35b0d26e 100644 --- a/crates/moon-ui-gpui/tests/theme_contract/strategies.rs +++ b/crates/moon-ui-gpui/tests/theme_contract/strategies.rs @@ -399,9 +399,14 @@ fn tree_state_events_reassert_the_windows_own_expansion() { fn a_collapsed_core_skips_its_subtree_but_keeps_its_row() { let src = read_src("strategies/tree/moon.rs"); let body = braced_body(&src, "fn build_core_root("); + // Whitespace-stripped so a `core_open` binding rustfmt wraps across lines is found the same + // way as one written inline. + let packed: String = body.chars().filter(|c| !c.is_whitespace()).collect(); assert!( - body.contains("searching || view.expanded_cores.contains(&core)"), - "an open core must include the search-forced case" + packed.contains( + "searching||view.expanded_cores.contains(&core)||view.rail_expanded_core==Some(core)" + ), + "an open core must cover the search-forced case, hand expansion, AND the rail overlay" ); let call_at = body .find("build_core_subtree(") @@ -1128,16 +1133,44 @@ fn a_core_folder_row_marker_stays_passive() { ); } -/// `strategies/state.rs::StrategiesView::new` restores the snapshot then additively re-seeds -/// the Auto-selected core into that set. +/// `strategies/tree/moon.rs::core_folder_row` keeps its counter cluster passive. /// -/// Mutation: restore `Some(s) => s.expanded_cores.clone()` without the following -/// `seed_selected_core_into(...)` call, so a stored empty set is kept. After collapsing the -/// selected Auto core, close and reopen Strategies; the core stays collapsed and the user -/// re-finds the server by hand. +/// Mutation: add `.cursor_pointer()` to the `counts_row_id` cluster. That installs an +/// interactive hitbox over the counters, so a click on the rightmost numbers no longer reaches +/// the row handler that expands or collapses the core; clicking elsewhere on the same row still +/// works and makes the regression look flaky. +#[test] +fn a_core_folder_row_counter_cluster_stays_passive() { + let src = read_src("strategies/tree/moon.rs"); + let row = code_only(braced_body(&src, "fn core_folder_row(")); + let cluster = chain_between( + &row, + "h_flex()\n .id(counts_row_id)", + ".tooltip(crate::panels::common::text_tooltip(counts.tip))", + "core/folder counter cluster", + ); + + assert!( + cluster.contains(".flex_none()") + && cluster.contains(".child(counts_slot(counts.primary, COUNTS_SLOT_W, step, app))") + && cluster.contains(".child(counts_slot(counts.orders, ORDERS_SLOT_W, step, app))"), + "the identified counter cluster must retain both fixed counter slots" + ); + assert!( + !cluster.contains(".cursor_pointer()") + && !cluster.contains(".hover(") + && !cluster.contains(".on_click("), + "the counter cluster must remain passive so clicks on its numbers reach the row handler" + ); +} + +/// `strategies/state.rs::StrategiesView::new` restores the persisted set UNCHANGED and seeds the +/// Auto rail's selection into a separate overlay field, never into `expanded_cores` itself. /// -/// The construction-local `&mut expanded_cores` argument is what distinguishes restore-path -/// seeding from the live observer (`&mut this.expanded_cores`) inside the same function. +/// Mutation: replace the two-field initialisation with an insert of the seed into the restored +/// set (the pre-`48bd2266` shape: `expanded_cores.extend(rail_seed)` / +/// `expanded_cores.insert(core)`). Every rail-visited core would then stay unfolded in Auto +/// Overview for the whole process lifetime — the reported bug, in full. #[test] fn strategies_window_seeds_expansion_from_the_auto_workspace() { let ctor = code_only(&braced_body( @@ -1147,13 +1180,19 @@ fn strategies_window_seeds_expansion_from_the_auto_workspace() { assert!( ctor.contains("Some(s) => s.expanded_cores.clone()") && ctor.contains("None => HashSet::new()") - && ctor.contains("seed_selected_core_into(") - && ctor.contains("&mut expanded_cores,"), - "construction must restore the snapshot then additively seed the Auto-selected core" + && ctor.contains("rail_seed_core(") + && ctor.contains("rail_expanded_core"), + "construction must restore the persisted set as-is and seed the rail overlay separately" ); assert!( !ctor.contains("expanded_cores: HashSet::new()"), - "construction must not assign an empty expansion field, bypassing restore-and-seed" + "construction must not assign an empty expansion field, bypassing restore" + ); + assert!( + !ctor.contains("&mut expanded_cores") + && !ctor.contains("expanded_cores.insert(") + && !ctor.contains("expanded_cores.extend("), + "the rail seed must never be written into the persisted expansion set" ); } @@ -1163,9 +1202,12 @@ fn strategies_window_seeds_expansion_from_the_auto_workspace() { /// snapshot into `WindowLayout`. Closing the tool window would then forget expansion, selection, /// and filters, or a full application restart would incorrectly retain them. /// -/// A stored empty expansion is not a reason to skip Auto-rail re-seed: construction restores -/// the snapshot, then `seed_selected_core_into` additively inserts the selected core so the -/// user does not re-find the server by hand after collapsing it and reopening the window. +/// Construction restores the persisted set as-is and seeds the Auto rail's selection into the +/// separate `rail_expanded_core` overlay, never into `expanded_cores` itself; the live workspace +/// observer moves only that overlay too. `capture` must mirror the same split: it snapshots +/// `expanded_cores` alone, so a "fix" that captured the union of both fields would let a rail +/// seed outlive the window that received it and reappear as if the user had expanded it by +/// hand, in another scope or window — and no runtime test in the tree would notice. #[test] fn strategies_reopen_state_is_process_lifetime_only() { let root = Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); @@ -1203,8 +1245,7 @@ fn strategies_reopen_state_is_process_lifetime_only() { state.contains("let session = backend.read(cx).ui_session.strategies.clone();") && state.contains("Some(s) => s.expanded_cores.clone()") && state.contains("None => HashSet::new()") - && state.contains("seed_selected_core_into(") - && state.contains("&mut expanded_cores,") + && state.contains("rail_seed_core(") && state.contains("input.default_value(s.search.clone())") && state.contains("b.ui_session.strategies = Some(snapshot)") && state.contains("this.reconcile_ui_folders(this.backend.read(cx).session.store())") @@ -1223,9 +1264,23 @@ fn strategies_reopen_state_is_process_lifetime_only() { ); let observer = code_only(&braced_body(&state, "cx.observe(&workspace_revision,")); assert!( - observer.contains("seed_selected_core_into(") - && observer.contains("&mut this.expanded_cores,"), - "the live window's workspace observer must stay additive while the window is open" + observer.contains("this.rail_seen_core = rail") + && !observer.contains("this.expanded_cores"), + "the live window's workspace observer must move only the rail overlay, never the persisted set" + ); + let capture = code_only(&braced_body(&session, "pub(super) fn capture(")); + assert!( + capture.contains("expanded_cores: view.expanded_cores.clone()") + && !capture.contains("rail"), + "capture must snapshot the persisted set alone, never the rail overlay" + ); + let session_struct = code_only(&braced_body( + &session, + "pub(crate) struct StrategiesSessionState", + )); + assert!( + !session_struct.contains("rail"), + "StrategiesSessionState must never carry a rail overlay field" ); assert!( !ui_session.contains("Serialize") diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index b63ba2d2..2e61bf7d 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -403,6 +403,22 @@ unified USDT conversion remains complete-only, and a scope where not one trade r publishes no volume at all. A source without `sellreason` likewise cannot prove that a closed row is not Funding, so it contributes to completeness but cannot publish volume. +The footer states realized profit as a percentage of the average order only when the loaded +snapshot's own filter names exactly one core — averaging order sizes across cores would mix +universes with different typical sizes, so the fact is absent for an All or multi-core selection. +Its currency follows the head figure exactly: the one native bucket the scope carries, or a +unified USDT total, denominated and gated the same two ways the head promotes one — never a +figure in a currency the row does not otherwise show. The unified arm carries its OWN USDT leg +computed alongside the native one rather than reading the valuation cache's `UsdtTotal::spent`, +which admits any numeric spend with no positive-spend guard and no Funding exclusion; borrowing it +would average over a wider row set than the native arm while still claiming a complete count. The +average itself is over rows with a positive numeric settled spend, excluding Funding rows exactly +as the traded-volume leg does — the same definition Analytics' own average-order figure uses. Rows +the scope cannot account for (unknown quote, Funding, non-positive or non-numeric settled spend) +are excluded from both sums and stated as a count in the fact's tooltip, in either arm: even the +unified figure can be partial, because its own completeness is judged against the counted rows, +not the scope's full row count. + For a group-owned `AutoCore` Report only, `core_name` is contextually unavailable because every row already belongs to the selected core. This is a display lens, not a persistence mutation: the raw visible set, `app_meta`/`layout.toml`, sort state, and widths remain untouched. The grid, Columns diff --git a/locales/connections.yml b/locales/connections.yml index 99ddf3de..13abb313 100644 --- a/locales/connections.yml +++ b/locales/connections.yml @@ -76,6 +76,9 @@ conn.pending_cores: ru: "Новые ядра — настройте перед сохранением" en: "New cores — configure before saving" es: "Núcleos nuevos — configúralos antes de guardar" +# Shared by BOTH the group header and the pending/exchange subsection headings, where a count of +# 1..4 is ordinary -- so the abbreviated form stays: Russian has no single plural form that is +# right at 1, 2 and 56, and this file has no plural-selection convention to reach for. conn.member_count: ru: "%{n} ядр." en: "%{n} cores" @@ -244,6 +247,12 @@ conn.key_ph: ru: "вставьте ключ ядра" en: "paste the core key" es: "pega la clave del núcleo" +# Placeholder of the "Charts" cell. The field holds a bundle NAME, not a count (see +# `conn.tip.bundle` and `ServerConfig::chart_bundle`), so the placeholder has to say "name". +conn.bundle_ph: + ru: "имя связки" + en: "bundle name" + es: "nombre del conjunto" conn.paste_key: ru: "Вставить" en: "Paste" diff --git a/locales/dock.yml b/locales/dock.yml index dbf96cc7..5d6a7145 100644 --- a/locales/dock.yml +++ b/locales/dock.yml @@ -138,6 +138,21 @@ detects.cfg.vpos_below: ru: "Под графиком (клик — над)" en: "Below the chart (click — above)" es: "Debajo del gráfico (clic — encima)" +# What the feed states instead of a blank pane. Four different facts, four strings: only +# `empty_no_cores` asks the user to go and connect something, and the shared hidden-by-preset +# sentence (`workspace.scope.all_hidden`) outranks all three from `scope_empty_text`. +detects.empty: + ru: "Детектов пока нет — карточка появится, когда сработает ядро" + en: "No detects yet — a card appears as soon as a core fires one" + es: "Aún no hay detecciones: aparecerá una tarjeta cuando un núcleo active una" +detects.empty_filtered: + ru: "Детекты есть, но их скрывает текущая область просмотра" + en: "There are detects, but the current scope hides them" + es: "Hay detecciones, pero el ámbito actual las oculta" +detects.empty_no_cores: + ru: "В этой группе нет доступных ядер — детектам неоткуда прийти" + en: "No cores are available in this group — nothing can detect yet" + es: "No hay núcleos disponibles en este grupo: nada puede detectar aún" detects.field.none: ru: "—" en: "—" diff --git a/locales/orders.yml b/locales/orders.yml index 55a4b3e2..c49efbff 100644 --- a/locales/orders.yml +++ b/locales/orders.yml @@ -92,11 +92,29 @@ orders.only_current: ru: "Только ордера текущего маркета" en: "Only current market orders" es: "Solo órdenes del mercado actual" -# Bottom footer: total open orders, split into real and emulated. +# Bottom footer: total open orders, split into real and emulated. The split is SPELLED rather +# than parenthesised — `(923/100)` is unreadable without this file open — and the tooltip below +# repeats the same three numbers with what each one counts. +# +# LABEL: VALUE, never an inflected noun phrase. «реальных %{real}» reads wrong at a count of one +# («реальных 1»), and Russian and Spanish would each need their own plural rule to fix it; a +# colon-separated label is grammatical at every count in all three languages and needs none. The +# labels are the order-kind filter's own words (`orders.kind.real` / `.emu`) so the footer and the +# dropdown above it name the same two things identically. orders.footer_total: - ru: "Всего %{total} (%{real}/%{emu})" - en: "Total %{total} (%{real}/%{emu})" - es: "Total %{total} (%{real}/%{emu})" + ru: "Всего: %{total} · реальные: %{real} · эмуляторные: %{emu}" + en: "Total: %{total} · real: %{real} · emulated: %{emu}" + es: "Total: %{total} · reales: %{real} · emuladas: %{emu}" +# Tooltip of that footer. States the scope the counts are taken over, and that the order-kind +# filter above the table does not narrow them (the split is computed before that filter runs). +# +# The real line says «real account, not the emulator» and deliberately NOT «placed on the +# exchange»: the count is `!OrderRow::emulator` alone, and a real order that is still +# `OrderRow::pending` (`moon-core/src/feed/types.rs:250`) has not reached the exchange yet. +orders.footer_total_tip: + ru: "Всего: %{total} — открытые ордера выбранных ядер (и только текущего маркета, если этот фильтр включён).\nРеальные: %{real} — ордера реального счёта, не эмулятор.\nЭмуляторные: %{emu} — ордера эмулятора Moonbot.\nФильтр «Все / Реальные / Эмуляторные» на эти счётчики не влияет." + en: "Total: %{total} — open orders of the selected cores (and of the current market only, when that filter is on).\nReal: %{real} — orders on the real account, not the emulator.\nEmulated: %{emu} — Moonbot emulator orders.\nThe All / Real / Emulator filter does not change these counts." + es: "Total: %{total} — órdenes abiertas de los núcleos seleccionados (y solo del mercado actual, si ese filtro está activo).\nReales: %{real} — órdenes de la cuenta real, no del emulador.\nEmuladas: %{emu} — órdenes del emulador de Moonbot.\nEl filtro Todas / Reales / Emulador no cambia estos recuentos." orders.show_all: ru: "Показать все" en: "Show all" diff --git a/locales/report.yml b/locales/report.yml index e8aa44ea..e6861031 100644 --- a/locales/report.yml +++ b/locales/report.yml @@ -152,7 +152,7 @@ report.selection.restore_queued: ru: "Поставлено на восстановление: %{n}. Выбор снимется после ответа ядра." en: "Queued for restore: %{n}. Selection clears after the core response." es: "En cola para restaurar: %{n}. La selección se borra tras la respuesta del núcleo." -# The ▦ glyph is added separately in code; the dictionary contains text only. +# The trigger draws a MoonUI icon asset, added in code; the dictionary contains text only. report.columns_menu: ru: "Колонки" en: "Columns" @@ -268,6 +268,36 @@ report.traded_volume_unknown_quote: ru: "валюта неизвестна" en: "unknown quote" es: "moneda desconocida" +# Realized profit as a percentage of the average order, single-core scope only. %{pct} arrives +# pre-signed from signed_pct, e.g. "+11.6%". +report.avg_order_pct: + ru: "%{pct} от ср. ордера" + en: "%{pct} of avg order" + es: "%{pct} de la orden media" +# Some counted rows were excluded from both sums (unknown quote, Funding, non-positive spend). +report.avg_order_pct_partial: + ru: "%{pct} от ср. ордера (частично)" + en: "%{pct} of avg order (partial)" + es: "%{pct} de la orden media (parcial)" +# The count is stated as a LABEL ("orders counted: N"), never as a noun the number must agree +# with, so it reads correctly at n=1 in every locale without a plural form - the same shape +# report.traded_volume_partial_tip already uses for its shortfall. +report.avg_order_pct_tip: + ru: "Прибыль к среднему ордеру: %{pct}. Средний ордер: %{avg}, ордеров учтено: %{n}" + en: "Profit to average order: %{pct}. Average order: %{avg}, orders counted: %{n}" + es: "Beneficio sobre la orden media: %{pct}. Orden media: %{avg}, órdenes contadas: %{n}" +report.avg_order_pct_partial_tip: + ru: "Прибыль к среднему ордеру: %{pct}. Средний ордер: %{avg}, ордеров учтено: %{n}; не учтено: %{excluded}" + en: "Profit to average order: %{pct}. Average order: %{avg}, orders counted: %{n}; not accounted for: %{excluded}" + es: "Beneficio sobre la orden media: %{pct}. Orden media: %{avg}, órdenes contadas: %{n}; no contabilizadas: %{excluded}" +# The unified arm under ValuationMode::Current only. A unified scope CAN still be partial - Funding, +# non-positive-spend and unknown-quote rows shrink the counted scope independently of valuation - and +# in that case the partial wording above wins and this key is not reached, because the shortfall is +# the more important signal. +report.avg_order_pct_current_tip: + ru: "Прибыль к среднему ордеру по текущему курсу: %{pct}. Средний ордер: %{avg}, ордеров учтено: %{n}" + en: "Profit to average order at the current rate: %{pct}. Average order: %{avg}, orders counted: %{n}" + es: "Beneficio sobre la orden media a la tasa actual: %{pct}. Orden media: %{avg}, órdenes contadas: %{n}" report.unknown_quote_orders: ru: "валюта неизвестна: %{n}" en: "unknown quote: %{n}" diff --git a/locales/strategies.yml b/locales/strategies.yml index c5b577c1..ffb711b2 100644 --- a/locales/strategies.yml +++ b/locales/strategies.yml @@ -325,6 +325,18 @@ strat.deleted_folder: ru: "Удалённые" en: "Deleted" es: "Eliminadas" +strat.tree_counts_tip: + ru: "Включённых стратегий из общего числа в этой ветке (с учётом фильтров по типу и стороне)" + en: "Enabled strategies out of the total in this branch (kind and side filters applied)" + es: "Estrategias activadas del total en esta rama (con los filtros de tipo y lado aplicados)" +strat.tree_open_orders_tip: + ru: "В скобках — открытые ордера ядра" + en: "In brackets — the core's open orders" + es: "Entre paréntesis — las órdenes abiertas del núcleo" +strat.tree_deleted_count_tip: + ru: "Сколько удалённых на сервере, но сохранённых локально стратегий показано в этой папке" + en: "How many server-deleted, locally kept strategies this folder is showing" + es: "Cuántas estrategias eliminadas en el servidor y conservadas localmente muestra esta carpeta" strat.menu_restore: ru: "Восстановить" en: "Restore"