Files
open-design/apps/web/tests/observability/chat-correlation-boot.test.ts
lefarcenandGitHub 00c12c6451 feat(web): wire up the ChatPanel observability that was never called (#7930)
* feat(web): make chat telemetry name the run it is talking about

`setChatCorrelation()` has had no production caller since #7518. Every
`client_chat_*` event therefore shipped with an empty correlation block, and
`client_chat_interaction_latency` — the one chat observability event with real
traffic (2,694 events / 30 days) — derives its `streaming` flag from
`chatCorrelation().run_id`. With no producer the flag was false on every event
ever sent, so the panel read "every chat stall happens while the app sits
idle": a self-consistent, wrong story, and the reason the metric was worse
than no metric.

Wire the four producers that already had their values in hand:

- run id + agent id, opened at every run start (`streamViaDaemon`, the next
  physical run of a strategy-task chain, and `reattachDaemonRun` — the path a
  page refresh takes back onto a run still in flight) and closed in the same
  `finally` that settles the stuck-run watchdog. Named helpers, because the
  pair is load-bearing: only an opener and `streaming` is stuck true forever;
  only a closer and it is stuck false.
- conversation id + project id, once `listMessages` resolves.
- release channel, off `/api/version` — the daemon has always answered with
  `channel` and the web read `version` and dropped the rest.
- replay session id, handed over from the posthog-js instance inside
  `loaded()`. This is the only possible source: the ESM build never publishes
  itself as `window.posthog`, so the global-lookup fallback returns undefined
  forever while looking implemented.

No new events. This only makes the traffic already being sent tell the truth.

`model_id` stays absent — it is not in scope at either run-start site, and a
guess would be worse than the gap.

* feat(analytics): record which failure sentence the user actually read

`run_failed_toast` says a user was shown a failure card and names the daemon's
`error_code`. It cannot say what the user READ — those two facts are separated
by a mapping table, and `resolveRunErrorCardDescription` states in as many
words that the table can always be one row short. When it is, the card falls
back to a blank "the task failed" with no diagnosis. That is the worst
impression this surface produces, and it was the one we could not count.

Add two fields to the exposure:

- `message_key` — the i18n key of the mapped copy, or `generic_fallback` when
  the table had no line for it. Always present, never omitted: the fallback
  RATE is the number that matters, and a rate needs a denominator, so an
  omitted key would drop the fallback impressions out of the very count meant
  to measure them.
- `failure_category` — the daemon's own classification off the run's error
  event, defaulting to the enum's existing `unknown` member rather than
  vanishing, so this event stays joinable against `run_finished` and
  `run_recovery_action`.

`message_key` is typed as `string`, not the web's `RunFailureMessageKey`
union: that union lives in apps/web and grows with every copy change, and
pinning it in contracts would make adding a sentence a contracts change.

* feat(web): attach the chat-health monitor to the chat log

`chat-health.ts` has answered four questions since #7518 — how long until the
user can read the conversation, is the DOM growing without bound, who is about
to OOM, how janky is the UI while a run streams — and nothing but its own unit
spec has ever called it. All four tiles were empty in production.

Open a surface on the `.chat-log` element while the Chat tab is showing it,
and tear it down when it goes away. Two effect dependencies, each guarding a
real failure:

- `tab`, because the log is conditionally rendered and `logRef.current` is
  null on the Comments tab — without it, coming back to Chat never reattaches.
- `activeConversationId`, because that div carries no conversation key, so
  React reuses the very same DOM node across a switch. There is no node-level
  signal that the surface should be replaced; the id is the only one. The
  spec pins this by asserting node identity is unchanged AND that a second
  surface opened anyway.

`markFirstPaint` lands together with `setStreamEventCount`, deliberately: a
first-paint duration on its own cannot be attributed, because a slow open is
either "many messages" or "one message with hundreds of tool events behind
it", and only the event count separates them. Rendered-row count uses the same
`:scope > *` definition `client_chat_dom_growth` already uses, so the two
events are comparable.

Run start / end feed the jank window from the daemon provider, which is where
the authoritative run lifecycle lives. Both no-op when no chat surface is
mounted.

Also extracts `isChatVirtualized()` so the renderer and the telemetry read the
same predicate. Two copies of `items.length > THRESHOLD` are identical today
and drift the first time the rule grows a second term — at which point the
telemetry would describe a render mode the renderer no longer uses, silently.

`markChatOpenIntent` is NOT wired here. Hanging it off the two conversation
handlers would only cover `conversation_switch`; `project_switch` has 10+
navigate sites in App.tsx and `cold_boot` passes through no handler at all.
The right home is `router.ts`, which is the single navigation entry point and
can derive the open kind from the route transition. That is its own change.
Until then `open_kind` honestly reads `remount` rather than a fabricated
`cold_boot`.

* fix(web): stop the wiring from reintroducing the bug it removes

Three review findings, all real, all the same family: an event that cannot
name what it measured.

1. `client_chat_stream_health` was going out with an empty `run_id`.
   `chatSurfaceRunEnded()` does not merely bookkeep — it FLUSHES the jank
   window, and the flush spreads `chatCorrelation()`. The terminal path
   cleared the correlation one line earlier, so the one event this PR newly
   enables shipped anonymous: the exact defect the PR exists to remove,
   reintroduced by the fix for it.

   One rule now covers both ends of a run: the chat-health call goes FIRST,
   because it is the one that can emit; the correlation mutation goes second.
   Applied symmetrically at the three run-start sites too — `runStarted()`
   flushes any window a previous run left open (its terminal event never
   arrived), and that flush belongs to the OLD run, so it has to happen before
   the block is repointed. `trackRunTerminal` is unaffected either way; it
   carries its own context and never reads this block.

2. A surface opened mid-run did not know a run was running. Surfaces are
   recreated on every conversation switch and every return to the Chat tab,
   but `chatSurfaceRunStarted` only ever reached the surface that existed when
   the provider fired it. The new surface kept its window shut and dropped
   every long task until the next run start — precisely the stretch a user is
   watching, because they came back to the tab to see something generate.

   `openChatSurface` now adopts the in-flight run. It reads it from the
   correlation context rather than keeping a second flag: `chat-interaction.ts`
   already derives its `streaming` breakdown the same way, and one source
   cannot disagree with itself. No double counting — `runCount` and the window
   clock are per-surface, and adoption only ever runs against the surface
   constructed a line above it. The spec pins this with three long tasks split
   across a mid-run swap and asserts they add up to three, not four.

3. `conversation_id` / `project_id` moved from ProjectView to the ChatPane
   boundary. ChatPane has three hosts — ProjectView, DesignSystemFlow and
   workspace/SideChatTab — feeding one set of observability modules and one
   dashboard. Setting it in a single host left the other two emitting orphan
   rows indistinguishable from genuinely un-correlated ones. Both already pass
   the ids in as props. The effect is declared ahead of the surface-open effect
   so the opening `conversation_open` sample is already stamped.
2026-09-09 08:14:18 +00:00

116 lines
3.9 KiB
TypeScript

// @vitest-environment jsdom
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
/**
* The two boot-time halves of the chat correlation block.
*
* `replay_session_id` and `release_channel` are the fields that turn a
* `client_chat_*` row from a number into something a triager can act on — one
* opens the session replay, the other separates "the packaged stable build is
* janky" from "a dev daemon is janky". Both had a producer sitting one line
* away from a value that was already in hand, and neither was ever called.
*
* `registerChatReplaySessionSource` in particular is a trap with no failure
* mode: we load posthog-js through `await import('posthog-js')`, whose ESM
* build never publishes itself as `window.posthog`, so the global-lookup
* fallback silently returns undefined forever and every replay link on the
* dashboard is dead while the code reads as implemented.
*/
let lastRegisterPayload: Record<string, unknown> | null = null;
vi.mock('posthog-js', () => {
const stub = {
init: (_key: string, config: Record<string, unknown>) => {
const loaded = config.loaded as ((i: unknown) => void) | undefined;
loaded?.(stub);
return stub;
},
register: (payload: Record<string, unknown>) => {
lastRegisterPayload = payload;
},
setPersonProperties: () => undefined,
opt_in_capturing: () => undefined,
opt_out_capturing: () => undefined,
reset: () => undefined,
identify: () => undefined,
get_session_id: () => 'replay-sess-1',
};
return { default: stub };
});
const ORIGINAL_FETCH = globalThis.fetch;
beforeEach(() => {
lastRegisterPayload = null;
vi.resetModules();
globalThis.fetch = vi.fn(async (input: RequestInfo | URL) => {
const url =
typeof input === 'string' ? input : input instanceof URL ? input.href : input.url;
if (url.endsWith('/api/analytics/config')) {
return new Response(
JSON.stringify({
enabled: true,
env: 'local_development',
key: 'phc_test_key',
host: 'https://us.i.posthog.com',
installationId: 'install-123',
}),
{ status: 200, headers: { 'content-type': 'application/json' } },
);
}
if (url.endsWith('/api/version')) {
return new Response(
JSON.stringify({
version: {
version: '1.2.3',
channel: 'prerelease',
packaged: true,
platform: 'darwin',
arch: 'arm64',
},
}),
{ status: 200, headers: { 'content-type': 'application/json' } },
);
}
return new Response('not found', { status: 404 });
}) as unknown as typeof fetch;
});
afterEach(() => {
globalThis.fetch = ORIGINAL_FETCH;
vi.restoreAllMocks();
});
describe('chat correlation — boot-time identity', () => {
it('takes the replay session id from the posthog instance, not from a global', async () => {
const { getAnalyticsClient } = await import('../../src/analytics/client');
const { chatCorrelation } = await import('../../src/observability/chat-context');
// No `window.posthog` on purpose: that is exactly the shape production
// runs in, and the shape the global-lookup fallback cannot serve.
expect((globalThis as { posthog?: unknown }).posthog).toBeUndefined();
await getAnalyticsClient({
anonymousId: 'anon-1',
sessionId: 'sess-1',
clientType: 'web',
locale: 'en',
appVersion: '1.2.3',
});
expect(lastRegisterPayload).not.toBeNull();
expect(chatCorrelation().replay_session_id).toBe('replay-sess-1');
});
it('keeps the release channel the daemon already answers with', async () => {
const { resolveAppVersionForCapture } = await import('../../src/analytics/provider');
const { chatCorrelation } = await import('../../src/observability/chat-context');
await resolveAppVersionForCapture('0.0.0');
expect(chatCorrelation().release_channel).toBe('prerelease');
});
});