Add orchestration client telemetry: entry sources, decisions, pill-bar interactions#11162
Add orchestration client telemetry: entry sources, decisions, pill-bar interactions#11162advait-m wants to merge 6 commits into
Conversation
…ill 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 <oz-agent@warp.dev>
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 <oz-agent@warp.dev>
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR adds client telemetry for orchestration entry points, run_agents decisions, plan-card approval toggles, and pill-bar interactions. The payloads are metadata-only and the security pass did not find secret or UGC leakage in the changed telemetry fields.
Concerns
- Plan-card
OrchestrationEnteredis not idempotent per card instance; repeated off→on toggles can overcountplan_card_approvedentries. - Pill body clicks on conversations already open elsewhere emit both
switchandfocus_opened_conversation, double-counting one interaction and misclassifying the action.
Verdict
Found: 0 critical, 2 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
* 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 <oz-agent@warp.dev>
|
Spoke w Daniel on slack wrt these PRs - moving |
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 <oz-agent@warp.dev>
danielpeng2
left a comment
danielpeng2
left a comment
There was a problem hiding this comment.
approving to unblock. my main question is around the right places to track in the Entered event.
| /// Plan-card `Use orchestration` switch toggled to approved. | ||
| PlanCardApproved, |
There was a problem hiding this comment.
Is PlanCardApproved the right event to track here? What if the user toggles the state to disapproved then back to approved, would we track an Entered event again?
Feels like here we'd want to track the plan card being shown (basically the AgentProposedConfig event we're also adding).
| 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 { |
There was a problem hiding this comment.
thoughts on whether this makes the telemetry harder to work with? might not be clear that to track all switches, we have to join FocusOpenedConversation and the Switch events
There was a problem hiding this comment.
yeah, I originally did have it emitting both but Oz comment made me consider switching it to this 😅. I agree emitting both makes telem easier to query tho.
Let me actually simplify this and make switch have switched_in_place, focused_existing_pane as an enum field (only 2 for now, but in case we need to add more in the future - and enum is a bit easier to understand than a bool here). This better matches semantically too.
Note that this does require some logic to be extracted out to a helper function (for navigate_to_owner_pane) - doing that now.
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 <oz-agent@warp.dev>
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 <oz-agent@warp.dev>
2 participants
Loom: https://www.loom.com/share/9dc498c4adae4fd39bc9b0183b57575a
Resolves: https://linear.app/warpdotdev/issue/QUALITY-683/do-a-telemetry-pass-on-client-server
Description
Adds client-side telemetry to instrument every orchestration entry point and the run-wide config / pill-bar interactions that follow. Prior to this change, the only orchestration telemetry on the client was
AgentMode.Orchestration.TeamAgentCommunicationFailed(error path only); we had no visibility into how often users entered orchestration, which surface they entered from, how they edited the configs, or how they used the pill bar. Server-sideAgentMode.OrchestrateOutcometold us that the launched config diverged from the tool call, but not which fields, and there was no analog ofis_planning_queryfor/orchestratequeries.This PR is the client half. The server half is up at warpdotdev/warp-server#11236 — it adds
is_orchestrate_queryeverywhere, the launched config fingerprint onOrchestrateOutcome, andparent_execution_locationonStartAgentInvocation. The pair gives us closed-enum, UGC-free coverage across every orchestration surface.Design principles
OrchestrationHarnessKindandOrchestrationExecutionModeKindbucket harness strings and execution modes into a closed set so the analytics columns stay low-cardinality even if the server adds a new harness.Enteredand oneDecisionevent (guarded byentered_event_emitted/decision_event_emitted) so re-renders don't double-count.has_environment,had_previous,now_set) to avoid leaking workspace topology through analytics.New events
All variants live on
BlocklistOrchestrationTelemetryEventinapp/src/ai/blocklist/telemetry.rs.AgentMode.Orchestration.PlanConfigApprovalToggledFires from
OrchestrationConfigBlockView::handle_actionwhen the user toggles the Use orchestration switch on a plan card. The user-driven counterpart toAgentProposedConfig(below), which captures agent-driven config snapshots.conversation_idplan_idstatusapproved | disapprovedexecution_modelocal | remoteharnessoz | claude_code | codex | open_code | gemini | unknownhas_model,has_environment,has_worker_host,has_auth_secretAgentMode.Orchestration.EnteredFires once per orchestration entry, attributing the source. Three call sites:
BlocklistAIController::send_query— when the extractedUserQueryMode == Orchestrate(slash command). Emits withentry_source = slash_command_orchestrate.OrchestrationConfigBlockView::handle_action— on thedisapproved → approvedtoggle transition only. Emits withentry_source = plan_card_approved.RunAgentsCardView::try_auto_launch_on_stream_complete— once per card instance when the card is actually rendered (i.e. not in the auto-launched path). Emits withentry_source = run_agents_card_shown.conversation_idplan_identry_sourceslash_command_orchestrate | plan_card_approved | run_agents_card_shownAgentMode.Orchestration.RunAgentsCardDecisionFires once per
RunAgentsCardViewinstance on Accept, Accept without orchestration, or Reject (guarded bydecision_event_emitted).conversation_id,plan_iddecisionaccept | accept_without_orchestration | rejectagent_countharness,execution_modemodified_fields_from_tool_callVec<&'static str>RunAgentsRequest. Values are stable identifiers inorchestration_modified_field::{MODEL_ID, HARNESS, EXECUTION_MODE, ENVIRONMENT_ID, WORKER_HOST, AUTH_SECRET}— the same strings the server'sRunAgentsOutcome.modified_fields(RunAgentsModifiedField*in the server PR) uses so the two streams join 1:1.modified_fields_from_active_configVec<&'static str>had_active_config,active_config_statusapproved | disapprovedThis event answers questions like:
To compute the per-edit diff we snapshot
original_tool_call_requeston everyupdate_requestcall (so it always reflects the final streamed request) and diff againststate.orchat decision time. Seediverged_orch_fields/diverged_orch_fields_against_configinrun_agents_card_view.rs.AgentMode.Orchestration.AgentProposedConfigFires once per
OrchestrationConfigBlockViewinstance when an agent-authoredOrchestrationConfigSnapshotfirst becomes visible to the user on a plan card. Replaces the server-sideCreateConfigInvocationevent so all plan-card telemetry sits on the client. Server-only fields from the earlier design (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 invalid configs never surface to the client anyway (no snapshot is produced).conversation_idplan_idharnessoz | claude_code | codex | open_code | gemini | unknownexecution_modelocal | remotehas_model,has_environment,has_worker_hostAgentMode.Orchestration.PillBarInteractionFires from
OrchestrationPillBar::handle_actionviaemit_pill_bar_interaction. Captures every meaningful pill-bar interaction; hover events are intentionally not tracked.actionswitch | open_in_new_pane | open_in_new_tab | focus_opened_conversation | stop | kill | toggle_pin_on | toggle_pin_off | open_menu.switchcovers all pill-body clicks (slice byswitch_outcome);focus_opened_conversationis reserved for the menu-driven "Focus pane" gesture.pill_kindorchestrator | childtotal_pills,total_pinnedpill_specsat emit timesource_conversation_idtarget_conversation_idswitch_outcomeswitched_in_place | focused_existing_paneaction = switch; closed enum so future outcomes don't require a new action variantTo make
Switch(pill click) telemeterable, the body click handler now routes through a newOrchestrationPillBarAction::PillClicked { conversation_id, pill_kind }rather than dispatching the navigationTerminalActiondirectly from the click closure.handle_actionemits the telemetry, then reproduces the original "focus opened pane vs. switch in place" logic. Net behavior is unchanged.All body clicks always emit a single
switchevent, withswitch_outcomecapturing the resulting navigation — "how often do users click pills?" isWHERE action = 'switch', no UNION needed. The menu-driven "Focus pane" gesture stays its ownfocus_opened_conversationevent so the two user intents remain separable. The shared focus-existing-pane navigation lives in a privatenavigate_to_owner_panehelper reused by both code paths.No labels (which can be user/agent-generated agent names) are ever sent.
Pre-existing telemetry that stays as-is
AgentMode.Orchestration.TeamAgentCommunicationFailed— unchanged. Still the only client-side error-path telemetry.Cross-side coverage map
create_orchestration_configAgentProposedConfig(fires when the snapshot first becomes visible)PlanConfigApprovalToggledEntered { entry_source: plan_card_approved }OrchestrateOutcome(this PR's paired server change adds launched fingerprint +is_orchestrate_query)Entered { entry_source: run_agents_card_shown }RunAgentsCardDecisionEntered { entry_source: slash_command_orchestrate }is_orchestrate_querynow present onOrchestrateOutcomeandStartAgentInvocationPillBarInteractionStartAgentInvocation(paired PR addsparent_execution_location+is_orchestrate_query)Companion server PR
warpdotdev/warp-server#11236 — additive, can ship independently in either order.
Co-Authored-By: Oz oz-agent@warp.dev