Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 17 additions & 7 deletions crates/moon-ui-gpui/src/analytics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand All @@ -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();
Expand Down
11 changes: 9 additions & 2 deletions crates/moon-ui-gpui/src/analytics/tuner/filter/actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down
46 changes: 46 additions & 0 deletions crates/moon-ui-gpui/src/analytics/tuner/filter/actions/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
);
}
99 changes: 94 additions & 5 deletions crates/moon-ui-gpui/src/analytics/tuner/filter/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<SearchSplit>,
/// 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),
Expand Down Expand Up @@ -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.
///
Expand All @@ -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,
},
}

Expand All @@ -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<usize>)> {
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`.
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
Loading
Loading