From 63fc22b0372f8a4ed686d96e27346fcb1a574857 Mon Sep 17 00:00:00 2001 From: advait-m Date: Sun, 17 May 2026 17:15:10 -0700 Subject: [PATCH 1/7] Add orchestration telemetry events for entry points, decisions, and pill bar Adds four new client-side BlocklistOrchestrationTelemetryEvent variants to instrument orchestration entry points and user interactions: * AgentMode.Orchestration.PlanConfigApprovalToggled - fires when the user toggles Use orchestration on a plan card. Captures the approval transition plus a fingerprint of the current run-wide config (execution mode, harness, has_model/env/host/auth booleans). * AgentMode.Orchestration.Entered - fires once per entry into orchestration, attributing the source: slash_command_orchestrate, plan_card_approved, or run_agents_card_shown. Dispatched from the blocklist controller (slash command path), the plan-card config block (on disapproved -> approved transition), and the run_agents confirmation card (once per card when shown, guarded by entered_event_emitted). * AgentMode.Orchestration.RunAgentsCardDecision - fires once on Accept, Accept-without-orchestration, or Reject. Reports modified_fields_from_tool_call and modified_fields_from_active_config using stable identifiers in orchestration_modified_field that match the server-side RunAgentsOutcome.modified_fields constants so the two streams can be joined 1:1. * AgentMode.Orchestration.PillBarInteraction - fires on Switch (pill click), Open in new pane/tab, Focus opened conversation, Stop, Kill, TogglePinOn/Off, and OpenMenu. Pill body clicks now route through a new PillClicked typed action so telemetry runs before the navigation. Hover events are intentionally not tracked. All payloads are metadata-only (enums, IDs, booleans, counts). No prompts, summaries, agent names, environment names, worker host slugs, or auth secret labels are ever serialized. Harness and execution mode are bucketed into closed enums (OrchestrationHarnessKind, OrchestrationExecutionModeKind) so the analytics columns stay low-cardinality. Co-Authored-By: Oz --- .../agent_view/orchestration_pill_bar.rs | 209 +++++++++++++++--- app/src/ai/blocklist/controller.rs | 18 ++ .../inline_action/run_agents_card_view.rs | 201 +++++++++++++++++ app/src/ai/blocklist/telemetry.rs | 205 +++++++++++++++++ .../ai/document/orchestration_config_block.rs | 80 +++++++ 5 files changed, 680 insertions(+), 33 deletions(-) diff --git a/app/src/ai/blocklist/agent_view/orchestration_pill_bar.rs b/app/src/ai/blocklist/agent_view/orchestration_pill_bar.rs index 38b546fdc7..5cafacbaf0 100644 --- a/app/src/ai/blocklist/agent_view/orchestration_pill_bar.rs +++ b/app/src/ai/blocklist/agent_view/orchestration_pill_bar.rs @@ -45,6 +45,10 @@ use crate::ai::blocklist::agent_view::orchestration_pill_bar_model::{ }; use crate::ai::blocklist::agent_view::{AgentViewController, AgentViewControllerEvent}; use crate::ai::blocklist::orchestration_topology::descendant_conversation_ids_in_spawn_order; +use crate::ai::blocklist::telemetry::{ + BlocklistOrchestrationTelemetryEvent, PillBarActionKind, PillBarInteractionEvent, + PillBarPillKind, +}; use crate::ai::blocklist::{BlocklistAIHistoryEvent, BlocklistAIHistoryModel}; use crate::ai::harness_display; use crate::features::FeatureFlag; @@ -56,6 +60,7 @@ use crate::ui_components::icon_with_status::{ }; use crate::ui_components::icons::Icon; use crate::workspace::WorkspaceAction; +use warp_core::send_telemetry_from_ctx; use warp_core::ui::theme::color::internal_colors; use warpui::EntityId; @@ -132,12 +137,25 @@ pub(crate) fn render_agent_avatar_disc( } /// What kind of pill we are rendering, which determines click behavior. -#[derive(Clone, Copy, Debug)] -enum PillKind { +/// +/// `pub(crate)` so it can appear as a field on the public +/// `OrchestrationPillBarAction::PillClicked` variant without leaking a +/// private type through the action enum. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum PillKind { Orchestrator, Child, } +impl PillKind { + fn telemetry_kind(self) -> PillBarPillKind { + match self { + Self::Orchestrator => PillBarPillKind::Orchestrator, + Self::Child => PillBarPillKind::Child, + } + } +} + /// Whether the user has pinned this pill to the leading section of the bar. #[derive(Clone, Copy, PartialEq, Eq)] enum PillPinState { @@ -254,6 +272,16 @@ pub enum OrchestrationPillBarAction { FocusOpenedConversation(AIConversationId), /// Toggle the pin state for the given child conversation. TogglePin(AIConversationId), + /// Pill body was clicked. Routes through `handle_action` (rather + /// than dispatching the navigation `TerminalAction` directly from + /// the click closure) so telemetry can be emitted before the + /// navigation runs. The handler resolves whether the conversation + /// is already open elsewhere and dispatches the appropriate + /// downstream action. + PillClicked { + conversation_id: AIConversationId, + pill_kind: PillKind, + }, } /// Renders the pill bar above the agent view: one pill for the orchestrator @@ -641,12 +669,73 @@ fn orchestrator_label(orchestrator: &AIConversation) -> String { .unwrap_or_else(|| "Orchestrator".to_string()) } +impl OrchestrationPillBar { + /// Resolves the `(source_conversation_id, total_pills, total_pinned)` + /// triple used to enrich every `PillBarInteraction` event. Returns + /// `None` when there is no active orchestration tree to attribute + /// the interaction to (in which case we skip the telemetry). + fn pill_bar_telemetry_context( + &self, + app: &AppContext, + ) -> Option<(AIConversationId, usize, usize)> { + let (orchestrator_id, specs) = self.pill_specs(app)?; + let total_pills = specs.len(); + let total_pinned = specs + .iter() + .filter(|spec| matches!(spec.pin_state, PillPinState::Pinned)) + .count(); + Some((orchestrator_id, total_pills, total_pinned)) + } + + /// Looks up the pill kind for `target_id` in the current pill + /// specs. Used by `handle_action` to attribute menu actions (which + /// don't carry the pill kind on the action variant) to either the + /// orchestrator or a child. + fn pill_kind_for(&self, target_id: AIConversationId, app: &AppContext) -> PillBarPillKind { + self.pill_specs(app) + .and_then(|(_, specs)| { + specs + .into_iter() + .find(|spec| spec.conversation_id == target_id) + .map(|spec| spec.kind.telemetry_kind()) + }) + .unwrap_or(PillBarPillKind::Child) + } + + fn emit_pill_bar_interaction( + &self, + action: PillBarActionKind, + pill_kind: PillBarPillKind, + target_conversation_id: AIConversationId, + ctx: &mut ViewContext, + ) { + let Some((source_conversation_id, total_pills, total_pinned)) = + self.pill_bar_telemetry_context(ctx) + else { + return; + }; + send_telemetry_from_ctx!( + BlocklistOrchestrationTelemetryEvent::PillBarInteraction(PillBarInteractionEvent { + action, + pill_kind, + total_pills, + total_pinned, + source_conversation_id, + target_conversation_id, + }), + ctx + ); + } +} + impl TypedActionView for OrchestrationPillBar { type Action = OrchestrationPillBarAction; fn handle_action(&mut self, action: &Self::Action, ctx: &mut ViewContext) { match action { OrchestrationPillBarAction::OpenMenu(id) => { + let pill_kind = self.pill_kind_for(*id, ctx); + self.emit_pill_bar_interaction(PillBarActionKind::OpenMenu, pill_kind, *id, ctx); self.open_menu_for(*id, ctx); } OrchestrationPillBarAction::CloseMenu => { @@ -660,6 +749,12 @@ impl TypedActionView for OrchestrationPillBar { // dispatch it through the pane header action surface so // it bubbles up the standard way (mirrors the pill-click // path in `render_pill`). + self.emit_pill_bar_interaction( + PillBarActionKind::OpenInNewPane, + PillBarPillKind::Child, + *id, + ctx, + ); self.close_menu(ctx); ctx.dispatch_typed_action( &PaneHeaderAction::::CustomAction( @@ -670,6 +765,12 @@ impl TypedActionView for OrchestrationPillBar { ); } OrchestrationPillBarAction::OpenInNewTab(id) => { + self.emit_pill_bar_interaction( + PillBarActionKind::OpenInNewTab, + PillBarPillKind::Child, + *id, + ctx, + ); self.close_menu(ctx); ctx.dispatch_typed_action( &PaneHeaderAction::::CustomAction( @@ -680,6 +781,12 @@ impl TypedActionView for OrchestrationPillBar { ); } OrchestrationPillBarAction::Stop(id) => { + self.emit_pill_bar_interaction( + PillBarActionKind::Stop, + PillBarPillKind::Child, + *id, + ctx, + ); self.close_menu(ctx); ctx.dispatch_typed_action( &PaneHeaderAction::::CustomAction( @@ -690,6 +797,12 @@ impl TypedActionView for OrchestrationPillBar { ); } OrchestrationPillBarAction::Kill(id) => { + self.emit_pill_bar_interaction( + PillBarActionKind::Kill, + PillBarPillKind::Child, + *id, + ctx, + ); self.close_menu(ctx); ctx.dispatch_typed_action( &PaneHeaderAction::::CustomAction( @@ -706,11 +819,57 @@ impl TypedActionView for OrchestrationPillBar { // Singleton emits an event that drives the re-render in every // pill bar, so no `ctx.notify()` needed here. let id = *id; + // Determine which way the toggle is going before applying + // it so the telemetry payload reports the resulting state + // rather than the prior one. + let was_pinned = OrchestrationPillBarModel::as_ref(ctx).is_pinned(&id); + let action_kind = if was_pinned { + PillBarActionKind::TogglePinOff + } else { + PillBarActionKind::TogglePinOn + }; + self.emit_pill_bar_interaction(action_kind, PillBarPillKind::Child, id, ctx); OrchestrationPillBarModel::handle(ctx).update(ctx, |model, ctx| { model.toggle_pin(id, ctx); }); } + OrchestrationPillBarAction::PillClicked { + conversation_id, + pill_kind, + } => { + let id = *conversation_id; + self.emit_pill_bar_interaction( + PillBarActionKind::Switch, + pill_kind.telemetry_kind(), + id, + ctx, + ); + // Mirror the dispatch logic from the old in-place click + // closure: prefer focusing an existing pane that already + // owns this conversation; otherwise switch in place. + let self_terminal_view_id = + self.agent_view_controller.as_ref(ctx).terminal_view_id(); + let is_open_elsewhere = + is_conversation_open_in_other_visible_view(id, self_terminal_view_id, ctx); + if is_open_elsewhere { + ctx.dispatch_typed_action( + &OrchestrationPillBarAction::FocusOpenedConversation(id), + ); + } else { + ctx.dispatch_typed_action( + &PaneHeaderAction::::CustomAction( + navigation_action_for_pill(*pill_kind, id), + ), + ); + } + } OrchestrationPillBarAction::FocusOpenedConversation(id) => { + self.emit_pill_bar_interaction( + PillBarActionKind::FocusOpenedConversation, + PillBarPillKind::Child, + *id, + ctx, + ); self.close_menu(ctx); // "Focus pane" is purely a focus operation: the // conversation already lives in some other visible @@ -1707,40 +1866,24 @@ fn render_pill( }; ctx.dispatch_typed_action(OrchestrationPillBarAction::SetHoveredPill(payload)); }) - .on_click(move |ctx, app, _| { + .on_click(move |ctx, _app, _| { if is_selected { return; } - // Single source of truth: if the conversation is currently owned - // by a *different* visible terminal view than this orchestrator - // pane (because it was split off into a separate pane or tab), - // the pill should focus that existing pane rather than re-render - // the conversation in place. Route through the pill bar's own - // `FocusOpenedConversation` action so this path and the 3-dot - // menu's "Focus pane" item share a single implementation — the - // pill bar's `handle_action` then dispatches - // `WorkspaceAction::FocusTerminalViewInWorkspace` from a - // `ViewContext`, which reliably reaches the workspace. - let is_open_elsewhere = - is_conversation_open_in_other_visible_view(conversation_id, self_terminal_view_id, app); - - if is_open_elsewhere { - ctx.dispatch_typed_action(OrchestrationPillBarAction::FocusOpenedConversation( - conversation_id, - )); - return; - } - // Child pills reveal the existing child pane/session and orchestrator - // pills swap back to the orchestrator's pane. The hidden child pane - // owns the live harness/ambient session (locally) or the shared- - // session viewer that joins the child's session (for shared session - // viewers, materialized lazily by `OrchestrationViewerModel` when a - // `session_id` is known). In both cases the swap-based navigation is - // the correct destination. - let action = navigation_action_for_pill(kind, conversation_id); - ctx.dispatch_typed_action( - PaneHeaderAction::::CustomAction(action), - ); + // Route the click through `PillClicked` so the pill bar's + // `handle_action` can emit telemetry before forwarding the + // navigation. The handler resolves whether the conversation is + // already owned by a different visible terminal view (and, if + // so, dispatches `FocusOpenedConversation`) or whether to swap + // the current pane in place via `navigation_action_for_pill`. + // The `self_terminal_view_id` captured into this closure is no + // longer needed because the handler reads it from the pill + // bar's own controller. + let _ = self_terminal_view_id; + ctx.dispatch_typed_action(OrchestrationPillBarAction::PillClicked { + conversation_id, + pill_kind: kind, + }); }) .finish(); diff --git a/app/src/ai/blocklist/controller.rs b/app/src/ai/blocklist/controller.rs index ce55f77e79..69a7284903 100644 --- a/app/src/ai/blocklist/controller.rs +++ b/app/src/ai/blocklist/controller.rs @@ -674,6 +674,24 @@ impl BlocklistAIController { let (query, user_query_mode) = extract_user_query_mode(query); + // Fire `AgentMode.Orchestration.Entered` once per /orchestrate + // user query so we can attribute conversations to the slash + // command entry point alongside plan-card and run_agents-card + // entries (which emit their own variants of this event). + if matches!(user_query_mode, UserQueryMode::Orchestrate) { + send_telemetry_from_ctx!( + super::telemetry::BlocklistOrchestrationTelemetryEvent::OrchestrationEntered( + super::telemetry::OrchestrationEnteredEvent { + conversation_id, + plan_id: None, + entry_source: + super::telemetry::OrchestrationEntrySource::SlashCommandOrchestrate, + } + ), + ctx + ); + } + let should_prepend_finished_action_results = matches!( input_query.input_query, InputQueryType::UserSubmittedQueryFromInput { .. } diff --git a/app/src/ai/blocklist/inline_action/run_agents_card_view.rs b/app/src/ai/blocklist/inline_action/run_agents_card_view.rs index 0ea56358b3..53ebb634b7 100644 --- a/app/src/ai/blocklist/inline_action/run_agents_card_view.rs +++ b/app/src/ai/blocklist/inline_action/run_agents_card_view.rs @@ -10,10 +10,17 @@ use ai::agent::action_result::{RunAgentsAgentOutcomeKind, RunAgentsResult}; use ai::agent::orchestration_config::{OrchestrationConfig, OrchestrationConfigStatus}; use crate::ai::agent::conversation::AIConversationId; +use crate::ai::blocklist::telemetry::{ + orchestration_modified_field, BlocklistOrchestrationTelemetryEvent, + OrchestrationApprovalStatus, OrchestrationEnteredEvent, OrchestrationEntrySource, + OrchestrationExecutionModeKind, OrchestrationHarnessKind, RunAgentsCardDecision, + RunAgentsCardDecisionEvent, +}; use crate::BlocklistAIHistoryModel; use ai::skills::SkillReference; use pathfinder_geometry::vector::vec2f; use std::rc::Rc; +use warp_core::send_telemetry_from_ctx; use warpui::elements::{ Border, ChildView, Container, CornerRadius, CrossAxisAlignment, Empty, Flex, MainAxisSize, OffsetPositioning, ParentElement, Radius, Stack, Text, @@ -205,6 +212,16 @@ pub struct RunAgentsCardView { /// UI-only per-harness model memory so switching harnesses preserves /// the user's previous model selection for each harness. saved_model_per_harness: HashMap, + /// Snapshot of the most recently received raw `RunAgentsRequest` + /// from the LLM stream. Used at decision time to compute which + /// run-wide config fields the user modified before accepting. + original_tool_call_request: RunAgentsRequest, + /// Set to `true` once `OrchestrationEntered` has been emitted for + /// this card so we don't double-fire across re-renders. + entered_event_emitted: bool, + /// Set to `true` once a terminal decision event has been emitted so + /// we don't double-fire if the user retries. + decision_event_emitted: bool, } /// Computes the `is_denied` flag at construction time. /// @@ -299,6 +316,10 @@ impl RunAgentsCardView { // (called after streaming finishes and agent_run_configs is populated). let state = RunAgentsEditState::from_request(request); let auto_launched = false; + // Snapshot the raw incoming request so we can compute + // user-applied diffs at Accept time, even if `update_request` + // later replaces `self.state` from a follow-up stream chunk. + let original_tool_call_request = request.clone(); let reject_keystroke = CTRL_C_KEYSTROKE.clone(); let accept_keystroke = ENTER_KEYSTROKE.clone(); @@ -463,6 +484,9 @@ impl RunAgentsCardView { action_model, block_model, saved_model_per_harness: HashMap::new(), + original_tool_call_request, + entered_event_emitted: false, + decision_event_emitted: false, }; // Eagerly create picker views so the editor section is always @@ -477,6 +501,10 @@ impl RunAgentsCardView { if self.spawning.is_some() || self.auto_launched || self.is_denied { return; } + // Keep our "raw tool call" snapshot in sync with the latest + // streamed chunk so the diff at Accept time reflects what the + // LLM ultimately emitted. + self.original_tool_call_request = request.clone(); let mut new_state = RunAgentsEditState::from_request(request); // Resolve empty fields from the active config (same as in new()). if let Some((config, status)) = &self.active_config { @@ -581,6 +609,11 @@ impl RunAgentsCardView { } resolve_interactive_defaults(&mut self.state, &*self.block_model, ctx); oc::repopulate_all_pickers(&mut self.state.orch, &self.handles.pickers, ctx); + + // Reaching this branch means the card is about to be shown to + // the user — log it as an orchestration entry point once per + // card instance. + self.emit_orchestration_entered_once(conversation_id, ctx); } /// Validates and dispatches the resolved request. @@ -597,12 +630,88 @@ impl RunAgentsCardView { return; } let request = self.state.to_request(); + self.emit_decision(RunAgentsCardDecision::Accept, ctx); let action_id = self.action_id.clone(); self.action_model.update(ctx, |action_model, action_ctx| { action_model.execute_run_agents(&action_id, request, action_ctx); }); } + /// Emits `OrchestrationEntered::RunAgentsCardShown` once per card + /// instance. Idempotent across re-runs of + /// `try_auto_launch_on_stream_complete`. + fn emit_orchestration_entered_once( + &mut self, + conversation_id: AIConversationId, + ctx: &mut ViewContext, + ) { + if self.entered_event_emitted { + return; + } + self.entered_event_emitted = true; + send_telemetry_from_ctx!( + BlocklistOrchestrationTelemetryEvent::OrchestrationEntered(OrchestrationEnteredEvent { + conversation_id, + plan_id: (!self.state.plan_id.is_empty()).then(|| self.state.plan_id.clone()), + entry_source: OrchestrationEntrySource::RunAgentsCardShown, + }), + ctx + ); + } + + /// Emits `RunAgentsCardDecision` once per card instance, capturing + /// the dispatched run-wide config and the set of fields that + /// diverged from the original tool call / active approved config. + fn emit_decision(&mut self, decision: RunAgentsCardDecision, ctx: &mut ViewContext) { + if self.decision_event_emitted { + return; + } + self.decision_event_emitted = true; + let Some(conversation_id) = self.block_model.conversation_id(ctx) else { + return; + }; + let modified_fields_from_tool_call = + diverged_orch_fields(&self.state.orch, &self.original_tool_call_request); + let (had_active_config, active_config_status, modified_fields_from_active_config) = + match &self.active_config { + Some((cfg, status)) => { + let status_enum = if status.is_approved() { + Some(OrchestrationApprovalStatus::Approved) + } else if status.is_disapproved() { + Some(OrchestrationApprovalStatus::Disapproved) + } else { + None + }; + let diff = if status.is_approved() { + diverged_orch_fields_against_config(&self.state.orch, cfg) + } else { + Vec::new() + }; + (true, status_enum, diff) + } + None => (false, None, Vec::new()), + }; + send_telemetry_from_ctx!( + BlocklistOrchestrationTelemetryEvent::RunAgentsCardDecision( + RunAgentsCardDecisionEvent { + conversation_id, + plan_id: (!self.state.plan_id.is_empty()).then(|| self.state.plan_id.clone()), + decision, + agent_count: self.state.agent_run_configs.len(), + harness: OrchestrationHarnessKind::from_str(&self.state.orch.harness_type), + execution_mode: OrchestrationExecutionModeKind::from_run_agents( + &self.state.orch.execution_mode, + ), + modified_fields_from_tool_call, + modified_fields_from_active_config, + had_active_config, + active_config_status, + } + ), + ctx + ); + } + /// Construct the picker dropdown views (idempotent). fn ensure_pickers(&mut self, ctx: &mut ViewContext) { let appearance = Appearance::as_ref(ctx); @@ -894,6 +1003,7 @@ impl TypedActionView for RunAgentsCardView { self.handle_accept(ctx); } RunAgentsCardViewAction::AcceptWithoutOrchestration => { + self.emit_decision(RunAgentsCardDecision::AcceptWithoutOrchestration, ctx); let action_id = self.action_id.clone(); self.action_model.update(ctx, |action_model, action_ctx| { action_model.deny_run_agents(&action_id, String::new(), action_ctx); @@ -903,6 +1013,7 @@ impl TypedActionView for RunAgentsCardView { self.toggle_accept_menu(ctx); } RunAgentsCardViewAction::Reject => { + self.emit_decision(RunAgentsCardDecision::Reject, ctx); ctx.emit(RunAgentsCardViewEvent::RejectRequested); } RunAgentsCardViewAction::ExecutionModeToggled { is_remote } => { @@ -955,6 +1066,96 @@ impl TypedActionView for RunAgentsCardView { } } +/// Returns the names of run-wide config fields that differ between the +/// user-edited `state` and the original `RunAgentsRequest` (raw tool +/// call as emitted by the LLM). Field names are stable identifiers from +/// [`orchestration_modified_field`] so they can be joined with the +/// server-side `RunAgentsOutcome.modified_fields` payload. +fn diverged_orch_fields( + state: &oc::OrchestrationEditState, + original: &RunAgentsRequest, +) -> Vec<&'static str> { + let mut fields = Vec::new(); + if state.model_id != original.model_id { + fields.push(orchestration_modified_field::MODEL_ID); + } + if state.harness_type != original.harness_type { + fields.push(orchestration_modified_field::HARNESS); + } + let state_remote = state.execution_mode.is_remote(); + let original_remote = original.execution_mode.is_remote(); + if state_remote != original_remote { + fields.push(orchestration_modified_field::EXECUTION_MODE); + } else if let ( + RunAgentsExecutionMode::Remote { + environment_id: state_env, + worker_host: state_host, + .. + }, + RunAgentsExecutionMode::Remote { + environment_id: orig_env, + worker_host: orig_host, + .. + }, + ) = (&state.execution_mode, &original.execution_mode) + { + if state_env != orig_env { + fields.push(orchestration_modified_field::ENVIRONMENT_ID); + } + if state_host != orig_host { + fields.push(orchestration_modified_field::WORKER_HOST); + } + } + if state.auth_secret_name != original.harness_auth_secret_name { + fields.push(orchestration_modified_field::AUTH_SECRET); + } + fields +} + +/// Same shape as [`diverged_orch_fields`] but compares against an +/// approved `OrchestrationConfig`. The auth_secret slot is omitted +/// because managed secrets are stored per-user, not on the config. +fn diverged_orch_fields_against_config( + state: &oc::OrchestrationEditState, + config: &OrchestrationConfig, +) -> Vec<&'static str> { + use ai::agent::orchestration_config::OrchestrationExecutionMode; + let mut fields = Vec::new(); + if state.model_id != config.model_id { + fields.push(orchestration_modified_field::MODEL_ID); + } + if state.harness_type != config.harness_type { + fields.push(orchestration_modified_field::HARNESS); + } + let state_remote = state.execution_mode.is_remote(); + let config_remote = matches!( + config.execution_mode, + OrchestrationExecutionMode::Remote { .. } + ); + if state_remote != config_remote { + fields.push(orchestration_modified_field::EXECUTION_MODE); + } else if let ( + RunAgentsExecutionMode::Remote { + environment_id: state_env, + worker_host: state_host, + .. + }, + OrchestrationExecutionMode::Remote { + environment_id: cfg_env, + worker_host: cfg_host, + }, + ) = (&state.execution_mode, &config.execution_mode) + { + if state_env != cfg_env { + fields.push(orchestration_modified_field::ENVIRONMENT_ID); + } + if state_host != cfg_host { + fields.push(orchestration_modified_field::WORKER_HOST); + } + } + fields +} + fn render_confirmation_card( state: &RunAgentsEditState, handles: &RunAgentsCardHandles, diff --git a/app/src/ai/blocklist/telemetry.rs b/app/src/ai/blocklist/telemetry.rs index ab48432aa1..926dcfd26e 100644 --- a/app/src/ai/blocklist/telemetry.rs +++ b/app/src/ai/blocklist/telemetry.rs @@ -8,6 +8,10 @@ use warp_core::telemetry::{EnablementState, TelemetryEvent, TelemetryEventDesc}; #[strum_discriminants(derive(EnumIter))] pub(crate) enum BlocklistOrchestrationTelemetryEvent { TeamAgentCommunicationFailed(TeamAgentCommunicationFailedEvent), + PlanConfigApprovalToggled(PlanConfigApprovalToggledEvent), + RunAgentsCardDecision(RunAgentsCardDecisionEvent), + PillBarInteraction(PillBarInteractionEvent), + OrchestrationEntered(OrchestrationEnteredEvent), } #[derive(Clone, Copy, Debug, Serialize)] @@ -58,6 +62,187 @@ pub(crate) struct TeamAgentCommunicationFailedEvent { pub error_message: Option, } +/// Coarse approval transition for the plan card's +/// `Use orchestration` toggle. +#[derive(Clone, Copy, Debug, Serialize)] +#[serde(rename_all = "snake_case")] +pub(crate) enum OrchestrationApprovalStatus { + Approved, + Disapproved, +} + +/// Run-wide execution mode reported on telemetry payloads. Mirrors +/// `RunAgentsExecutionMode` but is a flat enum so the payload stays +/// metadata-only and never carries an environment id or worker host. +#[derive(Clone, Copy, Debug, Serialize)] +#[serde(rename_all = "snake_case")] +pub(crate) enum OrchestrationExecutionModeKind { + Local, + Remote, +} + +impl OrchestrationExecutionModeKind { + pub(crate) fn from_run_agents(mode: &ai::agent::action::RunAgentsExecutionMode) -> Self { + if mode.is_remote() { + Self::Remote + } else { + Self::Local + } + } +} + +/// Stable bucket for the run-wide harness selection. Maps a raw harness +/// string (which is server-/catalog-controlled but may include strings +/// we don't yet recognize on the client) onto a closed set so the +/// analytics column stays low-cardinality. +#[derive(Clone, Copy, Debug, Serialize)] +#[serde(rename_all = "snake_case")] +pub(crate) enum OrchestrationHarnessKind { + Oz, + ClaudeCode, + Codex, + OpenCode, + Gemini, + Unknown, +} + +impl OrchestrationHarnessKind { + pub(crate) fn from_str(harness_type: &str) -> Self { + // Match the canonical strings used in `OrchestrationEditState` + // and the orchestration config proto. Anything else collapses + // to `Unknown` so the analytics column stays bounded even if + // the server adds a new harness before the client catalogs it. + match harness_type { + "oz" | "" => Self::Oz, + "claude" | "claude-code" | "claude_code" => Self::ClaudeCode, + "codex" => Self::Codex, + "opencode" | "open-code" | "open_code" => Self::OpenCode, + "gemini" => Self::Gemini, + _ => Self::Unknown, + } + } +} + +/// Stable names for the run-wide config fields that can diverge between +/// the dispatched orchestration request and either the original tool +/// call payload or an active approved orchestration config. Mirrors the +/// server's `RunAgentsModifiedField*` constants so the two telemetry +/// streams can be joined on field name. +pub(crate) mod orchestration_modified_field { + pub const MODEL_ID: &str = "model_id"; + pub const HARNESS: &str = "harness"; + pub const EXECUTION_MODE: &str = "execution_mode"; + pub const ENVIRONMENT_ID: &str = "environment_id"; + pub const WORKER_HOST: &str = "worker_host"; + pub const AUTH_SECRET: &str = "auth_secret"; +} + +#[derive(Debug, Serialize)] +pub(crate) struct PlanConfigApprovalToggledEvent { + pub conversation_id: AIConversationId, + #[serde(skip_serializing_if = "Option::is_none")] + pub plan_id: Option, + pub previous_status: OrchestrationApprovalStatus, + pub new_status: OrchestrationApprovalStatus, + pub execution_mode: OrchestrationExecutionModeKind, + pub harness: OrchestrationHarnessKind, + pub has_model: bool, + pub has_environment: bool, + pub has_worker_host: bool, + pub has_auth_secret: bool, +} + +/// Decision a user took on the run_agents confirmation card. +#[derive(Clone, Copy, Debug, Serialize)] +#[serde(rename_all = "snake_case")] +pub(crate) enum RunAgentsCardDecision { + Accept, + AcceptWithoutOrchestration, + Reject, +} + +#[derive(Debug, Serialize)] +pub(crate) struct RunAgentsCardDecisionEvent { + pub conversation_id: AIConversationId, + #[serde(skip_serializing_if = "Option::is_none")] + pub plan_id: Option, + pub decision: RunAgentsCardDecision, + pub agent_count: usize, + pub harness: OrchestrationHarnessKind, + pub execution_mode: OrchestrationExecutionModeKind, + /// Names of run-wide config fields where the dispatched request + /// diverged from the original `RunAgentsRequest` emitted by the LLM. + /// Values are drawn from `orchestration_modified_field` so this can + /// be joined against the server-side `RunAgentsOutcome.modified_fields`. + /// Empty when the user accepted the tool call without edits. + pub modified_fields_from_tool_call: Vec<&'static str>, + /// Same shape as `modified_fields_from_tool_call`, but compared + /// against the approved orchestration config snapshot (when one + /// exists). Empty when no active approved config exists or when + /// the dispatched request matches it exactly. + pub modified_fields_from_active_config: Vec<&'static str>, + pub had_active_config: bool, + #[serde(skip_serializing_if = "Option::is_none")] + pub active_config_status: Option, +} + +/// Surface that first introduced orchestration into a conversation. +#[derive(Clone, Copy, Debug, Serialize)] +#[serde(rename_all = "snake_case")] +pub(crate) enum OrchestrationEntrySource { + /// User typed `/orchestrate` as the active query mode. + SlashCommandOrchestrate, + /// User toggled the `Use orchestration` switch on the plan card to + /// the approved state. + PlanCardApproved, + /// The LLM emitted a `run_agents` tool call that surfaced the + /// confirmation card (i.e. did not auto-launch). + RunAgentsCardShown, +} + +#[derive(Debug, Serialize)] +pub(crate) struct OrchestrationEnteredEvent { + pub conversation_id: AIConversationId, + #[serde(skip_serializing_if = "Option::is_none")] + pub plan_id: Option, + pub entry_source: OrchestrationEntrySource, +} + +#[derive(Clone, Copy, Debug, Serialize)] +#[serde(rename_all = "snake_case")] +pub(crate) enum PillBarPillKind { + Orchestrator, + Child, +} + +/// Concrete user actions against an orchestration pill bar entry. +#[derive(Clone, Copy, Debug, Serialize)] +#[serde(rename_all = "snake_case")] +pub(crate) enum PillBarActionKind { + Switch, + OpenInNewPane, + OpenInNewTab, + FocusOpenedConversation, + Stop, + Kill, + TogglePinOn, + TogglePinOff, + OpenMenu, +} + +#[derive(Debug, Serialize)] +pub(crate) struct PillBarInteractionEvent { + pub action: PillBarActionKind, + pub pill_kind: PillBarPillKind, + pub total_pills: usize, + pub total_pinned: usize, + /// The conversation that hosts the pill bar (the orchestrator's + /// conversation in the active pane). + pub source_conversation_id: AIConversationId, + /// The conversation that the action targets (the pill the user clicked). + pub target_conversation_id: AIConversationId, +} + impl TelemetryEvent for BlocklistOrchestrationTelemetryEvent { fn name(&self) -> &'static str { BlocklistOrchestrationTelemetryEventDiscriminants::from(self).name() @@ -66,6 +251,10 @@ impl TelemetryEvent for BlocklistOrchestrationTelemetryEvent { fn payload(&self) -> Option { match self { Self::TeamAgentCommunicationFailed(event) => Some(json!(event)), + Self::PlanConfigApprovalToggled(event) => Some(json!(event)), + Self::RunAgentsCardDecision(event) => Some(json!(event)), + Self::PillBarInteraction(event) => Some(json!(event)), + Self::OrchestrationEntered(event) => Some(json!(event)), } } @@ -92,6 +281,10 @@ impl TelemetryEventDesc for BlocklistOrchestrationTelemetryEventDiscriminants { Self::TeamAgentCommunicationFailed => { "AgentMode.Orchestration.TeamAgentCommunicationFailed" } + Self::PlanConfigApprovalToggled => "AgentMode.Orchestration.PlanConfigApprovalToggled", + Self::RunAgentsCardDecision => "AgentMode.Orchestration.RunAgentsCardDecision", + Self::PillBarInteraction => "AgentMode.Orchestration.PillBarInteraction", + Self::OrchestrationEntered => "AgentMode.Orchestration.Entered", } } @@ -100,6 +293,18 @@ impl TelemetryEventDesc for BlocklistOrchestrationTelemetryEventDiscriminants { Self::TeamAgentCommunicationFailed => { "Failed to send an orchestration message or lifecycle event for a TeamAgent" } + Self::PlanConfigApprovalToggled => { + "User toggled the Use orchestration switch on a plan card" + } + Self::RunAgentsCardDecision => { + "User accepted, accepted-without-orchestration, or rejected a run_agents confirmation card. Reports which config fields diverged from the original tool call and/or the active approved config." + } + Self::PillBarInteraction => { + "User interacted with the orchestration pill bar (switch, pin, open in pane/tab, stop, kill, etc.)" + } + Self::OrchestrationEntered => { + "Orchestration was activated in a conversation via /orchestrate, a plan-card approval toggle, or a run_agents confirmation card surfacing" + } } } diff --git a/app/src/ai/document/orchestration_config_block.rs b/app/src/ai/document/orchestration_config_block.rs index 6657e13e25..79b6ec4a4a 100644 --- a/app/src/ai/document/orchestration_config_block.rs +++ b/app/src/ai/document/orchestration_config_block.rs @@ -6,6 +6,7 @@ use ai::agent::action::RunAgentsExecutionMode; use ai::agent::orchestration_config::OrchestrationConfigStatus; use pathfinder_color::ColorU; use std::collections::HashMap; +use warp_core::send_telemetry_from_ctx; use warpui::elements::{ ConstrainedBox, Container, CornerRadius, CrossAxisAlignment, Empty, Flex, Hoverable, MainAxisAlignment, MainAxisSize, MouseStateHandle, ParentElement, Radius, Text, @@ -19,6 +20,11 @@ use crate::ai::blocklist::inline_action::host_picker::{HostPicker, HostPickerEve use crate::ai::blocklist::inline_action::orchestration_controls::{ self as oc, OrchestrationControlAction, OrchestrationEditState, OrchestrationPickerHandles, }; +use crate::ai::blocklist::telemetry::{ + BlocklistOrchestrationTelemetryEvent, OrchestrationApprovalStatus, OrchestrationEnteredEvent, + OrchestrationEntrySource, OrchestrationExecutionModeKind, OrchestrationHarnessKind, + PlanConfigApprovalToggledEvent, +}; use crate::ai::blocklist::BlocklistAIHistoryEvent; use crate::ai::document::ai_document_model::AIDocumentModel; use crate::ai::harness_availability::{HarnessAvailabilityEvent, HarnessAvailabilityModel}; @@ -28,6 +34,22 @@ use crate::ui_components::blended_colors; use crate::BlocklistAIHistoryModel; use warp_core::ui::theme::WarpTheme; +/// Returns `(had_previous, now_set)` for a remote-mode environment id +/// before/after an edit. +fn env_presence(execution_mode: &RunAgentsExecutionMode) -> bool { + matches!( + execution_mode, + RunAgentsExecutionMode::Remote { environment_id, .. } if !environment_id.is_empty() + ) +} + +fn host_presence(execution_mode: &RunAgentsExecutionMode) -> bool { + matches!( + execution_mode, + RunAgentsExecutionMode::Remote { worker_host, .. } if !worker_host.is_empty() + ) +} + /// Renders a pill-shaped toggle switch (36×18) matching the Figma mock. fn render_pill_toggle(is_on: bool, theme: &WarpTheme) -> Box { let thumb_size = 14.; @@ -560,10 +582,28 @@ impl TypedActionView for OrchestrationConfigBlockView { fn handle_action(&mut self, action: &Self::Action, ctx: &mut ViewContext) { match action { OrchestrationConfigBlockAction::ToggleApproval => { + let previous_status = if self.is_approved { + OrchestrationApprovalStatus::Approved + } else { + OrchestrationApprovalStatus::Disapproved + }; self.is_approved = !self.is_approved; if self.is_approved && !self.pickers_initialized { self.ensure_pickers(ctx); } + let new_status = if self.is_approved { + OrchestrationApprovalStatus::Approved + } else { + OrchestrationApprovalStatus::Disapproved + }; + self.emit_plan_config_approval_toggled(previous_status, new_status, ctx); + // Treat the off→on transition as the moment the user + // committed to using orchestration via the plan card. + if matches!(previous_status, OrchestrationApprovalStatus::Disapproved) + && matches!(new_status, OrchestrationApprovalStatus::Approved) + { + self.emit_orchestration_entered(ctx); + } self.apply_field_change(ctx); ctx.notify(); } @@ -645,3 +685,43 @@ impl TypedActionView for OrchestrationConfigBlockView { } } } + +impl OrchestrationConfigBlockView { + fn emit_plan_config_approval_toggled( + &self, + previous_status: OrchestrationApprovalStatus, + new_status: OrchestrationApprovalStatus, + ctx: &mut ViewContext, + ) { + send_telemetry_from_ctx!( + BlocklistOrchestrationTelemetryEvent::PlanConfigApprovalToggled( + PlanConfigApprovalToggledEvent { + conversation_id: self.conversation_id, + plan_id: (!self.plan_id.is_empty()).then(|| self.plan_id.clone()), + previous_status, + new_status, + execution_mode: OrchestrationExecutionModeKind::from_run_agents( + &self.edit_state.execution_mode, + ), + harness: OrchestrationHarnessKind::from_str(&self.edit_state.harness_type), + has_model: !self.edit_state.model_id.trim().is_empty(), + has_environment: env_presence(&self.edit_state.execution_mode), + has_worker_host: host_presence(&self.edit_state.execution_mode), + has_auth_secret: self.edit_state.auth_secret_name.is_some(), + } + ), + ctx + ); + } + + fn emit_orchestration_entered(&self, ctx: &mut ViewContext) { + send_telemetry_from_ctx!( + BlocklistOrchestrationTelemetryEvent::OrchestrationEntered(OrchestrationEnteredEvent { + conversation_id: self.conversation_id, + plan_id: (!self.plan_id.is_empty()).then(|| self.plan_id.clone()), + entry_source: OrchestrationEntrySource::PlanCardApproved, + }), + ctx + ); + } +} From 7938ec5998aa109525339ef24d5ed1a5cb183745 Mon Sep 17 00:00:00 2001 From: advait-m Date: Sun, 17 May 2026 17:35:30 -0700 Subject: [PATCH 2/7] Trim verbose comments on orchestration telemetry additions No behavior change. Drops doc references to sibling symbols that would rot (e.g. "Mirrors RunAgentsExecutionMode", "see WarpUI mouse-state guidance", "reserved badge slot when one is shown") and collapses repetitive multi-paragraph doc comments into single- line summaries. Verified with cargo fmt, cargo check -p warp --lib, and cargo clippy -p warp --lib --tests --all-features -- -D warnings. Co-Authored-By: Oz --- .../agent_view/orchestration_pill_bar.rs | 41 +++++---------- app/src/ai/blocklist/controller.rs | 5 +- .../inline_action/run_agents_card_view.rs | 48 +++++++---------- app/src/ai/blocklist/telemetry.rs | 52 +++++++------------ .../ai/document/orchestration_config_block.rs | 8 +-- 5 files changed, 57 insertions(+), 97 deletions(-) diff --git a/app/src/ai/blocklist/agent_view/orchestration_pill_bar.rs b/app/src/ai/blocklist/agent_view/orchestration_pill_bar.rs index 5cafacbaf0..fa870f97c1 100644 --- a/app/src/ai/blocklist/agent_view/orchestration_pill_bar.rs +++ b/app/src/ai/blocklist/agent_view/orchestration_pill_bar.rs @@ -137,10 +137,6 @@ pub(crate) fn render_agent_avatar_disc( } /// What kind of pill we are rendering, which determines click behavior. -/// -/// `pub(crate)` so it can appear as a field on the public -/// `OrchestrationPillBarAction::PillClicked` variant without leaking a -/// private type through the action enum. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) enum PillKind { Orchestrator, @@ -272,12 +268,9 @@ pub enum OrchestrationPillBarAction { FocusOpenedConversation(AIConversationId), /// Toggle the pin state for the given child conversation. TogglePin(AIConversationId), - /// Pill body was clicked. Routes through `handle_action` (rather - /// than dispatching the navigation `TerminalAction` directly from - /// the click closure) so telemetry can be emitted before the - /// navigation runs. The handler resolves whether the conversation - /// is already open elsewhere and dispatches the appropriate - /// downstream action. + /// Pill body was clicked. Dispatched in lieu of the navigation + /// `TerminalAction` so telemetry can be emitted before the + /// downstream navigation runs. PillClicked { conversation_id: AIConversationId, pill_kind: PillKind, @@ -670,10 +663,10 @@ fn orchestrator_label(orchestrator: &AIConversation) -> String { } impl OrchestrationPillBar { - /// Resolves the `(source_conversation_id, total_pills, total_pinned)` + /// Resolves the source-conversation / total-pills / total-pinned /// triple used to enrich every `PillBarInteraction` event. Returns /// `None` when there is no active orchestration tree to attribute - /// the interaction to (in which case we skip the telemetry). + /// the interaction to. fn pill_bar_telemetry_context( &self, app: &AppContext, @@ -687,10 +680,8 @@ impl OrchestrationPillBar { Some((orchestrator_id, total_pills, total_pinned)) } - /// Looks up the pill kind for `target_id` in the current pill - /// specs. Used by `handle_action` to attribute menu actions (which - /// don't carry the pill kind on the action variant) to either the - /// orchestrator or a child. + /// Pill kind for `target_id` in the current pill specs. Defaults + /// to `Child` if the id is no longer in the bar. fn pill_kind_for(&self, target_id: AIConversationId, app: &AppContext) -> PillBarPillKind { self.pill_specs(app) .and_then(|(_, specs)| { @@ -844,9 +835,8 @@ impl TypedActionView for OrchestrationPillBar { id, ctx, ); - // Mirror the dispatch logic from the old in-place click - // closure: prefer focusing an existing pane that already - // owns this conversation; otherwise switch in place. + // Prefer focusing an existing pane that already owns + // this conversation; otherwise switch in place. let self_terminal_view_id = self.agent_view_controller.as_ref(ctx).terminal_view_id(); let is_open_elsewhere = @@ -1870,15 +1860,10 @@ fn render_pill( if is_selected { return; } - // Route the click through `PillClicked` so the pill bar's - // `handle_action` can emit telemetry before forwarding the - // navigation. The handler resolves whether the conversation is - // already owned by a different visible terminal view (and, if - // so, dispatches `FocusOpenedConversation`) or whether to swap - // the current pane in place via `navigation_action_for_pill`. - // The `self_terminal_view_id` captured into this closure is no - // longer needed because the handler reads it from the pill - // bar's own controller. + // Route the click through `PillClicked` so the pill bar can + // emit telemetry before forwarding the navigation. The + // handler reads `self_terminal_view_id` from its own + // controller, so we no longer need the value captured here. let _ = self_terminal_view_id; ctx.dispatch_typed_action(OrchestrationPillBarAction::PillClicked { conversation_id, diff --git a/app/src/ai/blocklist/controller.rs b/app/src/ai/blocklist/controller.rs index 69a7284903..1caa49c248 100644 --- a/app/src/ai/blocklist/controller.rs +++ b/app/src/ai/blocklist/controller.rs @@ -674,10 +674,7 @@ impl BlocklistAIController { let (query, user_query_mode) = extract_user_query_mode(query); - // Fire `AgentMode.Orchestration.Entered` once per /orchestrate - // user query so we can attribute conversations to the slash - // command entry point alongside plan-card and run_agents-card - // entries (which emit their own variants of this event). + // Attribute /orchestrate queries to the slash-command entry surface. if matches!(user_query_mode, UserQueryMode::Orchestrate) { send_telemetry_from_ctx!( super::telemetry::BlocklistOrchestrationTelemetryEvent::OrchestrationEntered( diff --git a/app/src/ai/blocklist/inline_action/run_agents_card_view.rs b/app/src/ai/blocklist/inline_action/run_agents_card_view.rs index 53ebb634b7..431517d914 100644 --- a/app/src/ai/blocklist/inline_action/run_agents_card_view.rs +++ b/app/src/ai/blocklist/inline_action/run_agents_card_view.rs @@ -212,15 +212,13 @@ pub struct RunAgentsCardView { /// UI-only per-harness model memory so switching harnesses preserves /// the user's previous model selection for each harness. saved_model_per_harness: HashMap, - /// Snapshot of the most recently received raw `RunAgentsRequest` - /// from the LLM stream. Used at decision time to compute which - /// run-wide config fields the user modified before accepting. + /// Snapshot of the latest raw `RunAgentsRequest` from the LLM + /// stream. Used at decision time to diff the run-wide config + /// fields the user changed before accepting. original_tool_call_request: RunAgentsRequest, - /// Set to `true` once `OrchestrationEntered` has been emitted for - /// this card so we don't double-fire across re-renders. + /// Guards `OrchestrationEntered` against double-fires on re-renders. entered_event_emitted: bool, - /// Set to `true` once a terminal decision event has been emitted so - /// we don't double-fire if the user retries. + /// Guards the terminal decision event against double-fires. decision_event_emitted: bool, } /// Computes the `is_denied` flag at construction time. @@ -316,9 +314,8 @@ impl RunAgentsCardView { // (called after streaming finishes and agent_run_configs is populated). let state = RunAgentsEditState::from_request(request); let auto_launched = false; - // Snapshot the raw incoming request so we can compute - // user-applied diffs at Accept time, even if `update_request` - // later replaces `self.state` from a follow-up stream chunk. + // Snapshot the raw incoming request so we can diff against the + // edited state at Accept time. let original_tool_call_request = request.clone(); let reject_keystroke = CTRL_C_KEYSTROKE.clone(); @@ -501,9 +498,8 @@ impl RunAgentsCardView { if self.spawning.is_some() || self.auto_launched || self.is_denied { return; } - // Keep our "raw tool call" snapshot in sync with the latest - // streamed chunk so the diff at Accept time reflects what the - // LLM ultimately emitted. + // Keep the raw-tool-call snapshot in sync with the latest + // streamed chunk. self.original_tool_call_request = request.clone(); let mut new_state = RunAgentsEditState::from_request(request); // Resolve empty fields from the active config (same as in new()). @@ -610,9 +606,8 @@ impl RunAgentsCardView { resolve_interactive_defaults(&mut self.state, &*self.block_model, ctx); oc::repopulate_all_pickers(&mut self.state.orch, &self.handles.pickers, ctx); - // Reaching this branch means the card is about to be shown to - // the user — log it as an orchestration entry point once per - // card instance. + // The card is about to be shown — log it as an orchestration + // entry point. self.emit_orchestration_entered_once(conversation_id, ctx); } @@ -637,9 +632,8 @@ impl RunAgentsCardView { }); } - /// Emits `OrchestrationEntered::RunAgentsCardShown` once per card - /// instance. Idempotent across re-runs of - /// `try_auto_launch_on_stream_complete`. + /// Emits `OrchestrationEntered::RunAgentsCardShown` at most once + /// per card instance. fn emit_orchestration_entered_once( &mut self, conversation_id: AIConversationId, @@ -659,9 +653,7 @@ impl RunAgentsCardView { ); } - /// Emits `RunAgentsCardDecision` once per card instance, capturing - /// the dispatched run-wide config and the set of fields that - /// diverged from the original tool call / active approved config. + /// Emits `RunAgentsCardDecision` at most once per card instance. fn emit_decision(&mut self, decision: RunAgentsCardDecision, ctx: &mut ViewContext) { if self.decision_event_emitted { return; @@ -1066,11 +1058,9 @@ impl TypedActionView for RunAgentsCardView { } } -/// Returns the names of run-wide config fields that differ between the -/// user-edited `state` and the original `RunAgentsRequest` (raw tool -/// call as emitted by the LLM). Field names are stable identifiers from -/// [`orchestration_modified_field`] so they can be joined with the -/// server-side `RunAgentsOutcome.modified_fields` payload. +/// Field names from [`orchestration_modified_field`] that differ +/// between the user-edited `state` and the LLM's original +/// `RunAgentsRequest`. fn diverged_orch_fields( state: &oc::OrchestrationEditState, original: &RunAgentsRequest, @@ -1113,8 +1103,8 @@ fn diverged_orch_fields( } /// Same shape as [`diverged_orch_fields`] but compares against an -/// approved `OrchestrationConfig`. The auth_secret slot is omitted -/// because managed secrets are stored per-user, not on the config. +/// approved `OrchestrationConfig`. auth_secret is omitted: managed +/// secrets are per-user, not stored on the config. fn diverged_orch_fields_against_config( state: &oc::OrchestrationEditState, config: &OrchestrationConfig, diff --git a/app/src/ai/blocklist/telemetry.rs b/app/src/ai/blocklist/telemetry.rs index 926dcfd26e..7653a782ea 100644 --- a/app/src/ai/blocklist/telemetry.rs +++ b/app/src/ai/blocklist/telemetry.rs @@ -71,9 +71,9 @@ pub(crate) enum OrchestrationApprovalStatus { Disapproved, } -/// Run-wide execution mode reported on telemetry payloads. Mirrors -/// `RunAgentsExecutionMode` but is a flat enum so the payload stays -/// metadata-only and never carries an environment id or worker host. +/// Run-wide execution mode reported on telemetry payloads. A flat +/// enum so the payload stays metadata-only and never carries an +/// environment id or worker host. #[derive(Clone, Copy, Debug, Serialize)] #[serde(rename_all = "snake_case")] pub(crate) enum OrchestrationExecutionModeKind { @@ -91,10 +91,9 @@ impl OrchestrationExecutionModeKind { } } -/// Stable bucket for the run-wide harness selection. Maps a raw harness -/// string (which is server-/catalog-controlled but may include strings -/// we don't yet recognize on the client) onto a closed set so the -/// analytics column stays low-cardinality. +/// Closed-set bucket for the run-wide harness selection. Anything +/// unrecognized collapses to `Unknown` to keep the analytics column +/// low-cardinality. #[derive(Clone, Copy, Debug, Serialize)] #[serde(rename_all = "snake_case")] pub(crate) enum OrchestrationHarnessKind { @@ -108,10 +107,6 @@ pub(crate) enum OrchestrationHarnessKind { impl OrchestrationHarnessKind { pub(crate) fn from_str(harness_type: &str) -> Self { - // Match the canonical strings used in `OrchestrationEditState` - // and the orchestration config proto. Anything else collapses - // to `Unknown` so the analytics column stays bounded even if - // the server adds a new harness before the client catalogs it. match harness_type { "oz" | "" => Self::Oz, "claude" | "claude-code" | "claude_code" => Self::ClaudeCode, @@ -123,11 +118,10 @@ impl OrchestrationHarnessKind { } } -/// Stable names for the run-wide config fields that can diverge between +/// Stable names for run-wide config fields that can diverge between /// the dispatched orchestration request and either the original tool -/// call payload or an active approved orchestration config. Mirrors the -/// server's `RunAgentsModifiedField*` constants so the two telemetry -/// streams can be joined on field name. +/// call or an active approved config. Match the server's equivalent +/// field-name constants so the two telemetry streams can be joined. pub(crate) mod orchestration_modified_field { pub const MODEL_ID: &str = "model_id"; pub const HARNESS: &str = "harness"; @@ -170,16 +164,13 @@ pub(crate) struct RunAgentsCardDecisionEvent { pub agent_count: usize, pub harness: OrchestrationHarnessKind, pub execution_mode: OrchestrationExecutionModeKind, - /// Names of run-wide config fields where the dispatched request - /// diverged from the original `RunAgentsRequest` emitted by the LLM. - /// Values are drawn from `orchestration_modified_field` so this can - /// be joined against the server-side `RunAgentsOutcome.modified_fields`. - /// Empty when the user accepted the tool call without edits. + /// Field names from [`orchestration_modified_field`] that diverged + /// between the dispatched request and the LLM's original + /// `RunAgentsRequest`. Empty when the user accepted without edits. pub modified_fields_from_tool_call: Vec<&'static str>, - /// Same shape as `modified_fields_from_tool_call`, but compared - /// against the approved orchestration config snapshot (when one - /// exists). Empty when no active approved config exists or when - /// the dispatched request matches it exactly. + /// Same shape, but compared against the approved orchestration + /// config snapshot. Empty when no approved config exists or the + /// dispatched request matches it. pub modified_fields_from_active_config: Vec<&'static str>, pub had_active_config: bool, #[serde(skip_serializing_if = "Option::is_none")] @@ -190,13 +181,11 @@ pub(crate) struct RunAgentsCardDecisionEvent { #[derive(Clone, Copy, Debug, Serialize)] #[serde(rename_all = "snake_case")] pub(crate) enum OrchestrationEntrySource { - /// User typed `/orchestrate` as the active query mode. + /// `/orchestrate` slash-command mode on a user query. SlashCommandOrchestrate, - /// User toggled the `Use orchestration` switch on the plan card to - /// the approved state. + /// Plan-card `Use orchestration` switch toggled to approved. PlanCardApproved, - /// The LLM emitted a `run_agents` tool call that surfaced the - /// confirmation card (i.e. did not auto-launch). + /// `run_agents` confirmation card was shown (not auto-launched). RunAgentsCardShown, } @@ -236,10 +225,9 @@ pub(crate) struct PillBarInteractionEvent { pub pill_kind: PillBarPillKind, pub total_pills: usize, pub total_pinned: usize, - /// The conversation that hosts the pill bar (the orchestrator's - /// conversation in the active pane). + /// The orchestrator that hosts the pill bar. pub source_conversation_id: AIConversationId, - /// The conversation that the action targets (the pill the user clicked). + /// The pill the action targets. pub target_conversation_id: AIConversationId, } diff --git a/app/src/ai/document/orchestration_config_block.rs b/app/src/ai/document/orchestration_config_block.rs index 79b6ec4a4a..89733d6438 100644 --- a/app/src/ai/document/orchestration_config_block.rs +++ b/app/src/ai/document/orchestration_config_block.rs @@ -34,8 +34,7 @@ use crate::ui_components::blended_colors; use crate::BlocklistAIHistoryModel; use warp_core::ui::theme::WarpTheme; -/// Returns `(had_previous, now_set)` for a remote-mode environment id -/// before/after an edit. +/// True when the mode is remote and `environment_id` is non-empty. fn env_presence(execution_mode: &RunAgentsExecutionMode) -> bool { matches!( execution_mode, @@ -43,6 +42,7 @@ fn env_presence(execution_mode: &RunAgentsExecutionMode) -> bool { ) } +/// True when the mode is remote and `worker_host` is non-empty. fn host_presence(execution_mode: &RunAgentsExecutionMode) -> bool { matches!( execution_mode, @@ -597,8 +597,8 @@ impl TypedActionView for OrchestrationConfigBlockView { OrchestrationApprovalStatus::Disapproved }; self.emit_plan_config_approval_toggled(previous_status, new_status, ctx); - // Treat the off→on transition as the moment the user - // committed to using orchestration via the plan card. + // off→on transition is the orchestration entry point + // for the plan-card surface. if matches!(previous_status, OrchestrationApprovalStatus::Disapproved) && matches!(new_status, OrchestrationApprovalStatus::Approved) { From ea6791b3f456e99c75cb3f6c4f80f536aa2baf61 Mon Sep 17 00:00:00 2001 From: advait-m Date: Sun, 17 May 2026 17:46:02 -0700 Subject: [PATCH 3/7] Fix two telemetry double-emit bugs flagged in review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * PlanCardApproved Entered event was unguarded — toggling the approval switch off and back on re-emitted it. Add a per-view entered_event_emitted flag matching the RunAgentsCardView pattern so the first off→on transition counts; later re-toggles don't. * Pill click on a conversation that's already open elsewhere emitted Switch and then redispatched FocusOpenedConversation, yielding two events per click. Check is_open_elsewhere first; only emit Switch on the in-place branch. Co-Authored-By: Oz --- .../agent_view/orchestration_pill_bar.rs | 17 +++++++++-------- .../ai/document/orchestration_config_block.rs | 10 ++++++++-- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/app/src/ai/blocklist/agent_view/orchestration_pill_bar.rs b/app/src/ai/blocklist/agent_view/orchestration_pill_bar.rs index fa870f97c1..fc29a0c4dd 100644 --- a/app/src/ai/blocklist/agent_view/orchestration_pill_bar.rs +++ b/app/src/ai/blocklist/agent_view/orchestration_pill_bar.rs @@ -829,23 +829,24 @@ impl TypedActionView for OrchestrationPillBar { pill_kind, } => { let id = *conversation_id; - self.emit_pill_bar_interaction( - PillBarActionKind::Switch, - pill_kind.telemetry_kind(), - id, - ctx, - ); - // Prefer focusing an existing pane that already owns - // this conversation; otherwise switch in place. let self_terminal_view_id = self.agent_view_controller.as_ref(ctx).terminal_view_id(); let is_open_elsewhere = is_conversation_open_in_other_visible_view(id, self_terminal_view_id, ctx); if is_open_elsewhere { + // FocusOpenedConversation's handler emits its own + // telemetry; skip the Switch event so one click = + // one event. ctx.dispatch_typed_action( &OrchestrationPillBarAction::FocusOpenedConversation(id), ); } else { + self.emit_pill_bar_interaction( + PillBarActionKind::Switch, + pill_kind.telemetry_kind(), + id, + ctx, + ); ctx.dispatch_typed_action( &PaneHeaderAction::::CustomAction( navigation_action_for_pill(*pill_kind, id), diff --git a/app/src/ai/document/orchestration_config_block.rs b/app/src/ai/document/orchestration_config_block.rs index 89733d6438..8b77f0a010 100644 --- a/app/src/ai/document/orchestration_config_block.rs +++ b/app/src/ai/document/orchestration_config_block.rs @@ -146,6 +146,9 @@ pub struct OrchestrationConfigBlockView { /// saves the config and the resulting event re-enters /// `refresh_from_model`. suppress_refresh: bool, + /// Guards `OrchestrationEntered` against re-emission on subsequent + /// off→on toggles within the same view instance. + entered_event_emitted: bool, } impl OrchestrationConfigBlockView { @@ -244,6 +247,7 @@ impl OrchestrationConfigBlockView { details_mouse_state: MouseStateHandle::default(), saved_model_per_harness: HashMap::new(), suppress_refresh: false, + entered_event_emitted: false, }; if view.is_approved { view.ensure_pickers(ctx); @@ -597,11 +601,13 @@ impl TypedActionView for OrchestrationConfigBlockView { OrchestrationApprovalStatus::Disapproved }; self.emit_plan_config_approval_toggled(previous_status, new_status, ctx); - // off→on transition is the orchestration entry point - // for the plan-card surface. + // First off→on is the plan-card entry point; later + // re-toggles don't re-fire (matches RunAgentsCardView). if matches!(previous_status, OrchestrationApprovalStatus::Disapproved) && matches!(new_status, OrchestrationApprovalStatus::Approved) + && !self.entered_event_emitted { + self.entered_event_emitted = true; self.emit_orchestration_entered(ctx); } self.apply_field_change(ctx); From d69fddbb830bebeb08e991da9c4d1d87a3f929bd Mon Sep 17 00:00:00 2001 From: advait-m Date: Sun, 17 May 2026 18:52:25 -0700 Subject: [PATCH 4/7] telemetry: add client-side AgentProposedConfig event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New BlocklistOrchestrationTelemetryEvent variant fired once per OrchestrationConfigBlockView instance when an agent-authored OrchestrationConfigSnapshot first becomes visible on a plan card. Replaces the server-side AgentMode.Orchestration.CreateConfigInvocation event (removed in warp-server PR #11236) to co-locate config-snapshot telemetry with the other plan-card user-driven events. Payload (cheap to derive client-side): conversation_id, plan_id, harness, execution_mode, has_model, has_environment, has_worker_host Server-side-only fields (validation_outcome, request_id, tool_call_id, task_id, orchestration_version, is_planning_query, is_orchestrate_query) are dropped — they require plumbing the client doesn't naturally have, and validation_outcome is moot client-side since invalid configs never produce a visible snapshot. Co-Authored-By: Oz --- app/src/ai/blocklist/telemetry.rs | 21 ++++++++++++ .../ai/document/orchestration_config_block.rs | 33 +++++++++++++++++-- 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/app/src/ai/blocklist/telemetry.rs b/app/src/ai/blocklist/telemetry.rs index 7653a782ea..ab7ce608b7 100644 --- a/app/src/ai/blocklist/telemetry.rs +++ b/app/src/ai/blocklist/telemetry.rs @@ -12,6 +12,7 @@ pub(crate) enum BlocklistOrchestrationTelemetryEvent { RunAgentsCardDecision(RunAgentsCardDecisionEvent), PillBarInteraction(PillBarInteractionEvent), OrchestrationEntered(OrchestrationEnteredEvent), + AgentProposedConfig(AgentProposedConfigEvent), } #[derive(Clone, Copy, Debug, Serialize)] @@ -197,6 +198,21 @@ pub(crate) struct OrchestrationEnteredEvent { pub entry_source: OrchestrationEntrySource, } +/// Fires when an agent-authored orchestration config snapshot first +/// becomes visible to the user on a plan card. One emission per +/// `OrchestrationConfigBlockView` instance. +#[derive(Debug, Serialize)] +pub(crate) struct AgentProposedConfigEvent { + pub conversation_id: AIConversationId, + #[serde(skip_serializing_if = "Option::is_none")] + pub plan_id: Option, + pub harness: OrchestrationHarnessKind, + pub execution_mode: OrchestrationExecutionModeKind, + pub has_model: bool, + pub has_environment: bool, + pub has_worker_host: bool, +} + #[derive(Clone, Copy, Debug, Serialize)] #[serde(rename_all = "snake_case")] pub(crate) enum PillBarPillKind { @@ -243,6 +259,7 @@ impl TelemetryEvent for BlocklistOrchestrationTelemetryEvent { Self::RunAgentsCardDecision(event) => Some(json!(event)), Self::PillBarInteraction(event) => Some(json!(event)), Self::OrchestrationEntered(event) => Some(json!(event)), + Self::AgentProposedConfig(event) => Some(json!(event)), } } @@ -273,6 +290,7 @@ impl TelemetryEventDesc for BlocklistOrchestrationTelemetryEventDiscriminants { Self::RunAgentsCardDecision => "AgentMode.Orchestration.RunAgentsCardDecision", Self::PillBarInteraction => "AgentMode.Orchestration.PillBarInteraction", Self::OrchestrationEntered => "AgentMode.Orchestration.Entered", + Self::AgentProposedConfig => "AgentMode.Orchestration.AgentProposedConfig", } } @@ -293,6 +311,9 @@ impl TelemetryEventDesc for BlocklistOrchestrationTelemetryEventDiscriminants { Self::OrchestrationEntered => { "Orchestration was activated in a conversation via /orchestrate, a plan-card approval toggle, or a run_agents confirmation card surfacing" } + Self::AgentProposedConfig => { + "An agent-authored orchestration config snapshot first became visible to the user on a plan card" + } } } diff --git a/app/src/ai/document/orchestration_config_block.rs b/app/src/ai/document/orchestration_config_block.rs index 8b77f0a010..d1a61d38fe 100644 --- a/app/src/ai/document/orchestration_config_block.rs +++ b/app/src/ai/document/orchestration_config_block.rs @@ -21,9 +21,9 @@ use crate::ai::blocklist::inline_action::orchestration_controls::{ self as oc, OrchestrationControlAction, OrchestrationEditState, OrchestrationPickerHandles, }; use crate::ai::blocklist::telemetry::{ - BlocklistOrchestrationTelemetryEvent, OrchestrationApprovalStatus, OrchestrationEnteredEvent, - OrchestrationEntrySource, OrchestrationExecutionModeKind, OrchestrationHarnessKind, - PlanConfigApprovalToggledEvent, + AgentProposedConfigEvent, BlocklistOrchestrationTelemetryEvent, OrchestrationApprovalStatus, + OrchestrationEnteredEvent, OrchestrationEntrySource, OrchestrationExecutionModeKind, + OrchestrationHarnessKind, PlanConfigApprovalToggledEvent, }; use crate::ai::blocklist::BlocklistAIHistoryEvent; use crate::ai::document::ai_document_model::AIDocumentModel; @@ -158,6 +158,10 @@ impl OrchestrationConfigBlockView { ctx: &mut ViewContext, ) -> Self { let history = BlocklistAIHistoryModel::as_ref(ctx); + let snapshot_loaded = history + .conversation(&conversation_id) + .and_then(|conv| conv.orchestration_config_for_plan(&plan_id)) + .is_some(); let (edit_state, is_approved) = history .conversation(&conversation_id) .and_then(|conv| { @@ -252,6 +256,12 @@ impl OrchestrationConfigBlockView { if view.is_approved { view.ensure_pickers(ctx); } + // Capture the agent's config proposal once per view instance. + // Gated on `snapshot_loaded` so we don't fire when the view is + // constructed with placeholder defaults (no real snapshot yet). + if snapshot_loaded { + view.emit_agent_proposed_config(ctx); + } view } @@ -730,4 +740,21 @@ impl OrchestrationConfigBlockView { ctx ); } + + fn emit_agent_proposed_config(&self, ctx: &mut ViewContext) { + send_telemetry_from_ctx!( + BlocklistOrchestrationTelemetryEvent::AgentProposedConfig(AgentProposedConfigEvent { + conversation_id: self.conversation_id, + plan_id: (!self.plan_id.is_empty()).then(|| self.plan_id.clone()), + harness: OrchestrationHarnessKind::from_str(&self.edit_state.harness_type), + execution_mode: OrchestrationExecutionModeKind::from_run_agents( + &self.edit_state.execution_mode, + ), + has_model: !self.edit_state.model_id.trim().is_empty(), + has_environment: env_presence(&self.edit_state.execution_mode), + has_worker_host: host_presence(&self.edit_state.execution_mode), + }), + ctx + ); + } } From f58db0d730380017c9556e720310d65228412148 Mon Sep 17 00:00:00 2001 From: advait-m Date: Sun, 17 May 2026 21:28:00 -0700 Subject: [PATCH 5/7] telemetry: drop redundant previous_status from PlanConfigApprovalToggled Per @danielpeng2 review: for a binary toggle, new_status uniquely determines previous_status (always the opposite), so carrying both is redundant. Drops previous_status and renames new_status -> status. The off->on transition gate that fires OrchestrationEntered still uses local was_approved + self.is_approved booleans, so the entry attribution logic is unchanged. Co-Authored-By: Oz --- app/src/ai/blocklist/telemetry.rs | 5 +++-- .../ai/document/orchestration_config_block.rs | 21 ++++++------------- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/app/src/ai/blocklist/telemetry.rs b/app/src/ai/blocklist/telemetry.rs index ab7ce608b7..323ef8930d 100644 --- a/app/src/ai/blocklist/telemetry.rs +++ b/app/src/ai/blocklist/telemetry.rs @@ -137,8 +137,9 @@ pub(crate) struct PlanConfigApprovalToggledEvent { pub conversation_id: AIConversationId, #[serde(skip_serializing_if = "Option::is_none")] pub plan_id: Option, - pub previous_status: OrchestrationApprovalStatus, - pub new_status: OrchestrationApprovalStatus, + /// State after the toggle. The pre-toggle state is the opposite + /// since this event only fires on a binary flip. + pub status: OrchestrationApprovalStatus, pub execution_mode: OrchestrationExecutionModeKind, pub harness: OrchestrationHarnessKind, pub has_model: bool, diff --git a/app/src/ai/document/orchestration_config_block.rs b/app/src/ai/document/orchestration_config_block.rs index d1a61d38fe..aadb0f1fe6 100644 --- a/app/src/ai/document/orchestration_config_block.rs +++ b/app/src/ai/document/orchestration_config_block.rs @@ -596,27 +596,20 @@ impl TypedActionView for OrchestrationConfigBlockView { fn handle_action(&mut self, action: &Self::Action, ctx: &mut ViewContext) { match action { OrchestrationConfigBlockAction::ToggleApproval => { - let previous_status = if self.is_approved { - OrchestrationApprovalStatus::Approved - } else { - OrchestrationApprovalStatus::Disapproved - }; + let was_approved = self.is_approved; self.is_approved = !self.is_approved; if self.is_approved && !self.pickers_initialized { self.ensure_pickers(ctx); } - let new_status = if self.is_approved { + let status = if self.is_approved { OrchestrationApprovalStatus::Approved } else { OrchestrationApprovalStatus::Disapproved }; - self.emit_plan_config_approval_toggled(previous_status, new_status, ctx); + self.emit_plan_config_approval_toggled(status, ctx); // First off→on is the plan-card entry point; later // re-toggles don't re-fire (matches RunAgentsCardView). - if matches!(previous_status, OrchestrationApprovalStatus::Disapproved) - && matches!(new_status, OrchestrationApprovalStatus::Approved) - && !self.entered_event_emitted - { + if !was_approved && self.is_approved && !self.entered_event_emitted { self.entered_event_emitted = true; self.emit_orchestration_entered(ctx); } @@ -705,8 +698,7 @@ impl TypedActionView for OrchestrationConfigBlockView { impl OrchestrationConfigBlockView { fn emit_plan_config_approval_toggled( &self, - previous_status: OrchestrationApprovalStatus, - new_status: OrchestrationApprovalStatus, + status: OrchestrationApprovalStatus, ctx: &mut ViewContext, ) { send_telemetry_from_ctx!( @@ -714,8 +706,7 @@ impl OrchestrationConfigBlockView { PlanConfigApprovalToggledEvent { conversation_id: self.conversation_id, plan_id: (!self.plan_id.is_empty()).then(|| self.plan_id.clone()), - previous_status, - new_status, + status, execution_mode: OrchestrationExecutionModeKind::from_run_agents( &self.edit_state.execution_mode, ), From 16da47aaf0bf3464ac48165adad7ccb647b86946 Mon Sep 17 00:00:00 2001 From: advait-m Date: Sun, 17 May 2026 21:34:48 -0700 Subject: [PATCH 6/7] telemetry: consolidate pill-body-click telemetry under Switch + outcome Per @danielpeng2 review: counting 'how often users click a pill to navigate' previously required UNIONing 'switch' and 'focus_opened_conversation' action values, since a body click would emit one or the other depending on whether the target was already open elsewhere. Non-obvious for analysts and easy to undercount. Pill-body clicks now always emit a single 'switch' action, with a new optional 'switch_outcome' field (closed enum) capturing whether the click navigated in place or focused an existing pane: - switched_in_place - focused_existing_pane The 'focus_opened_conversation' action is preserved for the distinct menu-driven 'Focus pane' gesture (3-dot menu pick) so the two user intents stay separable. Plumbing: extracted the existing focus-existing-pane navigation into a private 'navigate_to_owner_pane' helper so the PillClicked body-click path can reuse the nav without re-emitting menu-driven telemetry. Added 'emit_pill_switch' helper for the Switch+outcome emission so the call site stays one line. Co-Authored-By: Oz --- .../agent_view/orchestration_pill_bar.rs | 195 +++++++++++------- app/src/ai/blocklist/telemetry.rs | 23 +++ 2 files changed, 143 insertions(+), 75 deletions(-) diff --git a/app/src/ai/blocklist/agent_view/orchestration_pill_bar.rs b/app/src/ai/blocklist/agent_view/orchestration_pill_bar.rs index fc29a0c4dd..e00beaf6f3 100644 --- a/app/src/ai/blocklist/agent_view/orchestration_pill_bar.rs +++ b/app/src/ai/blocklist/agent_view/orchestration_pill_bar.rs @@ -47,7 +47,7 @@ use crate::ai::blocklist::agent_view::{AgentViewController, AgentViewControllerE use crate::ai::blocklist::orchestration_topology::descendant_conversation_ids_in_spawn_order; use crate::ai::blocklist::telemetry::{ BlocklistOrchestrationTelemetryEvent, PillBarActionKind, PillBarInteractionEvent, - PillBarPillKind, + PillBarPillKind, PillSwitchOutcome, }; use crate::ai::blocklist::{BlocklistAIHistoryEvent, BlocklistAIHistoryModel}; use crate::ai::harness_display; @@ -699,6 +699,43 @@ impl OrchestrationPillBar { pill_kind: PillBarPillKind, target_conversation_id: AIConversationId, ctx: &mut ViewContext, + ) { + self.emit_pill_bar_interaction_with_outcome( + action, + pill_kind, + target_conversation_id, + None, + ctx, + ); + } + + /// Same as [`Self::emit_pill_bar_interaction`] but stamps a + /// `switch_outcome` on the payload. Use for `Switch` actions where + /// the analyst needs to know whether the click navigated in place + /// or focused an existing pane. + fn emit_pill_switch( + &self, + pill_kind: PillBarPillKind, + target_conversation_id: AIConversationId, + outcome: PillSwitchOutcome, + ctx: &mut ViewContext, + ) { + self.emit_pill_bar_interaction_with_outcome( + PillBarActionKind::Switch, + pill_kind, + target_conversation_id, + Some(outcome), + ctx, + ); + } + + fn emit_pill_bar_interaction_with_outcome( + &self, + action: PillBarActionKind, + pill_kind: PillBarPillKind, + target_conversation_id: AIConversationId, + switch_outcome: Option, + ctx: &mut ViewContext, ) { let Some((source_conversation_id, total_pills, total_pinned)) = self.pill_bar_telemetry_context(ctx) @@ -713,10 +750,78 @@ impl OrchestrationPillBar { total_pinned, source_conversation_id, target_conversation_id, + switch_outcome, }), ctx ); } + + /// Dispatches the focus-existing-pane navigation. Pulled out of + /// the `FocusOpenedConversation` handler so the `PillClicked` + /// handler can reuse the same nav logic without emitting the + /// menu-driven `FocusOpenedConversation` telemetry event. + fn navigate_to_owner_pane(&self, id: AIConversationId, ctx: &mut ViewContext) { + // "Focus pane" is purely a focus operation: the conversation + // already lives in some other visible terminal view (verified + // by `is_conversation_open_in_other_visible_view` at the call + // site) and we just want to move the user's cursor there. We + // deliberately do *not* go through + // `RestoreOrNavigateToConversation`: that path calls + // `set_active_conversation_id` with whichever + // `terminal_view_id` it receives, which would either + // re-transfer ownership to a stale id pulled from + // `AgentConversationsModel::nav_data` or, worse, blank out + // the real owner pane while the conversation pops back into + // the orchestrator. + // + // Resolve the canonical owner directly from + // `BlocklistAIHistoryModel` (the single source of truth) and + // pick the appropriate focus action based on whether the + // owner pane lives in the same pane group as us: + // * Same pane group (sibling pane in this tab) — + // dispatch `TerminalAction::RevealChildAgent`. The pane + // group's handler walks visible terminal panes and calls + // `group.focus_pane(.., true, ctx)` from its own + // `ViewContext`, which actually shifts focus + // to the sibling pane. Going through the workspace's + // `focus_pane` from a different `ViewContext` doesn't + // reliably move focus when the destination is in the + // same pane group. + // * Different pane group (other tab / window) — + // dispatch `WorkspaceAction::FocusTerminalViewInWorkspace`, + // which walks all tabs/windows and activates the + // containing tab as needed. + let owner_view_id = + BlocklistAIHistoryModel::as_ref(ctx).terminal_view_id_for_conversation(&id); + let Some(owner_view_id) = owner_view_id else { + log::warn!( + "navigate_to_owner_pane: no canonical owner for {id:?}; falling back to switch-in-place" + ); + ctx.dispatch_typed_action( + &PaneHeaderAction::::CustomAction( + TerminalAction::SwitchAgentViewToConversation { + conversation_id: id, + }, + ), + ); + return; + }; + let self_pane_group_id = self.agent_view_controller.as_ref(ctx).pane_group_id(); + let owner_pane_group_id = pane_group_id_containing_terminal_view(owner_view_id, ctx); + if owner_pane_group_id.is_some() && owner_pane_group_id == self_pane_group_id { + ctx.dispatch_typed_action( + &PaneHeaderAction::::CustomAction( + TerminalAction::RevealChildAgent { + conversation_id: id, + }, + ), + ); + } else { + ctx.dispatch_typed_action(&WorkspaceAction::FocusTerminalViewInWorkspace { + terminal_view_id: owner_view_id, + }); + } + } } impl TypedActionView for OrchestrationPillBar { @@ -833,20 +938,21 @@ impl TypedActionView for OrchestrationPillBar { self.agent_view_controller.as_ref(ctx).terminal_view_id(); let is_open_elsewhere = is_conversation_open_in_other_visible_view(id, self_terminal_view_id, ctx); + // Pill-body clicks always emit a single `Switch` event, + // with `switch_outcome` capturing what navigation + // actually happened. Analysts can count all pill clicks + // with `action = switch` and slice by outcome — no need + // to UNION with `FocusOpenedConversation` (which is + // reserved for the menu-driven "Focus pane" gesture). + let outcome = if is_open_elsewhere { + PillSwitchOutcome::FocusedExistingPane + } else { + PillSwitchOutcome::SwitchedInPlace + }; + self.emit_pill_switch(pill_kind.telemetry_kind(), id, outcome, ctx); if is_open_elsewhere { - // FocusOpenedConversation's handler emits its own - // telemetry; skip the Switch event so one click = - // one event. - ctx.dispatch_typed_action( - &OrchestrationPillBarAction::FocusOpenedConversation(id), - ); + self.navigate_to_owner_pane(id, ctx); } else { - self.emit_pill_bar_interaction( - PillBarActionKind::Switch, - pill_kind.telemetry_kind(), - id, - ctx, - ); ctx.dispatch_typed_action( &PaneHeaderAction::::CustomAction( navigation_action_for_pill(*pill_kind, id), @@ -862,68 +968,7 @@ impl TypedActionView for OrchestrationPillBar { ctx, ); self.close_menu(ctx); - // "Focus pane" is purely a focus operation: the - // conversation already lives in some other visible - // terminal view (verified by - // `is_conversation_open_in_other_visible_view` before we - // surface this menu item) and we just want to move the - // user's cursor there. We deliberately do *not* go - // through `RestoreOrNavigateToConversation`: that path - // calls `set_active_conversation_id` with whichever - // `terminal_view_id` it receives, which would either - // re-transfer ownership to a stale id pulled from - // `AgentConversationsModel::nav_data` or, worse, blank - // out the real owner pane while the conversation pops - // back into the orchestrator. - // - // Resolve the canonical owner directly from - // `BlocklistAIHistoryModel` (the single source of truth) - // and pick the appropriate focus action based on whether - // the owner pane lives in the same pane group as us: - // * Same pane group (sibling pane in this tab) — - // dispatch `TerminalAction::RevealChildAgent`. The - // pane group's handler walks visible terminal panes - // and calls `group.focus_pane(.., true, ctx)` from - // its own `ViewContext`, which actually - // shifts focus to the sibling pane. Going through - // the workspace's `focus_pane` from a different - // `ViewContext` doesn't reliably move focus when the - // destination is in the same pane group. - // * Different pane group (other tab / window) — - // dispatch `WorkspaceAction::FocusTerminalViewInWorkspace`, - // which walks all tabs/windows and activates the - // containing tab as needed. - let owner_view_id = - BlocklistAIHistoryModel::as_ref(ctx).terminal_view_id_for_conversation(id); - let Some(owner_view_id) = owner_view_id else { - log::warn!( - "FocusOpenedConversation: no canonical owner for {id:?}; falling back to switch-in-place" - ); - ctx.dispatch_typed_action( - &PaneHeaderAction::::CustomAction( - TerminalAction::SwitchAgentViewToConversation { - conversation_id: *id, - }, - ), - ); - return; - }; - let self_pane_group_id = self.agent_view_controller.as_ref(ctx).pane_group_id(); - let owner_pane_group_id = - pane_group_id_containing_terminal_view(owner_view_id, ctx); - if owner_pane_group_id.is_some() && owner_pane_group_id == self_pane_group_id { - ctx.dispatch_typed_action( - &PaneHeaderAction::::CustomAction( - TerminalAction::RevealChildAgent { - conversation_id: *id, - }, - ), - ); - } else { - ctx.dispatch_typed_action(&WorkspaceAction::FocusTerminalViewInWorkspace { - terminal_view_id: owner_view_id, - }); - } + self.navigate_to_owner_pane(*id, ctx); } } } diff --git a/app/src/ai/blocklist/telemetry.rs b/app/src/ai/blocklist/telemetry.rs index 323ef8930d..f93aa11fde 100644 --- a/app/src/ai/blocklist/telemetry.rs +++ b/app/src/ai/blocklist/telemetry.rs @@ -225,9 +225,14 @@ pub(crate) enum PillBarPillKind { #[derive(Clone, Copy, Debug, Serialize)] #[serde(rename_all = "snake_case")] pub(crate) enum PillBarActionKind { + /// User clicked the pill body. See `switch_outcome` for what + /// happened next. Switch, OpenInNewPane, OpenInNewTab, + /// User picked "Focus pane" from a pill's 3-dot menu. Distinct + /// from a pill-body click that resolves to the same outcome + /// (those are `Switch` with `switch_outcome = focused_existing_pane`). FocusOpenedConversation, Stop, Kill, @@ -236,6 +241,19 @@ pub(crate) enum PillBarActionKind { OpenMenu, } +/// Outcome of a pill-body click. Closed enum so future navigation +/// outcomes can be added without splitting `Switch` into multiple +/// action variants again. +#[derive(Clone, Copy, Debug, Serialize)] +#[serde(rename_all = "snake_case")] +pub(crate) enum PillSwitchOutcome { + /// Pill click navigated within the current pane. + SwitchedInPlace, + /// Target conversation was already owned by another visible + /// terminal view; focus moved there instead of switching in place. + FocusedExistingPane, +} + #[derive(Debug, Serialize)] pub(crate) struct PillBarInteractionEvent { pub action: PillBarActionKind, @@ -246,6 +264,11 @@ pub(crate) struct PillBarInteractionEvent { pub source_conversation_id: AIConversationId, /// The pill the action targets. pub target_conversation_id: AIConversationId, + /// Present only when `action == Switch`. Distinguishes whether the + /// pill-body click navigated within the current pane or moved + /// focus to an existing pane already owning the conversation. + #[serde(skip_serializing_if = "Option::is_none")] + pub switch_outcome: Option, } impl TelemetryEvent for BlocklistOrchestrationTelemetryEvent { From f8f22572969a998bcf1ff51b18e144d1166819eb Mon Sep 17 00:00:00 2001 From: advait-m Date: Sun, 17 May 2026 21:48:19 -0700 Subject: [PATCH 7/7] telemetry: drop PlanCardApproved variant; AgentProposedConfig covers plan card Per @danielpeng2 review: PlanCardApproved overlapped with the AgentProposedConfig event we already emit when a snapshot first becomes visible on a plan card. Drops the PlanCardApproved variant from OrchestrationEntrySource, the OrchestrationEntered emission from OrchestrationConfigBlockView's toggle handler, and the entered_event_emitted guard field. Plan-card entry is now exclusively captured by AgentProposedConfig (agent surfaced) + PlanConfigApprovalToggled (user committed), which are both already present in this PR. This aligns the remaining Entered variants (SlashCommandOrchestrate, RunAgentsCardShown) under a more consistent 'orchestration surfaced in this conversation' semantic. Co-Authored-By: Oz --- app/src/ai/blocklist/telemetry.rs | 9 ++++--- .../ai/document/orchestration_config_block.rs | 25 +------------------ 2 files changed, 7 insertions(+), 27 deletions(-) diff --git a/app/src/ai/blocklist/telemetry.rs b/app/src/ai/blocklist/telemetry.rs index f93aa11fde..21385ee719 100644 --- a/app/src/ai/blocklist/telemetry.rs +++ b/app/src/ai/blocklist/telemetry.rs @@ -180,13 +180,16 @@ pub(crate) struct RunAgentsCardDecisionEvent { } /// Surface that first introduced orchestration into a conversation. +/// +/// Plan-card surfacing is intentionally NOT a variant here — that signal +/// is covered by [`AgentProposedConfigEvent`] (fires once per plan card +/// instance when an agent-authored snapshot first becomes visible) plus +/// [`PlanConfigApprovalToggledEvent`] (the user's approval toggle). #[derive(Clone, Copy, Debug, Serialize)] #[serde(rename_all = "snake_case")] pub(crate) enum OrchestrationEntrySource { /// `/orchestrate` slash-command mode on a user query. SlashCommandOrchestrate, - /// Plan-card `Use orchestration` switch toggled to approved. - PlanCardApproved, /// `run_agents` confirmation card was shown (not auto-launched). RunAgentsCardShown, } @@ -333,7 +336,7 @@ impl TelemetryEventDesc for BlocklistOrchestrationTelemetryEventDiscriminants { "User interacted with the orchestration pill bar (switch, pin, open in pane/tab, stop, kill, etc.)" } Self::OrchestrationEntered => { - "Orchestration was activated in a conversation via /orchestrate, a plan-card approval toggle, or a run_agents confirmation card surfacing" + "Orchestration was activated in a conversation via /orchestrate or a run_agents confirmation card surfacing. Plan-card entries are tracked separately via AgentProposedConfig + PlanConfigApprovalToggled." } Self::AgentProposedConfig => { "An agent-authored orchestration config snapshot first became visible to the user on a plan card" diff --git a/app/src/ai/document/orchestration_config_block.rs b/app/src/ai/document/orchestration_config_block.rs index aadb0f1fe6..9fe29dfc8c 100644 --- a/app/src/ai/document/orchestration_config_block.rs +++ b/app/src/ai/document/orchestration_config_block.rs @@ -22,8 +22,7 @@ use crate::ai::blocklist::inline_action::orchestration_controls::{ }; use crate::ai::blocklist::telemetry::{ AgentProposedConfigEvent, BlocklistOrchestrationTelemetryEvent, OrchestrationApprovalStatus, - OrchestrationEnteredEvent, OrchestrationEntrySource, OrchestrationExecutionModeKind, - OrchestrationHarnessKind, PlanConfigApprovalToggledEvent, + OrchestrationExecutionModeKind, OrchestrationHarnessKind, PlanConfigApprovalToggledEvent, }; use crate::ai::blocklist::BlocklistAIHistoryEvent; use crate::ai::document::ai_document_model::AIDocumentModel; @@ -146,9 +145,6 @@ pub struct OrchestrationConfigBlockView { /// saves the config and the resulting event re-enters /// `refresh_from_model`. suppress_refresh: bool, - /// Guards `OrchestrationEntered` against re-emission on subsequent - /// off→on toggles within the same view instance. - entered_event_emitted: bool, } impl OrchestrationConfigBlockView { @@ -251,7 +247,6 @@ impl OrchestrationConfigBlockView { details_mouse_state: MouseStateHandle::default(), saved_model_per_harness: HashMap::new(), suppress_refresh: false, - entered_event_emitted: false, }; if view.is_approved { view.ensure_pickers(ctx); @@ -596,7 +591,6 @@ impl TypedActionView for OrchestrationConfigBlockView { fn handle_action(&mut self, action: &Self::Action, ctx: &mut ViewContext) { match action { OrchestrationConfigBlockAction::ToggleApproval => { - let was_approved = self.is_approved; self.is_approved = !self.is_approved; if self.is_approved && !self.pickers_initialized { self.ensure_pickers(ctx); @@ -607,12 +601,6 @@ impl TypedActionView for OrchestrationConfigBlockView { OrchestrationApprovalStatus::Disapproved }; self.emit_plan_config_approval_toggled(status, ctx); - // First off→on is the plan-card entry point; later - // re-toggles don't re-fire (matches RunAgentsCardView). - if !was_approved && self.is_approved && !self.entered_event_emitted { - self.entered_event_emitted = true; - self.emit_orchestration_entered(ctx); - } self.apply_field_change(ctx); ctx.notify(); } @@ -721,17 +709,6 @@ impl OrchestrationConfigBlockView { ); } - fn emit_orchestration_entered(&self, ctx: &mut ViewContext) { - send_telemetry_from_ctx!( - BlocklistOrchestrationTelemetryEvent::OrchestrationEntered(OrchestrationEnteredEvent { - conversation_id: self.conversation_id, - plan_id: (!self.plan_id.is_empty()).then(|| self.plan_id.clone()), - entry_source: OrchestrationEntrySource::PlanCardApproved, - }), - ctx - ); - } - fn emit_agent_proposed_config(&self, ctx: &mut ViewContext) { send_telemetry_from_ctx!( BlocklistOrchestrationTelemetryEvent::AgentProposedConfig(AgentProposedConfigEvent {