chore: add notebook/MCP telemetry events

[emrberk]

Adds console telemetry for notebooks and the MCP bridge so we can see how these features are actually used. No functional changes — every emit is a fire-and-forget trackEvent wrapped around existing handlers, batched through the existing IndexedDB send pipeline.

What's tracked

Notebook interactions — create/duplicate, layout mode, cell add/delete/duplicate/move/rename/run/draw, resize and size-reset (gesture end only, never per tick), view changes and maximize, chart settings (open/save/cancel, save-blocked, type change, reset-to-auto), markdown apply, variables popover, result-tab switching, onboarding modal funnel (open/step/close, including Escape/overlay dismissals).

MCP lifecycle — pairing prompted/accepted/declined (with granted permission level), manual pair, connect/disconnect/connect-failed, permissions changes, setup-command copy, agent-changes popper, status popover. Intentional teardowns (user disconnect, re-pair) are excluded from mcp.disconnected so the event only counts real drops.

MCP tool calls — one event per call, named per tool (mcp.add_cell, mcp.run_query, …), emitted after dispatch with an outcome (ok / denied / tool_error / dispatch_error / aborted), a denial reason code where applicable (permission vs. freshness gate), and duration. Unrecognized tool names emit mcp.unknown_tool with the name capped at 100 chars.

Implementation notes

• Outcome classification lives in a new tested util (ConsoleEventTracker/mcpToolEvents.ts): it prefix-matches the known denial message constants and deliberately does not parse serialized tool results.
• Tool-name → event mapping is derived from the enum values; unit tests cover classification, mapping, and the permission-level hierarchy.
• Payloads are bounded enums, counts, and booleans only — no SQL text, table/cell names, variable values, tokens, or error messages leave the browser.

🤖 Generated with Claude Code

[emrberk]

Review: PR #593 — chore: add notebook/MCP telemetry events (level 3)

Scope: 36 files, +868/−70. A telemetry-only PR: fire-and-forget trackEvent calls wrapped around existing handlers, plus three structural additions (a pending-event queue, an MCP connection state-machine, and a classification util). Reviewed with 13 review domains (7 consolidated agents + fresh-context adversarial), per-finding source verification, and all four quality gates. Note: the repo had no node_modules on arrival — the gates only became meaningful after I ran yarn install.

Issues

#	Issue	Category	Severity	Location	Description	Repro	Suggested fix
1	Pending-queue widens start()/stop() pipeline race	Async, timers & cancellation	Moderate	in-diff — ConsoleEventTracker/index.ts:31-45; enabler out-of-diff in sendPipeline.ts:161-173 (non-idempotent startPipeline)	start() sets started=true, then awaits trimToMaxRows + a new loop of up-to-100 await putEvent, then calls startPipeline(config) with no started re-check. stop() is synchronous. If a telemetry-disable (or config-id change) lands inside that now-~100×-wider window: (a) the uploader starts after opt-out (uploads events despite disabled telemetry), and (b) startPipeline sets intervalId=setInterval without clearing the previous one, so an enable→disable→enable leaks the first 5-second interval permanently (duplicate uploads). Queued events can also be written to IDB after stop().	Enable telemetry; while start() is still flushing (large IDB store widens this), toggle telemetry off (and on). Uploader runs post-disable / two intervals run.	Re-check liveness after each await before startPipeline (capture a generation token at entry or if (!started) return after the trim, inside the loop, and before startPipeline); and have startPipeline clear any existing intervalId first.
2	tab.export payload type value changed "notebook"→"Notebook"	Cross-context caller impact	Moderate	in-diff — NotebookToolbar.tsx:284 (BufferType.NOTEBOOK="Notebook", buffers.ts:39)	Old code emitted { type: "notebook" } (lowercase literal); new code emits { type: BufferType.NOTEBOOK } = "Notebook". Any downstream analytics query grouping/filtering tab.export by type=='notebook' silently drops to zero — a discontinuity in an existing time series, with no compile-time signal. Likely an intentional normalization (now matches tab.add/tab.archive/tab.rename), but it's a wire-value change shipped under "no functional changes." (tabs.tsx:131 still emits TAB_EXPORT with no type, so the dimension stays partly inconsistent.)	Open a notebook → Export in the notebook toolbar → inspect queued tab.export: props.type is now "Notebook".	Keep the enum (good for consistency) but call out the value change in the PR description so analytics owners union type IN ('notebook','Notebook'); consider adding type to the tabs.tsx export-all emit too.
3	MCP connection state-machine telemetry has zero test coverage	Test review & coverage	Moderate	in-diff — MCPBridgeClient.test.ts not updated	The PR's most bug-prone change — the hadSession/everHadSession machine and the three emits (MCP_CONNECTED/MCP_DISCONNECTED/MCP_CONNECT_FAILED) — ships with no assertions, though the test file already exercises every branch and mocking trackEvent is trivial. Connect-once, disconnect-exclusion-on-explicit-close, and the failed-vs-disconnected split could all regress silently.	N/A (coverage gap).	Add vi.mock("../../modules/ConsoleEventTracker") cases: two handshakes on one client → MCP_CONNECTED once; disconnect() → no MCP_DISCONNECTED; terminal close before any hello → MCP_CONNECT_FAILED only; terminal close after handshake → MCP_DISCONNECTED{terminal:true}; give-up after MAX reconnects → MCP_CONNECT_FAILED.
4	mcp.connected/disconnected asymmetry + connect_failed conflation	Query execution & data integrity (telemetry semantics)	Minor	in-diff — MCPBridgeClient.ts:345-346,364-370,455-457	MCP_CONNECTED is gated by everHadSession → fires once per client, but a client auto-reconnects without a new instance, so reconnects emit no CONNECTED while each drop emits MCP_DISCONNECTED. Result: disconnected count exceeds connected; the two can't be paired. Separately, MCP_CONNECT_FAILED fires both on a never-connected terminal close and on reconnect-budget exhaustion after a real session, so it can't be read as an initial-connect failure rate.	Flaky bridge: connect → drop → reconnect → drop yields CONNECTED×1, DISCONNECTED×2, and a later give-up logs CONNECT_FAILED.	Confirm intended. If reconnects should be visible, emit MCP_CONNECTED on every hello_ack (or add a reconnect flag); split the post-session give-up into a distinct event (e.g. mcp.reconnect_failed).
5	Consent-window events buffered and flushed on later enable	Persistence & security/privacy	Minor	in-diff — ConsoleEventTracker/index.ts:16-19 + gate QuestProvider/index.tsx:143-150	Pre-PR, trackEvent before start() was a hard no-op (dropped). Now events buffer (≤100) and are flushed to IDB + sent if telemetry later transitions disabled→enabled (enabled/id deps can flip at runtime). Activity from a telemetry-off/undetermined window can be transmitted retroactively. Payloads are bounded enums/counts (no sensitive data), so severity is low, but it's a consent-semantics change. Also: queued events get their created stamped at flush time (db.ts:15), losing original timing.	Disable telemetry → perform notebook/MCP actions → re-enable → buffered off-window events are flushed.	Confirm with privacy. If off-window events must not be sent, drop pending on any disabled state (not just stop()), or gate buffering on a "telemetry not yet determined" flag rather than !started.
6	notebook.duplicate counts attempts; notebook.create counts successes	Cross-context (telemetry semantics)	Minor	in-diff — NotebookToolbar.tsx:270-276	NOTEBOOK_DUPLICATE fires before the await duplicateNotebook, so it counts even when duplication fails (e.g. MAX_TABS → toast, no notebook). NOTEBOOK_CREATE fires only inside addBuffer(...).then(b => if(b)) (success-only). The two "notebook made" events mean different things (attempt vs. success).	At the tab limit, click Duplicate → notebook.duplicate fires but no notebook is created.	Move NOTEBOOK_DUPLICATE into the success branch after duplicateNotebook resolves truthy, mirroring NOTEBOOK_CREATE.
7	notebook.markdown_apply over-counts on blur	React correctness (telemetry semantics)	Minor	in-diff — MarkdownCell.tsx:288-320	apply("blur") fires on any focused→blur while editing, including when nothing was typed and for freshly-added empty markdown cells (which open in edit mode). Inflates the "apply" metric. Mirrors the pre-existing unconditional user_updated_cell emit, so behavior is consistent, but the metric is noisy.	Add a markdown cell, click away without typing → notebook.markdown_apply{method:"blur"} fires.	If "apply" should mean a real edit, gate the blur emit on actual content change (compare draft vs. committed) before trackEvent.
8	Code-quality nits	Code structure, readability & types	Minor	in-diff	(a) MCPBridgeProvider repeats the identical {outcome:"aborted", durationMs: Date.now()-startedAt} block twice and durationMs 4× in one handler — extract a local trackOutcome(extra) closure. (b) classifyToolResult's return type {outcome; reasonCode?} structurally permits {outcome:"ok", reasonCode:"stale"} it never builds — a discriminated union would encode the invariant. (c) ChartSettingsDrawer Escape path inlines its own cancel emit instead of reusing dismiss(), and the dismiss method union omits "escape". (d) hadSession is a present-state flag with a past-tense name (hasSession/sessionLive reads clearer).	N/A	Apply the four small refactors above.

False-positives

Verified against source and dismissed (draft findings that did not survive):

• start() flush loop wedges telemetry if a putEvent rejects (fresh-adversarial add test for query run when cursor position is next to ending semicolon #4): putEvent and trimToMaxRows wrap their bodies in try/catch and return a boolean (db.ts:10-21,57-70) — they never reject, so start() cannot abort mid-flush and always reaches startPipeline. (The interleaving race in wip test for error range #1 is real; this rejection-wedge is not.)
• Cell.tsx onResizeEnd/onDoubleClick inline arrows cause a re-render regression: the receiving ResizeHandle is a plain React.FC, not React.memo, and the previous callbacks (maximizedChartResizeEnd, resetToDefaults) were recreated every render anyway. No memoization to break.
• getCellsSnapshot added to useCellWrapperInteractions deps destabilizes onKeyDown: useNotebookActions() returns a createStableActionProxy memoized with []; getCellsSnapshot is a stable proxy key. Reference is constant for the provider's life.
• handleDragStop/handleResizeStop missing deps → stale layout write: deps are complete; cells is in the resize deps; react-hooks/exhaustive-deps lint passes.
• MarkdownCell.apply stale closure / button+blur double-fire: deps [bufferIdForEvents, cell.id] are complete; the button/shortcut paths don't change isFocused, and setEditing(false) disarms the blur effect — single fire per real transition.
• setState after unmount in addBuffer().then(): the .then bodies contain only void trackEvent(...); no setState.
• MCP toolCall telemetry alters abort/deadline logic or the bridge result: deadlineTimer/inflightAborters cleanup in finally is unchanged, sendToolResult payload is byte-identical, and classifyToolResult can't throw, so the success path can't divert into catch.
• classifyToolResult throws on empty/non-text content: content is always ToolContent[]; firstText's for…of is empty-safe → "".
• classifyToolResult misclassifies due to permission-notice prepend/append: the prepend is gated on !result.isError and every denial is isError:true, so the deny prefix always stays at index 0; the <since_last_check> append only adds at the end, preserving startsWith.
• MCP_CONNECT_FAILED double-fires for one failure: the terminal-close branch returns before scheduleReconnect; scheduleReconnect emits once at the threshold and then stops scheduling.
• Privacy leak in a payload / token leak in MCP telemetry: audited all ~55 added payloads — bounded enums/counts/booleans only; classifyToolResult and applyNotebookStateLayoutMode construct new bounded objects (never spread result text or tool args); MCP_DISCONNECTED carries only {closeCode, terminal}; the pairing token is never referenced by any trackEvent.
• Grid-mode resize region telemetry never fires (grid item lacks data-axis): renderEdgeHandle attaches the RGL ref to the same <Handle> element that carries data-axis (EdgeHandle.tsx:141,144), and RGL forwards that node as onResizeStop's 6th arg (react-grid-layout dist confirmed) — element.dataset.axis resolves correctly.
• Dexie schema/version bump needed / MAX_EVENTS overshoot: no table-shape change (the events table pre-exists); the transient 10,100-row overshoot is self-correcting and consistent with the existing startup-only trim.
• Context provider re-render storm introduced: MCPBridgeProvider's value memo is untouched; no new state feeds any context value; module-level classifier Sets are built once; NotebookToolbar's added cells destructure reads an already-subscribed full-object context.
• NOTEBOOK_CELL_RUN / NOTEBOOK_CELL_SIZE_RESET / onboarding open+close double-fire: verified mutually-exclusive early-returns and that the custom CloseButton is a plain styled.button (programmatic setOpen(false) doesn't trigger Radix onOpenChange).

Summary

Verdict: Approve with minor changes (nothing blocking; two Moderate items worth addressing before merge).

• Regressions/tradeoffs: The PR's "no functional changes" claim is almost accurate. The two real behavioral deltas are (init react components package #2) the tab.export telemetry value change and (Console autocomplete tests #5) the consent-window buffering change — both are analytics/consent-facing, not user-facing. The only genuine defect risk is (wip test for error range #1), a latent start()/stop() race the pending queue amplifies; reachability is narrow (fast telemetry toggle at startup) but it's consent-relevant and the fix is trivial. The privacy contract holds fully.
• Verification: 20 draft findings verified → 8 confirmed, 12+ dismissed as false positives (listed above). The fresh-context adversarial pass independently corroborated wip test for error range #1, init react components package #2, add test for query run when cursor position is next to ending semicolon #4, Console autocomplete tests #5 and added add web-console package #6, raising confidence the structured review didn't miss a major defect.
• In-diff vs out-of-diff: 8 in-diff, 0 in-code out-of-diff. This is expected here and not a cross-context underrun — Agent 1+12 walked every changed exported symbol's callsites (bufferTypeOf ×3, McpSetupCommand, permissionLevelOf, the mcpToolEvents util, the 4 newly-exported denyReason* fns) and confirmed each is safe; the only cross-boundary impact is on downstream analytics dashboards (non-code), captured in init react components package #2.
• Quality gates: yarn typecheck ✅ · yarn lint ✅ · yarn build ✅ · yarn test:unit ✅ (83 files, 1738 tests). No quality-check findings.