From ad947a0906748f76f9ed5793bd69fe848e4cb9b0 Mon Sep 17 00:00:00 2001 From: kirillDevPro <113171057+kirillDevPro@users.noreply.github.com> Date: Tue, 22 Sep 2026 19:53:27 +0200 Subject: [PATCH] fix(tuner): keep a running composition across a report-axis move The tuner's field-set composition died a few minutes in, without an error, a summary line or a "stopped" caption: the progress and spinner vanished and the button looked never pressed. The Analytics window's report-axis observer retires every in-flight read identity when the axis changes - and it also invalidated the tuner, which cancelled the running search and reset its state to Idle. The axis moves on events unrelated to the search (a core's feed respawning, a time-offset bucket change, a newly connected core), and each later beam depth takes longer than the last, so the kill almost always landed around step 5-6. The observer now takes an axis-only path: read identities and drafts are still retired, but a live composition is kept, and its finished result is marked "report time axis shifted" in a warning tone. The run stays valid - its rows are snapshotted at start, and the shift is at most a 15-minute offset bucket against a weeks-long fitting window; a report generation advance, a larger change, already does not cancel it. The search never held a read-cancellation token, so keeping it raises no SQLite interrupt and no false read failure. A scope, period or field change still stops the search as before. --- crates/moon-ui-gpui/src/analytics/mod.rs | 24 +- .../src/analytics/tuner/filter/actions.rs | 11 +- .../analytics/tuner/filter/actions/tests.rs | 46 +++ .../src/analytics/tuner/filter/state.rs | 99 +++++- .../src/analytics/tuner/filter/state/tests.rs | 288 ++++++++++++++++++ .../moon-ui-gpui/src/analytics/tuner/shell.rs | 25 +- .../src/analytics/tuner/shell/tests.rs | 123 +++++++- .../tests/theme_contract/analytics.rs | 20 +- locales/analytics.yml | 4 + 9 files changed, 616 insertions(+), 24 deletions(-) diff --git a/crates/moon-ui-gpui/src/analytics/mod.rs b/crates/moon-ui-gpui/src/analytics/mod.rs index e6410bfea..801bd61b8 100644 --- a/crates/moon-ui-gpui/src/analytics/mod.rs +++ b/crates/moon-ui-gpui/src/analytics/mod.rs @@ -1135,15 +1135,25 @@ impl AnalyticsView { /// every period bound moves. But the SCOPE (period, filters) does not, so this is a /// writer-driven catch-up, not a user reload: the visible snapshot stays on screen, with no /// blocking overlay, until the replacement lands. The observer retires EVERY in-flight read - /// identity for the old axis — `seq`, `cal_seq`, `cancel_latest_reads`, plus `tuner`, - /// `time_tuner`, `coins` and `coin_lists` `invalidate()` for the axes that keep their own - /// request generations — because a cancelled read is not silently dropped: the DB layer - /// raises a real SQLite interrupt that gets classified as a durable `Settled` failure, so a - /// read whose identity was not retired would pass its own `seq != req` guard and publish that - /// failure as if it were a real result. Retiring the tuner's identity here also clears any + /// identity for the old axis — `seq`, `cal_seq`, `cancel_latest_reads`, plus `time_tuner`, + /// `coins` and `coin_lists` `invalidate()` for the axes that keep their own request + /// generations — because a cancelled read is not silently dropped: the DB layer raises a + /// real SQLite interrupt that gets classified as a durable `Settled` failure, so a read + /// whose identity was not retired would pass its own `seq != req` guard and publish that + /// failure as if it were a real result. Retiring the tuner's read identities still clears any /// unsaved filter draft, matching this observer's behavior before it stopped calling /// `reload()` for axis changes. /// + /// The "By filter" joint suggestion is the one exception. It runs through `spawn_db`, which + /// installs no read-cancellation token, so it is not among the lanes `cancel_latest_reads` + /// retires — no interrupt can reach it, and therefore no fake `Settled` can be published for + /// it. A live joint run finishes on the axis it started on, and its result is captioned as + /// fitted across the move. A minutes-long composition is the most expensive thing this window + /// does, and a report generation advance — a strictly larger change — already does not retire + /// it (`TunerState::mark_report_stale`). `TunerState::invalidate_for_axis` is that path. With + /// no joint run live the tuner is invalidated exactly as before: drafts cleared, every + /// identity retired. + /// /// Args: /// cx: Analytics window context used to schedule a catch-up only when the axis moved. /// @@ -1159,7 +1169,7 @@ impl AnalyticsView { self.seq = self.seq.wrapping_add(1); self.cal_seq = self.cal_seq.wrapping_add(1); self.cancel_latest_reads(); - self.tuner.invalidate(); + self.tuner.invalidate_for_axis(); self.time_tuner.invalidate(); self.coins.invalidate(); self.coin_lists.invalidate(); diff --git a/crates/moon-ui-gpui/src/analytics/tuner/filter/actions.rs b/crates/moon-ui-gpui/src/analytics/tuner/filter/actions.rs index b9efb9596..ff1396e61 100644 --- a/crates/moon-ui-gpui/src/analytics/tuner/filter/actions.rs +++ b/crates/moon-ui-gpui/src/analytics/tuner/filter/actions.rs @@ -82,11 +82,13 @@ impl AnalyticsView { self.tuner.sugg = SuggestState::Running(if compose { SuggestJob::Compose { handle: handle.clone(), + axis_moved: false, } } else { SuggestJob::AllFields { handle: handle.clone(), total: restarts, + axis_moved: false, } }); self.poll_suggest_progress(handle.clone(), cx); @@ -123,6 +125,9 @@ impl AnalyticsView { // comparing it against ONE run's restart count would call almost every completed // composition a stop and refuse to offer its seed. let stopped = handle.abandoned(); + // Copied off the live job before it is replaced. A failed read fits nothing and + // stays unmarked. + let axis_moved = this.tuner.sugg.axis_moved(); let found = match sugg { Ok(found) => found, // A failed read must not look like "found nothing": the button would just @@ -166,6 +171,7 @@ impl AnalyticsView { composed: res.composed.clone(), compose_skipped: res.compose_skipped, }), + axis_moved, }; // Offer the seed for pinning only after a COMPLETE run. A stopped search finishes // an arbitrary subset of restart indices, not the first N, so rerunning its seed @@ -208,11 +214,12 @@ impl AnalyticsView { ); log::info!( "analytics: smart suggestion — in sample {:+.2} over {}, \ - out of sample {holdout}, restarts {completed}, seed {}{decision}{}", + out of sample {holdout}, restarts {completed}, seed {}{decision}{}{}", res.train.profit, res.train.n, res.seed, - if stopped { " (stopped)" } else { "" } + if stopped { " (stopped)" } else { "" }, + if axis_moved { " (axis moved)" } else { "" } ); let by_field: HashMap<&str, _> = res.fields.into_iter().map(|f| (f.field, f)).collect(); diff --git a/crates/moon-ui-gpui/src/analytics/tuner/filter/actions/tests.rs b/crates/moon-ui-gpui/src/analytics/tuner/filter/actions/tests.rs index eca82f7d8..39c114c0f 100644 --- a/crates/moon-ui-gpui/src/analytics/tuner/filter/actions/tests.rs +++ b/crates/moon-ui-gpui/src/analytics/tuner/filter/actions/tests.rs @@ -11,3 +11,49 @@ fn analyzer_stamp_follows_the_selected_display_zone() { "25.07.2026 10:26:50 (Save from analyzer)" ); } + +/// `filter/actions.rs:suggest_into_v1` must copy the live axis mark onto `SuggestState::Done`. +/// +/// The completion closure is the callback `spawn_db` runs after the database worker returns. +/// Driving that closure needs an `AnalyticsView` and a GPUI context, which this binary crate's +/// unit tests do not construct. The gap is that this test does not execute the closure. It pins +/// the copy step itself: the value read from `SuggestState::axis_moved` is the value stored on +/// `Done`. +/// +/// Breakage: `let axis_moved = this.tuner.sugg.axis_moved();` becoming `let axis_moved = false`, +/// or the `Done` literal hardcoding `axis_moved: false`. The run finishes and the status band +/// never says the report time axis shifted, so the user treats a fit from the old axis as current. +#[test] +fn a_finished_joint_search_copies_the_axis_move_onto_the_result() { + let source = include_str!("../actions.rs").replace("\r\n", "\n"); + let body = source + .split_once("fn suggest_into_v1(") + .expect("suggest_into_v1") + .1 + .split("\n }\n") + .next() + .expect("suggest_into_v1 body"); + let completion = body + .split_once("move |this, sugg, cx|") + .expect("suggestion completion closure") + .1; + assert!( + completion.contains("let axis_moved = this.tuner.sugg.axis_moved();"), + "the completion must copy SuggestState::axis_moved, not a constant" + ); + let done = completion + .split_once("SuggestState::Done {") + .expect("Done construction") + .1 + .split_once("\n };") + .expect("Done construction close") + .0; + assert!( + done.contains("\n axis_moved,"), + "Done must store the copied binding" + ); + assert!( + !done.contains("axis_moved: false") && !done.contains("axis_moved: true"), + "hardcoding the mark on Done drops the copy" + ); +} diff --git a/crates/moon-ui-gpui/src/analytics/tuner/filter/state.rs b/crates/moon-ui-gpui/src/analytics/tuner/filter/state.rs index d762f8c63..4f347c61f 100644 --- a/crates/moon-ui-gpui/src/analytics/tuner/filter/state.rs +++ b/crates/moon-ui-gpui/src/analytics/tuner/filter/state.rs @@ -243,6 +243,10 @@ pub(in crate::analytics::tuner) enum SuggestState { /// On an uninterrupted search, no answer means the scope held fewer trades than the /// minimum demanded of a suggestion. split: Option, + /// The report axis was re-adopted while this run was live; copied off the job. + /// + /// Why the run is kept: [`TunerState::note_axis_moved`]. + axis_moved: bool, }, /// The last search could not read the report. Failed(ReadFail), @@ -302,6 +306,10 @@ pub(in crate::analytics::tuner) enum SuggestJob { handle: SearchHandle, /// Restarts requested for the run. total: usize, + /// The report axis was re-adopted while this run was live. + /// + /// Why the run is kept: [`TunerState::note_axis_moved`]. + axis_moved: bool, }, /// Composition: many searches behind one handle, choosing the field set before refitting it. /// @@ -311,6 +319,10 @@ pub(in crate::analytics::tuner) enum SuggestJob { Compose { /// Cancellation and progress for this run. handle: SearchHandle, + /// The report axis was re-adopted while this run was live. + /// + /// Why the run is kept: [`TunerState::note_axis_moved`]. + axis_moved: bool, }, } @@ -327,13 +339,29 @@ impl SuggestState { /// a sequence of them whose length depends on what it finds. pub(in crate::analytics::tuner) fn joint_run(&self) -> Option<(&SearchHandle, Option)> { match self { - SuggestState::Running(SuggestJob::AllFields { handle, total }) => { + SuggestState::Running(SuggestJob::AllFields { handle, total, .. }) => { Some((handle, Some(*total))) } - SuggestState::Running(SuggestJob::Compose { handle }) => Some((handle, None)), + SuggestState::Running(SuggestJob::Compose { handle, .. }) => Some((handle, None)), _ => None, } } + + /// Whether the report axis was re-adopted while this run was live. + /// + /// True only when a joint run, or the result copied off one, carries the mark. Idle, a + /// failed read and a single-field sweep have nothing that can say so. + /// + /// Returns: + /// Whether the carried mark is set. + pub(in crate::analytics::tuner) fn axis_moved(&self) -> bool { + match self { + SuggestState::Running(SuggestJob::AllFields { axis_moved, .. }) + | SuggestState::Running(SuggestJob::Compose { axis_moved, .. }) + | SuggestState::Done { axis_moved, .. } => *axis_moved, + _ => false, + } + } } /// Mutable state of the "By filter" tuner inside `AnalyticsView`. @@ -485,21 +513,53 @@ impl TunerState { /// /// Current data remains until recomputation completes to avoid a loading /// flash; a completed non-data result clears it. Recompute on mode entry or - /// an explicit reload. + /// an explicit reload. It is the user-driven query-changed path, where cancelling the + /// running search is correct because the query genuinely differs — which is why the axis + /// has its own path below. /// /// The method has no return value; callers start or defer replacement reads. pub(in crate::analytics) fn invalidate(&mut self) { + self.retire_reads_and_drafts(); + self.invalidate_suggest(); + } + + /// Drop KPI and histogram read identities and unsaved filter drafts. + /// + /// Shared by [`Self::invalidate`] and [`Self::invalidate_for_axis`]. The suggestion + /// generation stays put here: cancelling a search is the caller's decision. + /// + /// The method has no return value. + fn retire_reads_and_drafts(&mut self) { self.dirty = true; self.hist_dirty = true; self.seq = self.seq.wrapping_add(1); self.hist_seq = self.hist_seq.wrapping_add(1); self.hist_loading = false; - self.invalidate_suggest(); self.mark_dialog_draft_changed(); self.save_dialog = None; self.staged_ignore.clear(); } + /// Retire read identities after a report-axis adoption, keeping a live joint search. + /// + /// The axis is exempt from cancelling that search. The suggest search runs through + /// `spawn_db`, which installs no read-cancellation token, so it is not one of the lanes + /// `cancel_latest_reads` cancels — no interrupt, therefore no fake `Settled`. Why the run + /// is kept is [`Self::note_axis_moved`]. [`SuggestJob::SingleField`] + /// is deliberately not kept: a sub-second sweep under the blocking overlay, with no caption + /// that could carry the mark, so keeping it would break "kept implies marked". With no live + /// joint run this falls through to [`Self::invalidate`]. + /// + /// On the keep path the method never advances `sugg_seq` and never replaces `sugg` beyond + /// the mark [`Self::note_axis_moved`] sets. + pub(in crate::analytics) fn invalidate_for_axis(&mut self) { + if !self.note_axis_moved() { + self.invalidate(); + return; + } + self.retire_reads_and_drafts(); + } + /// Retire asynchronous Save-dialog preparation after a user scope or draft change. pub(in crate::analytics) fn mark_dialog_draft_changed(&mut self) { self.dialog_seq = self.dialog_seq.wrapping_add(1); @@ -566,12 +626,41 @@ impl TunerState { } } + /// Record that the report axis was re-adopted under a live joint search. + /// + /// The mark lives on the job, and is copied onto the result, rather than in a field of its + /// own on [`TunerState`], so it cannot outlive what it describes — the same reasoning + /// [`Self::compose_support`] gives. The run is kept: its query bounds were snapshotted when + /// the search started, and the rows of that in-memory sample do not change while it runs; + /// the axis shift is a core-time-offset nudge at 15-minute bucket granularity against a + /// fitting window of weeks to months; and [`Self::mark_report_stale`] already decided that + /// a committed report row — a strictly larger change — must not retire a manually started + /// search. + /// + /// Only [`SuggestJob::AllFields`] and [`SuggestJob::Compose`] can carry the mark. Idle, a + /// finished result, a failure and a single-field sweep are left unchanged. + /// + /// Returns: + /// Whether a live joint run was there to mark. + pub(in crate::analytics::tuner) fn note_axis_moved(&mut self) -> bool { + match &mut self.sugg { + SuggestState::Running(SuggestJob::AllFields { axis_moved, .. }) + | SuggestState::Running(SuggestJob::Compose { axis_moved, .. }) => { + *axis_moved = true; + true + } + _ => false, + } + } + /// Mark KPI and histogram calculations stale while preserving tuner drafts and suggestions. /// /// Report generations can advance throughout a minutes-long field-set composition. Retiring /// the suggestion on every advance could repeatedly cancel a manually started search before /// it finishes, so search-input and destination changes invalidate it through - /// [`Self::invalidate`] or [`Self::invalidate_suggest`] instead. + /// [`Self::invalidate`] or [`Self::invalidate_suggest`] instead. A report-axis adoption is + /// the third route, [`Self::invalidate_for_axis`]: it keeps a live joint run and otherwise + /// invalidates exactly as [`Self::invalidate`] does. pub(in crate::analytics) fn mark_report_stale(&mut self) { self.dirty = true; self.hist_dirty = true; diff --git a/crates/moon-ui-gpui/src/analytics/tuner/filter/state/tests.rs b/crates/moon-ui-gpui/src/analytics/tuner/filter/state/tests.rs index bc2ce4752..0e81ef41e 100644 --- a/crates/moon-ui-gpui/src/analytics/tuner/filter/state/tests.rs +++ b/crates/moon-ui-gpui/src/analytics/tuner/filter/state/tests.rs @@ -1,5 +1,7 @@ //! Unit tests for persisted filter-tuner controls. +use std::sync::Arc; + use super::{ DEFAULT_EDGES, DEFAULT_ITERS, DEFAULT_TRAIN, SuggestJob, SuggestState, TRAIN_OPTIONS, TunerState, canonical_iters, edge_options, edge_options_upto, fmt_bound, iters_of, parse_num, @@ -136,6 +138,7 @@ fn report_staleness_preserves_filter_drafts_and_the_running_search() { let handle = SearchHandle::new(); state.sugg = SuggestState::Running(SuggestJob::Compose { handle: handle.clone(), + axis_moved: false, }); let (seq, hist_seq, sugg_seq, dialog_seq) = (state.seq, state.hist_seq, state.sugg_seq, state.dialog_seq); @@ -199,6 +202,7 @@ fn retiring_a_suggestion_stops_the_search_behind_it() { state.sugg = SuggestState::Running(SuggestJob::AllFields { handle: handle.clone(), total: 100, + axis_moved: false, }); state.invalidate_suggest(); @@ -670,3 +674,287 @@ fn a_field_missing_from_the_saved_list_opens_unchecked() { "a mapped field absent from the saved list must stay unchecked" ); } + +/// `filter/state.rs:TunerState::invalidate_for_axis` must keep a live joint search and mark it. +/// +/// Breakage: the method, or `analytics/mod.rs:observe_report_axis`, going back to +/// `invalidate_suggest()`. The `SearchHandle` is cancelled, `sugg_seq` advances, and the +/// completion guard drops the result. A minutes-long "Pick the set" run then vanishes with no +/// error and no caption. +#[test] +fn an_axis_move_keeps_a_live_joint_search_and_marks_it() { + for compose in [true, false] { + let (job, handle) = joint_job(compose); + let mut state = prepared_axis_state(job); + let (seq, hist_seq, sugg_seq, dialog_seq) = + (state.seq, state.hist_seq, state.sugg_seq, state.dialog_seq); + assert!( + !state.needs_reload(), + "precondition: settled reads, so reload demand has to come from this call" + ); + assert!( + !state.staged_ignore.is_empty() && state.save_dialog.is_some(), + "precondition: a draft and a save dialog are open" + ); + + state.invalidate_for_axis(); + + assert!( + !handle.is_cancelled(), + "a report-axis move must not cancel a live joint search" + ); + assert!( + state.sugg.is_running(), + "the row must keep reporting that search as running" + ); + assert_eq!( + state.sugg_seq, sugg_seq, + "the search's own result must still be publishable when it lands" + ); + assert!( + state.sugg.axis_moved(), + "the kept run must carry the mark the caption reads" + ); + assert_ne!(state.seq, seq, "KPI identity must still retire"); + assert_ne!( + state.hist_seq, hist_seq, + "histogram identity must still retire" + ); + assert_ne!( + state.dialog_seq, dialog_seq, + "a pending Save dialog must no longer match the retired draft" + ); + assert!( + state.staged_ignore.is_empty(), + "unsaved ignore edits belong to the retired draft" + ); + assert!( + state.save_dialog.is_none(), + "the open Save dialog belongs to the retired draft" + ); + assert!( + state.needs_reload(), + "retired KPI and histogram reads must be reloadable" + ); + } +} + +/// `filter/state.rs:TunerState::invalidate_for_axis` falls through to `invalidate` when nothing +/// joint is running. +/// +/// Breakage: treating `SuggestJob::SingleField`, `Idle`, or a finished `Done` like a joint run +/// and leaving `sugg_seq` alone. The single-field sweep has no caption that can carry the mark, +/// and a finished result fitted on the old axis would stay on screen as if it still applied. +/// The "SingleField is deliberately not kept" rule was previously only a docstring. +#[test] +fn an_axis_move_with_no_joint_run_is_a_plain_invalidation() { + for label in ["idle", "done", "single-field"] { + let mut state = TunerState::load(None, None, None, None, None, false); + state.sugg = match label { + "idle" => SuggestState::Idle, + "done" => SuggestState::Done { + work: super::SuggestWork::Plain { completed: 2 }, + stopped: false, + split: None, + axis_moved: false, + }, + "single-field" => SuggestState::Running(SuggestJob::SingleField), + _ => unreachable!(), + }; + let sugg_seq = state.sugg_seq; + + state.invalidate_for_axis(); + + assert_ne!( + state.sugg_seq, sugg_seq, + "{label}: with no live joint run the suggestion generation must retire" + ); + assert!( + !state.sugg.is_running(), + "{label}: with no live joint run the row must not stay running" + ); + } +} + +/// `filter/state.rs:TunerState::invalidate` must still cancel a live joint search. +/// +/// Breakage: extracting `retire_reads_and_drafts` and dropping the `invalidate_suggest()` call +/// from `invalidate`. A scope change would leave the old search running and let its result +/// publish into the new scope. +#[test] +fn a_scope_change_still_stops_a_live_search() { + let mut state = TunerState::load(None, None, None, None, None, false); + let (job, handle) = joint_job(false); + state.sugg = SuggestState::Running(job); + + state.invalidate(); + + assert!( + handle.is_cancelled(), + "a scope change must still tell the running search to stop" + ); + assert!( + !state.sugg.is_running(), + "and the row must no longer read as running" + ); +} + +/// Calling `invalidate_for_axis` again on the same live joint run must not undo the mark. +/// +/// `analytics/mod.rs:observe_report_axis` fires on every report generation while a minutes-long +/// composition is in flight. Breakage: the mark as a toggle or a counter, or the second call +/// falling through to `invalidate_suggest()`. The handle would cancel, or the caption would +/// disappear, halfway through a run the user is still watching. +#[test] +fn a_repeated_axis_move_keeps_the_mark_and_does_not_cancel() { + for compose in [true, false] { + let (job, handle) = joint_job(compose); + let mut state = TunerState::load(None, None, None, None, None, false); + state.sugg = SuggestState::Running(job); + let sugg_seq = state.sugg_seq; + + state.invalidate_for_axis(); + let sugg_seq_after_first = state.sugg_seq; + state.invalidate_for_axis(); + + assert!( + state.sugg.axis_moved(), + "a second observation must leave the mark set" + ); + assert!( + !handle.is_cancelled(), + "a second observation must not cancel the search" + ); + assert_eq!( + sugg_seq_after_first, sugg_seq, + "the first observation must not advance sugg_seq" + ); + assert_eq!( + state.sugg_seq, sugg_seq, + "the second observation must not advance sugg_seq either" + ); + assert!(state.sugg.is_running()); + } +} + +/// Every `sugg_seq` advance must settle `sugg` in the same function, and an axis move must not +/// be one of those advances. +/// +/// Breakage: a new site that does `sugg_seq.wrapping_add(1)` without assigning +/// `SuggestState`. The completion closures at `filter/actions.rs` return when +/// `sugg_seq` no longer matches, so the result is dropped and whatever state was on screen +/// stays there. `invalidate_for_axis` deliberately does not take that route: there is no new +/// runtime guard. `suggest_into_v1` must also start both joint jobs with `axis_moved: false` +/// exactly twice — a third literal false is a copied mark being thrown away at launch. +#[test] +fn every_suggest_generation_advance_settles_the_state() { + let state_src = code_lines(include_str!("../state.rs")); + let actions_src = code_lines(include_str!("../actions.rs")); + let mut sites = 0usize; + for (label, source) in [("state.rs", &state_src), ("actions.rs", &actions_src)] { + for chunk in source.split("\n }\n") { + if !chunk.contains("sugg_seq.wrapping_add(1)") { + continue; + } + sites += 1; + assert!( + chunk.contains("sugg = SuggestState::"), + "{label} advances sugg_seq without settling sugg; the completion would be dropped" + ); + } + } + assert!( + sites >= 3, + "the three existing suggest-generation advances must still be visible, saw {sites}" + ); + + let axis = state_src + .split_once("fn invalidate_for_axis(") + .expect("invalidate_for_axis") + .1 + .split("\n }\n") + .next() + .expect("invalidate_for_axis body"); + assert!( + !axis.contains("sugg_seq"), + "invalidate_for_axis must not advance sugg_seq" + ); + + let suggest = actions_src + .split_once("fn suggest_into_v1(") + .expect("suggest_into_v1") + .1 + .split("\n }\n") + .next() + .expect("suggest_into_v1 body"); + assert_eq!( + suggest.matches("axis_moved: false").count(), + 2, + "suggest_into_v1 must start both joint jobs unmarked and hardcode false nowhere else" + ); +} + +/// Strip comments so a substring ban cannot be satisfied by the prose that names it. +fn code_lines(source: &str) -> String { + source + .replace("\r\n", "\n") + .lines() + .map(|line| match line.find("//") { + Some(at) => &line[..at], + None => line, + }) + .collect::>() + .join("\n") +} + +fn joint_job(compose: bool) -> (SuggestJob, SearchHandle) { + let handle = SearchHandle::new(); + let job = if compose { + SuggestJob::Compose { + handle: handle.clone(), + axis_moved: false, + } + } else { + SuggestJob::AllFields { + handle: handle.clone(), + total: 100, + axis_moved: false, + } + }; + (job, handle) +} + +/// Settled reads plus an open draft, so retirement assertions are transitions. +fn prepared_axis_state(job: SuggestJob) -> TunerState { + let mut state = TunerState::load(None, None, None, None, None, false); + state.stats.apply(Ok(Vec::new())); + state.dirty = false; + state.hist_dirty = false; + state.staged_ignore.insert("IgnoreFilters", true); + state.save_dialog = Some(parked_save_dialog()); + state.sugg = SuggestState::Running(job); + state +} + +fn parked_save_dialog() -> Arc { + use super::super::super::shared::{SaveAuthority, SaveDialog, SaveTarget}; + Arc::new(SaveDialog { + authority: SaveAuthority { + dialog_seq: 1, + workspace_generation: None, + workspace_cores: None, + targets: Vec::new(), + }, + targets: vec![SaveTarget { + sid: 1, + core: None, + name: "anchor".into(), + }], + changes: vec![("bound".into(), "1".into())], + olds: vec![None], + copy: false, + warns: Vec::new(), + per_target: None, + notes: vec![None], + }) +} diff --git a/crates/moon-ui-gpui/src/analytics/tuner/shell.rs b/crates/moon-ui-gpui/src/analytics/tuner/shell.rs index af52166b9..60f698f6d 100644 --- a/crates/moon-ui-gpui/src/analytics/tuner/shell.rs +++ b/crates/moon-ui-gpui/src/analytics/tuner/shell.rs @@ -397,7 +397,8 @@ impl AnalyticsView { /// Returns: /// The status band, or `None` while the row should stay quiet. fn filter_search_status_band(&self, p: MoonPalette, cx: &Context) -> Option { - let facts = status_facts(search_status_view(&self.tuner.sugg)); + let mut facts = status_facts(search_status_view(&self.tuner.sugg)); + note_axis_move(&mut facts, &self.tuner.sugg); if facts.essential.is_empty() && facts.tail.is_empty() { return None; } @@ -1087,7 +1088,7 @@ fn search_status_view(sugg: &SuggestState) -> SearchStatusView { SuggestState::Idle => SearchStatusView::Idle, // The blocking overlay is this one's progress feedback, so the row stays quiet. SuggestState::Running(SuggestJob::SingleField) => SearchStatusView::Idle, - SuggestState::Running(SuggestJob::AllFields { handle, total }) => { + SuggestState::Running(SuggestJob::AllFields { handle, total, .. }) => { SearchStatusView::AllFields { done: handle.completed(), total: *total, @@ -1097,7 +1098,7 @@ fn search_status_view(sugg: &SuggestState) -> SearchStatusView { // is decided by what it finds, so there is no honest denominator to show. Until the first // step publishes one, it says only that it is working. `done` is 0-based inside the // handle; the locale counts options from one. - SuggestState::Running(SuggestJob::Compose { handle }) => match handle.stage() { + SuggestState::Running(SuggestJob::Compose { handle, .. }) => match handle.stage() { Some((step, done, total)) => SearchStatusView::Compose { step, option: done + 1, @@ -1196,6 +1197,24 @@ fn status_facts(view: SearchStatusView) -> StatusFacts { } } +/// Append the axis-move note when a finished run was live under a re-adopted report axis. +/// +/// A running search appends nothing: its progress caption already owns the band, and a note +/// there would fight the spinner for width. The note goes last on the tail, so it clips first; +/// [`status_tooltip`] joins that tail back in when the band clips it. +/// +/// Args: +/// facts: Status band about to be drawn. +/// sugg: The suggestion that band is captioning. +fn note_axis_move(facts: &mut StatusFacts, sugg: &SuggestState) { + if sugg.axis_moved() && matches!(sugg, SuggestState::Done { .. }) { + facts + .tail + .push(t!("analytics.tuner.sugg_axis_moved").to_string()); + facts.tone = StatusTone::Warn; + } +} + /// Build the recovery text for the whole status band from the facts the row will render. /// /// Args: diff --git a/crates/moon-ui-gpui/src/analytics/tuner/shell/tests.rs b/crates/moon-ui-gpui/src/analytics/tuner/shell/tests.rs index 5510e530d..5b212df8e 100644 --- a/crates/moon-ui-gpui/src/analytics/tuner/shell/tests.rs +++ b/crates/moon-ui-gpui/src/analytics/tuner/shell/tests.rs @@ -3,7 +3,13 @@ //! Explicit imports throughout: the parent re-exports `gpui::*`, whose own `test` shadows the //! built-in attribute and makes `#[test]` expand recursively. -use super::{SearchStatusView, StatusTone, SuggestWork, status_facts, status_tooltip}; +use super::super::filter::state::{SearchSplit, SuggestJob, SuggestState}; +use super::{ + SearchStatusView, StatusTone, SuggestWork, note_axis_move, search_status_view, status_facts, + status_tooltip, +}; +use moon_core::db::metrics::Tally; +use moon_core::db::tuner::threshold_search::SearchHandle; use rust_i18n::t; /// Hold English for the whole assertion so a parallel locale switch cannot split the two sides. @@ -165,3 +171,118 @@ fn finished_captions_yield_and_stay_in_the_tooltip() { vec![t!("analytics.tuner.sugg_small").to_string()] ); } + +/// `shell.rs:note_axis_move` appends the axis caption only on a finished marked run. +/// +/// Breakage: dropping the `SuggestState::Done` match, so a finished composition never says the +/// report time axis shifted, or appending the note while the search is still running, where it +/// fights the progress caption for the fixed-width band. With the mark clear, the facts the +/// band already built must stay byte for byte the same. +#[test] +fn a_result_fitted_across_an_axis_move_says_so_and_warns() { + let _locale = en(); + let note = t!("analytics.tuner.sugg_axis_moved").to_string(); + let work = SuggestWork::Plain { completed: 4 }; + + let mut marked = status_facts(SearchStatusView::Done(work)); + let kept_head = marked.tail.clone(); + assert!( + !kept_head.is_empty(), + "precondition: a finished caption already occupies the tail" + ); + note_axis_move(&mut marked, &done_state(false, true, true)); + assert_eq!(marked.tone, StatusTone::Warn); + assert_eq!(marked.tail.len(), kept_head.len() + 1); + assert_eq!(&marked.tail[..kept_head.len()], kept_head.as_slice()); + assert_eq!(marked.tail.last().map(String::as_str), Some(note.as_str())); + + let mut unmarked = status_facts(SearchStatusView::Done(work)); + let essential = unmarked.essential.clone(); + let tail = unmarked.tail.clone(); + let tone = unmarked.tone; + note_axis_move(&mut unmarked, &done_state(false, false, true)); + assert_eq!(unmarked.essential, essential); + assert_eq!(unmarked.tail, tail); + assert_eq!(unmarked.tone, tone); + + let mut running_facts = status_facts(SearchStatusView::ComposeStarted); + let running_tail = running_facts.tail.clone(); + let running_tone = running_facts.tone; + let running = SuggestState::Running(SuggestJob::Compose { + handle: SearchHandle::new(), + axis_moved: true, + }); + note_axis_move(&mut running_facts, &running); + assert_eq!(running_facts.tail, running_tail); + assert_eq!(running_facts.tone, running_tone); +} + +/// A run the user stopped already warns. The axis caption must not be what flips that colour. +/// +/// Accepted trade, not a defect: `SearchStatusView::Stopped` in `status_facts` already sets +/// `StatusTone::Warn`, so `note_axis_move` is a no-op on the tone when `Done` is both +/// `stopped` and `axis_moved`. The two cases share a tone and differ only by the extra tail +/// entry. A finished run that was not stopped does move `Muted` to `Warn`, and an empty +/// finished run moves `Soft` to `Warn`, which is why the stopped case can stay colour-stable. +#[test] +fn a_stopped_run_warns_either_way_and_only_the_caption_changes() { + let _locale = en(); + let note = t!("analytics.tuner.sugg_axis_moved").to_string(); + let (essential_still, tail_still, tone_still) = captioned(&done_state(true, false, false)); + let (essential_moved, tail_moved, tone_moved) = captioned(&done_state(true, true, false)); + assert_eq!(tone_still, StatusTone::Warn); + assert_eq!(tone_moved, tone_still); + assert_eq!(essential_moved, essential_still); + let mut expected = tail_still.clone(); + expected.push(note.clone()); + assert_eq!(tail_moved, expected); + + let (_, _, fitted_tone) = captioned(&done_state(false, false, true)); + assert_eq!(fitted_tone, StatusTone::Muted); + let (_, fitted_tail, fitted_moved_tone) = captioned(&done_state(false, true, true)); + assert_eq!(fitted_moved_tone, StatusTone::Warn); + assert_eq!(fitted_tail.last().map(String::as_str), Some(note.as_str())); + + let (_, _, empty_tone) = captioned(&done_state(false, false, false)); + assert_eq!(empty_tone, StatusTone::Soft); + let (_, empty_tail, empty_moved_tone) = captioned(&done_state(false, true, false)); + assert_eq!(empty_moved_tone, StatusTone::Warn); + assert_eq!(empty_tail.last().map(String::as_str), Some(note.as_str())); +} + +/// `locales/analytics.yml:analytics.tuner.sugg_axis_moved` must be a real string in ru, en, and es. +/// +/// Breakage: deleting one language. `rust_i18n` echoes the missing key, so the status band would +/// show `analytics.tuner.sugg_axis_moved` instead of the axis-shift caption. +#[test] +fn sugg_axis_moved_is_translated_for_every_shipped_locale() { + for code in ["ru", "en", "es"] { + let _locale = crate::test_locale::force(code); + let text = t!("analytics.tuner.sugg_axis_moved").to_string(); + assert_ne!( + text, "analytics.tuner.sugg_axis_moved", + "{code} must not echo the key" + ); + assert!(!text.is_empty(), "{code} caption must not be empty"); + } +} + +fn done_state(stopped: bool, axis_moved: bool, with_split: bool) -> SuggestState { + SuggestState::Done { + work: SuggestWork::Plain { completed: 4 }, + stopped, + split: with_split.then(|| SearchSplit { + train: Tally::default(), + holdout: None, + composed: None, + compose_skipped: None, + }), + axis_moved, + } +} + +fn captioned(sugg: &SuggestState) -> (Vec, Vec, StatusTone) { + let mut facts = status_facts(search_status_view(sugg)); + note_axis_move(&mut facts, sugg); + (facts.essential, facts.tail, facts.tone) +} diff --git a/crates/moon-ui-gpui/tests/theme_contract/analytics.rs b/crates/moon-ui-gpui/tests/theme_contract/analytics.rs index 0bfb4e724..da68588d1 100644 --- a/crates/moon-ui-gpui/tests/theme_contract/analytics.rs +++ b/crates/moon-ui-gpui/tests/theme_contract/analytics.rs @@ -3,10 +3,14 @@ use super::support::*; -/// `analytics/mod.rs:observe_report_axis` must send a machine axis observation through the Writer -/// refresh path instead of reload, while `observe_valuation_mode` remains a real scope reload. -/// Merging these paths blanks and dims Analytics on every feed reconnect; removing the valuation -/// reload leaves an actual mode change under stale values. +/// `analytics/mod.rs:observe_report_axis` must refresh through the Writer path and must call +/// `TunerState::invalidate_for_axis`, while `observe_valuation_mode` remains a real scope reload. +/// +/// Breakage: restoring `self.tuner.invalidate();` cancels a live field-set composition. The +/// spinner vanishes minutes into "Pick the set", with no error and no caption. The time, coin, +/// and coin-list axes still call `invalidate()`; dropping one of those leaves that axis's drafts +/// alive across an axis adoption. Merging the axis path into `reload(` blanks Analytics on every +/// feed reconnect; removing the valuation reload leaves a mode change under stale values. #[test] fn report_axis_observation_uses_writer_refresh_while_valuation_mode_reloads() { let analytics = read_src("analytics/mod.rs"); @@ -17,6 +21,10 @@ fn report_axis_observation_uses_writer_refresh_while_valuation_mode_reloads() { !report_axis.contains("reload("), "a report-axis observation must not blank the settled surface through reload" ); + assert!( + !report_axis.contains("self.tuner.invalidate();"), + "a report-axis observation must not cancel a running composition" + ); assert!( report_axis.contains("self.request_report_refresh(") && report_axis.contains("RefreshUrgency::Writer,") @@ -27,14 +35,14 @@ fn report_axis_observation_uses_writer_refresh_while_valuation_mode_reloads() { "self.seq = self.seq.wrapping_add(1);", "self.cal_seq = self.cal_seq.wrapping_add(1);", "self.cancel_latest_reads();", - "self.tuner.invalidate();", + "self.tuner.invalidate_for_axis();", "self.time_tuner.invalidate();", "self.coins.invalidate();", "self.coin_lists.invalidate();", ] { assert!( report_axis.contains(required), - "a report-axis observation must retire every stale read identity: {required}" + "a report-axis observation must keep this call: {required}" ); } assert!( diff --git a/locales/analytics.yml b/locales/analytics.yml index 25d7dd348..22e4cd3e5 100644 --- a/locales/analytics.yml +++ b/locales/analytics.yml @@ -609,6 +609,10 @@ analytics.tuner.sugg_stopped: ru: "остановлено, попыток: %{rounds}" en: "stopped, restarts: %{rounds}" es: "parado, intentos: %{rounds}" +analytics.tuner.sugg_axis_moved: + ru: "шкала времени сдвинулась" + en: "report time axis shifted" + es: "el eje de tiempo cambió" analytics.tuner.sugg_small: ru: "сделок меньше минимума" en: "fewer trades than the minimum"