feat(telemetry): track Argent Lens design-review funnel#478
Conversation
Add three privacy-safe telemetry events for the Argent Lens flow that the generic tool:* path can't capture (submission is an HTTP POST, and the await tool is hidden in CLI sessions): - lens:preview_opened — a human loaded the preview UI (deduped per round) - lens:round_completed — the human submitted their picks (once per round) - lens:round_abandoned — a staged round was discarded before submit Payloads carry only aggregate counts, booleans, durations, and the device platform enum — never element names, comment text, variant code, paths, or raw device ids. The store emits privacy-safe aggregate structs decoupled from the telemetry package; structural assignability to the event props is enforced at the track() call site.
- add a preview_opened test asserting per-round re-emit (not fire-once), guarding the round != lastOpenedRound dedup against regression - add a round_completed test with an asymmetric comment case so element_comment_count (2) and skipped_comment_count (1) cannot be swapped - de-flake the "emits once" preview_opened test: the store device singleton survives reset(), so a platform: undefined toEqual was order-dependent (failed under --sequence.shuffle); assert the controlled fields via toMatchObject plus an exact key-set guard instead
latekvo
left a comment
latekvo
left a comment
There was a problem hiding this comment.
Reviewed end-to-end. Built the monorepo (tsc --build clean), ran the telemetry and tool-server suites plus both typecheck:tests gates (green - the one full-suite await-ui-element failure is a pre-existing timing flake that reproduces identically on unmodified main and is untouched here), and drove the real store -> real sanitize() pipeline with adversarial secret-laden inputs.
The privacy contract holds: every field is an aggregate count/boolean/duration or the coarse platform enum (classifyDevice never echoes a raw device id, even for a malformed one), with sanitize() as the enforced backstop. roundCompleted/roundAbandoned are exactly-once and mutually exclusive, the aggregate counts match their doc comments (including the asymmetric element_comment_count vs skipped_comment_count split), the compile-time drift guard between the store stats and the telemetry props genuinely fails on a mismatch, listener registration/removal is symmetric, and emission is throw-safe.
One item worth resolving is left inline: lens:preview_opened undercounts rounds in multi-round argent lens CLI sessions, which skews the open-to-complete funnel these events exist to measure. Approving, since the events are individually correct and privacy-safe and the rest of the change is thoroughly verified.
…indow In an `argent lens` CLI session the preview window opens once and is reused for the whole session: the UI swaps rounds client-side off the /preview/variants poll without ever reloading `/`, and await_user_selection is hidden so the window is never re-foregrounded. `lens:preview_opened` was emitted only on `GET /`, so it fired once per session while lens:round_completed/round_abandoned fire once per round — the open-to-decision funnel showed more decisions than opens for any 2+-round session. Emit from the /variants poll too (the request only an open preview window makes — the agent mutates the store in-process and never speaks HTTP), sharing the lastOpenedRound dedup via a trackPreviewOpenedOnce helper so a round is counted exactly once across both surfaces. The MCP path is unchanged: it respawns the window per round, so `GET /` still fires first and the same-round poll no-ops.
Extend the Argent Lens funnel with two signals the existing events can't answer: - lens:round_completed gains `inspector_used` and `offscreen_revealed` booleans — whether the human opened the "Add comment" inspector or clicked "Show them" (reveal off-screen choices) during the round. The preview UI tracks sticky per-round flags (reset on a new round / after a submit) and rides them on the selection POST; the store forwards them into the round-completed aggregate. annotation_count only counts saved comments, so it cannot measure inspector adoption on its own. - lens:cli_session_started fires once per `argent lens` invocation (the CLI session-begin), carrying agent_choice_count. The generic tool:* path counts per tool call and lens:preview_opened counts per round, so neither can answer how many times the tool was run or the unique-user population. Emitted on every begin (not only a clean transition) so an invocation that starts while a killed prior session is still marked active is not under-counted. All payloads stay privacy-safe aggregates (counts/booleans) — never element names, comment text, agent names, or device ids.
| /** Begin a fresh round, discarding the previous one's proposals/selections. */ | ||
| reset(): void { | ||
| // Drop-off telemetry: a round with staged proposals that is being discarded | ||
| // WITHOUT a submit is an abandonment (window closed, `argent lens` exited |
There was a problem hiding this comment.
In the MCP flow nothing ever routes a walk-away through reset(): its only production callers are autoRollIfCompleted (requires completed, so the !completed guard excludes it) and the setCliSession transition boundary. Closing the preview window touches neither the store nor this method (the window manager and the renderer's unload path never reach it), an await timeout leaves the round staged, and shutdown() in index.ts exits without a reset. So for a non-CLI round the "window closed" trigger listed here only materializes if a CLI session later begins in the same process — the abandonment is then timestamped at that sweep, potentially much later — and never materializes if the process exits first. Verified against a live tool-server on this branch: a staged, never-submitted MCP round produced no lens:round_abandoned through any number of polls, and only emitted (correctly labeled is_cli_session:false) when a CLI session begin happened to reset it. As shipped, the drop-off leg of the funnel effectively measures only CLI sessions; MCP abandonment lands in the unattributable residue of opens minus completions — the very thing this event exists to disambiguate.
|
|
||
| import { TypedEventEmitter } from "@argent/registry"; | ||
| import { TypedEventEmitter, type Platform } from "@argent/registry"; | ||
| import { classifyDevice } from "./device-info"; |
There was a problem hiding this comment.
This branch predates #477 (f38f0e3), which widened telemetry PLATFORMS with tvos/android-tv and routes tool:* events through refineTvPlatform in http.ts so TV sessions stop being counted as iOS/Android. classifyDevice is shape-only and can never return a TV value, and all four lens emit sites (here and in preview.ts) use it directly, so after merging onto current main a Lens round against an Apple TV simulator emits platform:"ios" on lens events while the same session's tool:* events say tvos — reintroducing for the lens funnel exactly the attribution blindness #477 removed. The merge is clean and both the build and the telemetry/tool-server suites stay green on the merged tree (I ran them), so nothing surfaces this at merge time.
| // under-count of the exact metric this event exists for. An end is | ||
| // active=false, so it never emits. Privacy-safe: just the picker size. | ||
| if (active) { | ||
| this.events.emit("cliSessionStarted", { agent_choice_count: this.lensAgents.length }); |
There was a problem hiding this comment.
The kill scenario this comment invokes to justify the unguarded emit is also the scenario in which everything around the emit degrades. After a SIGKILLed argent lens (no active:false POST — and the server has no dead-session detection; the lens-stream close handler only unhooks its own listeners), the store stays cliSession=true, so the next begin has transitioned=false and skips the boundary reset above: the dead session's staged, unsubmitted round is neither counted as abandoned nor cleared, and bleeds into the new invocation's first round, inflating its round_completed element/variant counts. Until that next transition, is_cli_session on preview_opened/round_completed/round_abandoned also reads true for rounds actually driven by an MCP flow, and the un-rolled round keeps lastOpenedRound in preview.ts, so the new session's first preview_opened is suppressed. Reproduced the no-sweep-on-re-begin against a live server on this branch (a staged round survived a second {active:true} POST with no round_abandoned). lens:cli_session_started itself stays correct — it is the surrounding events that mislabel.
| cliSessionChanged: (active: boolean) => void; | ||
| /** | ||
| * Emitted once on a CLI session BEGIN transition (not on end, not on a | ||
| * re-begin while already active) — i.e. once per `argent lens` invocation. |
There was a problem hiding this comment.
This docblock says the event fires "not on a re-begin while already active", but setCliSession deliberately emits on every active=true call — the comment at the emit site and the test ("emits once per begin (every invocation) ... never on session end") both pin the opposite behavior, and a no-transition re-begin does emit (verified live). A future reader taking this docblock at face value and adding a transitioned guard would reintroduce the exact under-count the emit-site comment warns about.
| * telemetry package. Only counts/booleans/durations — never element names, | ||
| * comment text, variant code, file paths, or device identifiers. Structurally | ||
| * assignable to `LensRoundCompletedProps` in @argent/telemetry; if the two | ||
| * drift, the `track()` call site in index.ts fails to compile. |
There was a problem hiding this comment.
"Fails to compile" holds in only one direction. The relay call sites in index.ts pass these stats as variables, so excess-property checking doesn't apply: adding a field to this struct (and its emit) without extending LensRoundCompletedProps compiles cleanly, and sanitize() then silently drops the new field — that drift shows up as a metric that never arrives, not as a compile error. Only missing, renamed, or re-typed fields fail the build.
| router.get("/variants", (_req: Request, res: Response) => { | ||
| res.set("Cache-Control", "no-store"); | ||
| res.json(variantProposalStore.snapshot()); | ||
| // This poll is only ever issued by an OPEN preview window (the agent mutates |
There was a problem hiding this comment.
"Only ever issued by an OPEN preview window" doesn't hold for browsers, which are a documented consumer of this UI: notifyWindowUnavailable tells the agent to hand the human the /preview/ URL, and argent lens prints it to the terminal. The page polls on an unconditional 1.2s setInterval with no visibilitychange gating and no teardown, so a forgotten background tab keeps claiming each later round's preview_opened (the dedup only collapses it to one phantom per round, and the server cannot tell a tab's poll from the Electron renderer's). Every round staged after the tab is abandoned counts as opened with nobody looking — verified that a windowless HTTP poller mints these events against a live server on this branch. This depresses open-to-completed conversion for as long as such a tab lives.
| /** Elements with staged proposals at open time (0 for a CLI up-front open). */ | ||
| element_count: number; | ||
| /** Total variants staged across all elements at open time (0 for a CLI up-front open). */ | ||
| variant_count: number; | ||
| /** Whether an `argent lens` CLI session owns the window (vs the MCP path). */ | ||
| is_cli_session: boolean; | ||
| /** Device platform the variants target; omitted until a device is bound (e.g. a CLI up-front open). */ |
There was a problem hiding this comment.
Two caveats on these fields beyond the documented round-1 zero. For any poll-claimed round — every CLI round after the first, or an MCP window reused within the close delay — the counts freeze at the first ~1.2s poll tick, i.e. typically after the agent's first propose_variant of the round but before the rest, so they systematically under-report relative to fresh-window MCP opens (which fire on GET / after staging settles), and the two populations are not comparable. And platform is not reliably omitted for a CLI up-front open: the store's device deliberately survives reset(), so a CLI session begun after an earlier flow on another device reports that flow's platform — on a live run of this branch, the CLI up-front open emitted platform:"chromium" (stale from a prior round) alongside element_count:0.
| // population for the tool. Privacy-safe: only an aggregate count, no PII. | ||
| export interface LensCliSessionStartedProps { | ||
| /** | ||
| * Coding-agent choices offered in the window's picker (0/1 when no picker is |
There was a problem hiding this comment.
The "0/1" here overstates what the CLI can send: planAgent in lens.ts resolves the agent CLI-side (and the POST sends an empty agents array, count 0) for --agent, for a remembered-and-still-installed choice, and for a single installed agent; the choose list it forwards is only ever length >= 2. So 1 is unreachable via argent lens — real values are 0 (no picker) or >= 2 (picker shown) — and a count of 1 in the data would indicate a hand-crafted POST rather than the documented single-installed-agent case.
| // AGENT acted. They can't tell us whether a HUMAN opened the preview or what they | ||
| // decided. These events capture the human side of the funnel: preview loaded → | ||
| // round decided OR abandoned. All carry only privacy-safe aggregate counts / | ||
| // booleans / durations plus the device `platform` enum — never element names, |
There was a problem hiding this comment.
Telemetry.md (the versioned "Argent Privacy Notice", with its own changes section) enumerates collected-data categories under "What we collect", and none of its bullets covers what these events add: human design-review interaction — per-round comment/annotation counts, decision durations, inspector/off-screen usage, and the installed-coding-agent count. Previous event additions instrumented categories the notice already listed (tool invocations, process lifetime); this is the first that expands the kind of data collected without extending the notice (which is also already stale on platforms — it names "Android, iOS, or a Chromium-based target").
| }); | ||
| }); | ||
|
|
||
| describe("VariantProposalStore — roundAbandoned telemetry event", () => { |
There was a problem hiding this comment.
These cases all trigger the event via a direct reset(), so is_cli_session is only ever asserted false. The flagship path the event was added for — argent lens exited mid-review, i.e. setCliSession(false) with a staged round — is pinned nowhere, and its correct is_cli_session:true attribution depends on ordering inside setCliSession (the transition reset runs before this.cliSession = active is assigned). Hoisting that assignment above the transition block would flip the label on every CLI-exit abandonment with the whole suite staying green. The begin-direction sweep (a leftover non-CLI round swept at session start, labeled false) is equally unpinned.
2 participants
What
Adds three privacy-safe telemetry events for the Argent Lens design-review flow, capturing the human-decision funnel that the generic
tool:*path can't see (submission is an HTTP POST, and theawait_user_selectiontool is hidden inargent lensCLI sessions):lens:preview_opened— a human loaded the preview UI (GET /preview/), deduped per round.lens:round_completed— the human submitted their picks (once per round, never on a resubmit).lens:round_abandoned— a staged round was discarded before submit (window closed,argent lensexited mid-review, or the round superseded).Privacy
Payloads carry only aggregate counts, booleans, durations, and the device
platformenum — never element names, comment text, variant code, file paths, or raw device ids. The store emits privacy-safe aggregate structs that are decoupled from the telemetry package; structural assignability to the event props is enforced at thetrack()call site (drift is a compile error).Event fields
preview_opened:round, element_count, variant_count, is_cli_session, platformround_completed:round, element_count, variant_count, annotation_count, element_comment_count, skipped_comment_count, has_global_comment, is_cli_session, had_parked_await, round_duration_ms, platformround_abandoned:round, element_count, variant_count, had_parked_await, is_cli_session, platformTesting