From e93cab1e435f955242bf411c3f51126c12556854 Mon Sep 17 00:00:00 2001 From: Craig Constable Date: Thu, 24 Sep 2026 09:24:02 +1000 Subject: [PATCH] fix(builder): cache derived helper catalogue state - Reuse catalogue entries across frames while keeping clock values live - Revalidate drafts after editor changes before applying - Add coverage for catalogue invalidation and validation timing --- src/studio_app/studio_helper.rs | 257 ++++++++++++++++++++++++++++---- src/theme_engine.rs | 12 ++ src/ui/components/helper.rs | 92 +++++++++++- 3 files changed, 323 insertions(+), 38 deletions(-) diff --git a/src/studio_app/studio_helper.rs b/src/studio_app/studio_helper.rs index 09615b5e..3bebd89a 100644 --- a/src/studio_app/studio_helper.rs +++ b/src/studio_app/studio_helper.rs @@ -348,6 +348,15 @@ pub(super) struct HelperSession { pub(super) target: HelperTarget, pub(super) editor: HelperState, forms: HelperForms, + catalogue: Option, +} + +struct HelperCatalogue { + context: DataContext, + language: LanguageId, + syntax: ValueSyntax, + entries: Vec, + scopes: Vec, } /// Values chosen in the details pane, kept while the user browses entries. @@ -389,9 +398,52 @@ impl HelperSession { target, editor: HelperState::new(draft), forms: HelperForms::default(), + catalogue: None, } } + fn refresh_catalogue( + &mut self, + context: &DataContext, + language: LanguageId, + syntax: ValueSyntax, + ) { + if let Some(catalogue) = self.catalogue.as_mut().filter(|catalogue| { + catalogue.language == language + && catalogue.syntax == syntax + && catalogue.context.same_catalogue_data(context) + }) { + // Keep milliseconds and the local/UTC clock live without rebuilding + // every provider/account entry and formatting every template. + for entry in &mut catalogue.entries { + if entry.category == DATE_AND_TIME + && catalogue.context.get(&entry.id).map(f64::to_bits) + != context.get(&entry.id).map(f64::to_bits) + { + entry.value = entry_value(context, syntax, &entry.id); + if let Some(value) = context.get(&entry.id) { + catalogue.context.insert(&entry.id, value); + } + } + } + return; + } + let actions = match self.target { + HelperTarget::MouseAction { .. } => MOUSE_ACTIONS, + HelperTarget::MenuAction { .. } => MENU_ACTIONS, + _ => &[], + }; + let mut entries = action_entries(actions, language); + entries.extend(value_entries(context, language, syntax)); + self.catalogue = Some(HelperCatalogue { + context: context.clone(), + language, + syntax, + entries, + scopes: helper_scopes(language), + }); + } + pub(super) fn expression(selection: Selection, field: ExpressionField, draft: String) -> Self { Self::new( HelperTarget::Expression { @@ -556,6 +608,22 @@ fn exists(context: &DataContext, name: &str) -> bool { context.get(name).is_some() || context.get_string(name).is_some() } +fn entry_value(context: &DataContext, syntax: ValueSyntax, name: &str) -> Option { + match syntax { + ValueSyntax::Expression | ValueSyntax::Action => context + .get(name) + .map(format_number_for_ui) + .or_else(|| context.get_string(name).map(str::to_string)), + ValueSyntax::Template => { + let format = preferred_text_format(value_kind(name, context)); + Some(theme_engine::format_template( + &text_template_token(name, format), + context, + )) + } + } +} + struct EntryList<'a> { context: &'a DataContext, language: LanguageId, @@ -585,20 +653,7 @@ impl EntryList<'_> { label: String, name: &str, ) { - let value = match self.syntax { - ValueSyntax::Expression | ValueSyntax::Action => self - .context - .get(name) - .map(format_number_for_ui) - .or_else(|| self.context.get_string(name).map(str::to_string)), - ValueSyntax::Template => { - let format = preferred_text_format(value_kind(name, self.context)); - Some(theme_engine::format_template( - &text_template_token(name, format), - self.context, - )) - } - }; + let value = entry_value(self.context, self.syntax, name); self.entries.push(HelperEntry { id: name.into(), category, @@ -1471,8 +1526,14 @@ impl StudioApp { } _ => return HelperAction::Close, }; - let entries = value_entries(&context, language, syntax); - let scopes = helper_scopes(language); + session.refresh_catalogue(&context, language, syntax); + let HelperSession { + editor, + forms, + catalogue, + .. + } = session; + let catalogue = catalogue.as_ref().expect("catalogue was refreshed"); let preview = |draft: &str| theme_engine::format_template(draft, &context); let view = match syntax { ValueSyntax::Expression | ValueSyntax::Action => HelperView { @@ -1485,8 +1546,8 @@ impl StudioApp { code_editor: true, editor_height: 96.0, categories: VALUE_CATEGORIES, - scopes: &scopes, - entries: &entries, + scopes: &catalogue.scopes, + entries: &catalogue.entries, }, ValueSyntax::Template => HelperView { kind_icon: LucideIcon::Type, @@ -1498,11 +1559,10 @@ impl StudioApp { code_editor: false, editor_height: 64.0, categories: VALUE_CATEGORIES, - scopes: &scopes, - entries: &entries, + scopes: &catalogue.scopes, + entries: &catalogue.entries, }, }; - let HelperSession { editor, forms, .. } = session; show_helper( ui, editor, @@ -1567,10 +1627,14 @@ impl StudioApp { .filter(|(id, _)| !id.eq_ignore_ascii_case(&self_id)) .collect::>(); let context = self.expression_context(selection); - let mut entries = action_entries(MOUSE_ACTIONS, language); - entries.extend(value_entries(&context, language, ValueSyntax::Action)); - let scopes = helper_scopes(language); - let HelperSession { editor, forms, .. } = session; + session.refresh_catalogue(&context, language, ValueSyntax::Action); + let HelperSession { + editor, + forms, + catalogue, + .. + } = session; + let catalogue = catalogue.as_ref().expect("catalogue was refreshed"); show_helper( ui, editor, @@ -1584,8 +1648,8 @@ impl StudioApp { code_editor: true, editor_height: 96.0, categories: MOUSE_ACTION_CATEGORIES, - scopes: &scopes, - entries: &entries, + scopes: &catalogue.scopes, + entries: &catalogue.entries, }, language, |draft| { @@ -1635,10 +1699,14 @@ impl StudioApp { let language = self.language(); let targets = self.layer_targets(); let context = self.menu_data_context(); - let mut entries = action_entries(MENU_ACTIONS, language); - entries.extend(value_entries(&context, language, ValueSyntax::Action)); - let scopes = helper_scopes(language); - let HelperSession { editor, forms, .. } = session; + session.refresh_catalogue(&context, language, ValueSyntax::Action); + let HelperSession { + editor, + forms, + catalogue, + .. + } = session; + let catalogue = catalogue.as_ref().expect("catalogue was refreshed"); show_helper( ui, editor, @@ -1652,8 +1720,8 @@ impl StudioApp { code_editor: true, editor_height: 40.0, categories: MENU_ACTION_CATEGORIES, - scopes: &scopes, - entries: &entries, + scopes: &catalogue.scopes, + entries: &catalogue.entries, }, language, |draft| match parse_context_menu_action_script(draft) { @@ -1798,3 +1866,126 @@ impl StudioApp { } } } + +#[cfg(test)] +mod cache_tests { + use super::*; + + fn assert_fresh( + session: &HelperSession, + context: &DataContext, + language: LanguageId, + syntax: ValueSyntax, + ) { + let cached = &session.catalogue.as_ref().unwrap().entries; + let fresh = value_entries(context, language, syntax); + assert_eq!(cached.len(), fresh.len()); + for (cached, fresh) in cached.iter().zip(&fresh) { + assert_eq!( + ( + &cached.id, + &cached.label, + &cached.group, + &cached.code, + &cached.value, + cached.category, + cached.scope + ), + ( + &fresh.id, + &fresh.label, + &fresh.group, + &fresh.code, + &fresh.value, + fresh.category, + fresh.scope + ), + ); + } + } + + #[test] + fn helper_catalogue_reuses_entries_and_keeps_clock_values_live() { + for syntax in [ + ValueSyntax::Expression, + ValueSyntax::Template, + ValueSyntax::Action, + ] { + let language = LanguageId::English; + let mut data = DataContext::from_usage(None, &Canvas::default()); + let mut session = HelperSession::context_menu_expression(vec![], String::new()); + session.refresh_catalogue(&data, language, syntax); + let allocation = session.catalogue.as_ref().unwrap().entries.as_ptr(); + session.refresh_catalogue(&data, language, syntax); + assert_eq!( + session.catalogue.as_ref().unwrap().entries.as_ptr(), + allocation + ); + for (name, value) in [ + ("time.now.unix", 1_800_000_000.25), + ("time.now.milliseconds", 1_800_000_000_250.0), + ("time.local.second", 45.0), + ("time.utc.minute", 20.0), + ] { + data.insert(name, value); + } + session.refresh_catalogue(&data, language, syntax); + assert_eq!( + session.catalogue.as_ref().unwrap().entries.as_ptr(), + allocation + ); + assert_fresh(&session, &data, language, syntax); + } + } + + #[test] + fn helper_catalogue_invalidates_for_data_language_and_syntax() { + let mut data = DataContext::from_usage(None, &Canvas::default()); + let original = data.clone(); + let mut session = HelperSession::context_menu_expression(vec![], String::new()); + let language = LanguageId::English; + let syntax = ValueSyntax::Template; + session.refresh_catalogue(&data, language, syntax); + for (name, value) in [ + ("active.session.percentage", 37.0), + ("active.session.reset.seconds", 120.0), + ("canvas.width", 640.0), + ("claude.limits.new_quota.available", 1.0), + ("claude.limits.new_quota.percentage", 42.0), + ] { + let allocation = session.catalogue.as_ref().unwrap().entries.as_ptr(); + data.insert(name, value); + session.refresh_catalogue(&data, language, syntax); + assert_ne!( + session.catalogue.as_ref().unwrap().entries.as_ptr(), + allocation + ); + assert_fresh(&session, &data, language, syntax); + } + data.insert_string("claude.limits.new_quota.label", "New quota"); + data.insert_string("accounts.claude.work.name", "Work account"); + session.refresh_catalogue(&data, language, syntax); + assert_fresh(&session, &data, language, syntax); + // Removed accounts/quotas must disappear as well. + session.refresh_catalogue(&original, language, syntax); + assert_fresh(&session, &original, language, syntax); + session.refresh_catalogue(&original, LanguageId::from_code("de").unwrap(), syntax); + assert_fresh( + &session, + &original, + LanguageId::from_code("de").unwrap(), + syntax, + ); + session.refresh_catalogue( + &original, + LanguageId::from_code("de").unwrap(), + ValueSyntax::Expression, + ); + assert_fresh( + &session, + &original, + LanguageId::from_code("de").unwrap(), + ValueSyntax::Expression, + ); + } +} diff --git a/src/theme_engine.rs b/src/theme_engine.rs index 42586965..69f3ccd6 100644 --- a/src/theme_engine.rs +++ b/src/theme_engine.rs @@ -1736,6 +1736,18 @@ impl DataContext { self.values.insert(name.to_ascii_lowercase(), value); } + /// Whether a Builder catalogue can be reused. Clock rows are refreshed + /// separately because the fractional Unix timestamp changes every frame. + pub(crate) fn same_catalogue_data(&self, other: &Self) -> bool { + self.strings == other.strings + && self.values.len() == other.values.len() + && self.values.iter().all(|(name, value)| { + other.values.get(name).is_some_and(|other| { + name.starts_with("time.") || value.to_bits() == other.to_bits() + }) + }) + } + pub fn get(&self, name: &str) -> Option { let name = name.to_ascii_lowercase(); self.values diff --git a/src/ui/components/helper.rs b/src/ui/components/helper.rs index 04634fda..2141cbb5 100644 --- a/src/ui/components/helper.rs +++ b/src/ui/components/helper.rs @@ -305,8 +305,8 @@ pub(crate) struct HelperScope { /// Draws the helper and returns what the user asked to do with the draft. /// -/// `status` validates the draft and `preview` renders it for display; both run -/// after the editor so they always reflect this frame's text. `details` draws +/// `status` validates once, then again only if the editor changes the draft. +/// `preview` runs after the editor to reflect this frame's text. `details` draws /// entry-specific controls and returns the text to insert for that entry at /// the given caret. pub(crate) fn show_helper( @@ -322,6 +322,8 @@ pub(crate) fn show_helper( let width = ui.available_width(); let height = ui.available_height(); let editor_id = ui.make_persistent_id("helper-editor"); + let mut validated_draft = state.draft.clone(); + let mut validation = status(&validated_draft); egui::Frame::new() .fill(helper_surface()) @@ -332,7 +334,7 @@ pub(crate) fn show_helper( ui.set_width((width - 28.0).max(1.0)); ui.set_min_height((height - 28.0).max(1.0)); - let can_apply = status(&state.draft).is_valid(); + let can_apply = validation.is_valid(); ui.horizontal_top(|ui| { ui.vertical(|ui| { let text_left = ui @@ -393,11 +395,14 @@ pub(crate) fn show_helper( } } - let status = status(&state.draft); + if validated_draft != state.draft { + validated_draft.clone_from(&state.draft); + validation = status(&validated_draft); + } ui.add_space(8.0); status_row( ui, - &status, + &validation, preview.map(|preview| preview(&state.draft)), language, ); @@ -406,6 +411,17 @@ pub(crate) fn show_helper( browser(ui, state, &view, language, details, editor_id); }); + // The editor or an insertion can change the draft after Apply is drawn. + // Never apply a new draft using the previous draft's validation result. + if action == HelperAction::Apply { + if validated_draft != state.draft { + validation = status(&state.draft); + } + if !validation.is_valid() { + action = HelperAction::Continue; + } + } + action } @@ -1286,6 +1302,72 @@ pub(crate) fn chip( mod tests { use super::*; + #[test] + fn helper_validates_once_per_pass_unless_the_editor_changes() { + let context = egui::Context::default(); + crate::ui::theme::configure_style(&context, LanguageId::English); + let mut state = HelperState::new("valid".into()); + let calls = std::cell::RefCell::new(Vec::new()); + for typed in [false, true] { + let mut passes = 0; + calls.borrow_mut().clear(); + let mut output = context.run_ui( + egui::RawInput { + screen_rect: Some(egui::Rect::from_min_size( + egui::Pos2::ZERO, + egui::vec2(1200.0, 900.0), + )), + events: if typed { + vec![egui::Event::Text("!".into())] + } else { + vec![] + }, + ..Default::default() + }, + |ui| { + passes += 1; + if !typed { + let id = ui.make_persistent_id("helper-editor"); + ui.memory_mut(|memory| memory.request_focus(id)); + } + show_helper( + ui, + &mut state, + HelperView { + kind_icon: LucideIcon::Braces, + kind: "Expression", + description: "", + hint: "", + code_editor: true, + editor_height: 96.0, + categories: &[], + scopes: &[], + entries: &[], + }, + LanguageId::English, + |draft| { + calls.borrow_mut().push(draft.to_string()); + if draft.contains('!') { + HelperStatus::Invalid("Invalid draft".into()) + } else { + HelperStatus::Valid { + message: "Valid".into(), + result: None, + } + } + }, + None, + |_, _, _| unreachable!(), + ); + }, + ); + output.textures_delta.clear(); + assert_eq!(calls.borrow().len(), passes + usize::from(typed)); + assert_eq!(calls.borrow().last(), Some(&state.draft)); + assert_eq!(state.draft.contains('!'), typed); + } + } + fn insert(draft: &str, cursor: usize, text: &str, mode: InsertMode) -> String { let mut state = HelperState::new(draft.into()); state.cursor = Some(cursor);