mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-10-30 17:27:09 +00:00
feat(studio): a per-page tab id, and the hash-validation verdict (#4224)
Two telemetry gaps, both pure instrumentation with no behaviour change.
`tab_id` — distinct_id identifies a browser, not a page. Several
investigations stalled on "two tabs, or one tab switching?", because a burst
of events alternating between two project hashes is indistinguishable from a
user moving between projects, and `session_start` fires once per page load so
counting it cannot tell them apart either. One id per page load, from one
shared module, attached by BOTH transports (`studio:*` and `studio_*`) — those
two families already drifted once on how `agent_runtime` is encoded.
Deliberately not persisted. Storage would survive a reload and quietly turn
this into a tab-session id, changing what every query means, and blocked or
cleared storage would make the property intermittently absent — which reads as
"old client" rather than "storage unavailable".
`project_hash_validated` — a tab that keeps its hash because the check itself
failed ("unknown") is currently indistinguishable in production from one that
never ran the check. Different causes, different fixes, one silent bucket.
Emits `stage` (mount | hashchange), `outcome` (ok | missing | unknown), and
the HTTP status when the server answered.
Verified: 8 mutants, 8 caught — including dropping tab_id from either
transport, regenerating it per read, persisting it to storage, emptying it in
the no-DOM fallback, and collapsing "unknown" into "missing". Full studio
suite 5,314 passed.
Co-authored-by: Claude Code <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Code
parent
ab6cbb383e
commit
7abbe49638
@@ -2,8 +2,11 @@
|
||||
import { act } from "react";
|
||||
import { createRoot } from "react-dom/client";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { trackStudioEvent } from "../utils/studioTelemetry";
|
||||
import { useServerConnection } from "./useServerConnection";
|
||||
|
||||
vi.mock("../utils/studioTelemetry", () => ({ trackStudioEvent: vi.fn() }));
|
||||
|
||||
(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
|
||||
|
||||
const captured: { projectId: string | null } = { projectId: null };
|
||||
@@ -55,6 +58,7 @@ describe("useServerConnection hash project id", () => {
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllGlobals();
|
||||
vi.mocked(trackStudioEvent).mockClear();
|
||||
});
|
||||
|
||||
it("keeps a hash id the server can resolve", async () => {
|
||||
@@ -92,3 +96,68 @@ describe("useServerConnection hash project id", () => {
|
||||
await act(async () => root.unmount());
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The verdict event. Until it existed, a tab that kept its hash because the
|
||||
* check itself failed was indistinguishable in production from one that never
|
||||
* ran the check — different causes, different fixes, one silent bucket.
|
||||
*/
|
||||
describe("project_hash_validated", () => {
|
||||
const verdictCalls = () =>
|
||||
vi.mocked(trackStudioEvent).mock.calls.filter(([name]) => name === "project_hash_validated");
|
||||
|
||||
beforeEach(() => {
|
||||
captured.projectId = null;
|
||||
window.location.hash = "";
|
||||
vi.mocked(trackStudioEvent).mockClear();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
it("reports an ok verdict with its status", async () => {
|
||||
stubFetch({ status: 200 });
|
||||
const root = await renderWithHash("#project/stale-or-not");
|
||||
expect(verdictCalls()).toEqual([
|
||||
["project_hash_validated", { stage: "mount", outcome: "ok", status: 200 }],
|
||||
]);
|
||||
await act(async () => root.unmount());
|
||||
});
|
||||
|
||||
it("reports a missing verdict with the 404", async () => {
|
||||
stubFetch({ status: 404 });
|
||||
const root = await renderWithHash("#project/deleted-project");
|
||||
expect(verdictCalls()).toEqual([
|
||||
["project_hash_validated", { stage: "mount", outcome: "missing", status: 404 }],
|
||||
]);
|
||||
await act(async () => root.unmount());
|
||||
});
|
||||
|
||||
it("reports unknown with no status when the check is rejected", async () => {
|
||||
// The point of the event: this case LOOKS like success from outside (the
|
||||
// hash is kept) but is a failed check. Only the verdict says so.
|
||||
stubFetch("reject");
|
||||
const root = await renderWithHash("#project/unreachable-check");
|
||||
expect(verdictCalls()).toEqual([
|
||||
["project_hash_validated", { stage: "mount", outcome: "unknown" }],
|
||||
]);
|
||||
await act(async () => root.unmount());
|
||||
});
|
||||
|
||||
it("reports unknown — not missing — for a 5xx", async () => {
|
||||
stubFetch({ status: 500 });
|
||||
const root = await renderWithHash("#project/server-erroring");
|
||||
expect(verdictCalls()).toEqual([
|
||||
["project_hash_validated", { stage: "mount", outcome: "unknown", status: 500 }],
|
||||
]);
|
||||
await act(async () => root.unmount());
|
||||
});
|
||||
|
||||
it("emits nothing when there is no hash id to validate", async () => {
|
||||
stubFetch({ status: 200 });
|
||||
const root = await renderWithHash("");
|
||||
expect(verdictCalls()).toEqual([]);
|
||||
await act(async () => root.unmount());
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { useEffect, useState } from "react";
|
||||
import { buildProjectHash, parseProjectIdFromHash } from "../utils/projectRouting";
|
||||
import { useMountEffect } from "./useMountEffect";
|
||||
import { trackStudioEvent } from "../utils/studioTelemetry";
|
||||
|
||||
interface ServerConnectionState {
|
||||
projectId: string | null;
|
||||
@@ -28,16 +29,36 @@ interface ServerConnectionState {
|
||||
* `/api/projects` list would be wrong twice over: that list omits session ids
|
||||
* (which resolve fine) and skips project dirs without an `index.html`.
|
||||
*/
|
||||
async function resolveHashProject(id: string): Promise<"ok" | "missing" | "unknown"> {
|
||||
type ProjectHashVerdict = {
|
||||
outcome: "ok" | "missing" | "unknown";
|
||||
/** HTTP status when the server answered; absent on a rejected request. */
|
||||
status?: number;
|
||||
};
|
||||
|
||||
async function resolveHashProject(id: string): Promise<ProjectHashVerdict> {
|
||||
try {
|
||||
const res = await fetch(`/api/projects/${encodeURIComponent(id)}`);
|
||||
if (res.ok) return "ok";
|
||||
return res.status === 404 ? "missing" : "unknown";
|
||||
if (res.ok) return { outcome: "ok", status: res.status };
|
||||
return { outcome: res.status === 404 ? "missing" : "unknown", status: res.status };
|
||||
} catch {
|
||||
return "unknown";
|
||||
return { outcome: "unknown" };
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* What the hash validation decided. Without this, a tab that kept a hash
|
||||
* because the check itself failed ("unknown") is indistinguishable from one
|
||||
* that never ran the check — the two have different fixes and, until this
|
||||
* event, no way to tell them apart in production.
|
||||
*/
|
||||
function reportHashVerdict(stage: "mount" | "hashchange", verdict: ProjectHashVerdict): void {
|
||||
trackStudioEvent("project_hash_validated", {
|
||||
stage,
|
||||
outcome: verdict.outcome,
|
||||
...(verdict.status === undefined ? {} : { status: verdict.status }),
|
||||
});
|
||||
}
|
||||
|
||||
export function useServerConnection(): ServerConnectionState {
|
||||
const [projectId, setProjectId] = useState<string | null>(null);
|
||||
const [resolving, setResolving] = useState(true);
|
||||
@@ -69,11 +90,12 @@ export function useServerConnection(): ServerConnectionState {
|
||||
// Telemetry after the read-reason split: 120 http_error/404 reads
|
||||
// across 5 users in 24h, ~24 each, never recovering.
|
||||
if (hashProjectId) {
|
||||
const state = await resolveHashProject(hashProjectId);
|
||||
const verdict = await resolveHashProject(hashProjectId);
|
||||
if (cancelled) return;
|
||||
reportHashVerdict("mount", verdict);
|
||||
// "unknown" keeps the old behaviour: a transient failure must not
|
||||
// rewrite the user's hash out from under a valid project.
|
||||
if (state !== "missing") {
|
||||
if (verdict.outcome !== "missing") {
|
||||
setProjectId(hashProjectId);
|
||||
setWaitingForServer(false);
|
||||
return;
|
||||
|
||||
@@ -0,0 +1,49 @@
|
||||
// @vitest-environment happy-dom
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
// The `studio_*` transport carries browser metadata built here, and this is its
|
||||
// only test. tab_id has a module test of its own, but nothing there fails if
|
||||
// this file stops attaching it — the two are separate facts.
|
||||
describe("browser system meta", () => {
|
||||
// Both modules are imported AFTER resetModules so they share one tabId
|
||||
// instance; a top-level import would be a different copy of the memo.
|
||||
beforeEach(() => {
|
||||
vi.resetModules();
|
||||
});
|
||||
|
||||
async function load() {
|
||||
const [{ getBrowserSystemMeta }, tab] = await Promise.all([
|
||||
import("./system"),
|
||||
import("./tabId"),
|
||||
]);
|
||||
return { getBrowserSystemMeta, resolveTabId: tab.resolveTabId };
|
||||
}
|
||||
|
||||
it("carries the tab id, from the same source as the studio:* transport", async () => {
|
||||
const { getBrowserSystemMeta, resolveTabId } = await load();
|
||||
expect(getBrowserSystemMeta().tab_id).toBe(resolveTabId());
|
||||
});
|
||||
|
||||
it("keeps the meta memoized, so one page reports one tab id", async () => {
|
||||
const { getBrowserSystemMeta } = await load();
|
||||
const first = getBrowserSystemMeta();
|
||||
expect(getBrowserSystemMeta()).toBe(first);
|
||||
expect(getBrowserSystemMeta().tab_id).toBe(first.tab_id);
|
||||
});
|
||||
|
||||
it("has a tab id in the no-DOM fallback, not an empty string", async () => {
|
||||
// The empty-meta branch is taken on SSR. An empty or missing tab_id would
|
||||
// read in PostHog as "this event predates the property" rather than "this
|
||||
// event has no page", which is the encoding mistake this property exists
|
||||
// to avoid. Stubbing the globals is what actually reaches that branch —
|
||||
// happy-dom otherwise satisfies the `typeof window` check every time.
|
||||
vi.stubGlobal("navigator", undefined);
|
||||
vi.stubGlobal("window", undefined);
|
||||
const { getBrowserSystemMeta } = await load();
|
||||
const meta = getBrowserSystemMeta();
|
||||
expect(meta.user_agent).toBe("");
|
||||
expect(typeof meta.tab_id).toBe("string");
|
||||
expect(meta.tab_id.length).toBeGreaterThan(0);
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
});
|
||||
@@ -5,6 +5,7 @@
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
import { agentRuntimeProperty } from "./agentRuntime";
|
||||
import { tabIdProperty } from "./tabId";
|
||||
|
||||
export interface BrowserSystemMeta {
|
||||
user_agent: string;
|
||||
@@ -17,6 +18,8 @@ export interface BrowserSystemMeta {
|
||||
studio_version: string;
|
||||
/** Which coding agent is driving this Studio, or null when a person is. */
|
||||
agent_runtime: string;
|
||||
/** One id per page load. distinct_id identifies a browser, not a page. */
|
||||
tab_id: string;
|
||||
}
|
||||
|
||||
const EMPTY_META: BrowserSystemMeta = {
|
||||
@@ -29,6 +32,7 @@ const EMPTY_META: BrowserSystemMeta = {
|
||||
is_mobile: false,
|
||||
studio_version: "dev",
|
||||
agent_runtime: "none",
|
||||
tab_id: "unknown",
|
||||
};
|
||||
|
||||
let cached: BrowserSystemMeta | null = null;
|
||||
@@ -54,6 +58,8 @@ export function getBrowserSystemMeta(): BrowserSystemMeta {
|
||||
// Same value the `studio:*` transport attaches, from the same accessor —
|
||||
// two families that disagreed here would split every agent breakdown.
|
||||
agent_runtime: agentRuntimeProperty(),
|
||||
// Same shared module as the `studio:*` transport, for the same reason.
|
||||
tab_id: tabIdProperty(),
|
||||
};
|
||||
return cached;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,43 @@
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { resetTabIdForTests, resolveTabId, tabIdProperty } from "./tabId";
|
||||
|
||||
afterEach(() => {
|
||||
resetTabIdForTests();
|
||||
});
|
||||
|
||||
describe("tab id", () => {
|
||||
it("is stable for the life of the page", () => {
|
||||
// The whole value of the property: every event from one page load must
|
||||
// carry the SAME id, or a burst still reads as several sources.
|
||||
const first = resolveTabId();
|
||||
expect(resolveTabId()).toBe(first);
|
||||
expect(tabIdProperty()).toBe(first);
|
||||
});
|
||||
|
||||
it("is not empty, since an empty value would read as an absent one", () => {
|
||||
expect(resolveTabId().length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it("mints a new id after a fresh page load", () => {
|
||||
// resetTabIdForTests stands in for a reload: the memo is all that survives
|
||||
// a page, so clearing it is exactly what a reload does.
|
||||
const first = resolveTabId();
|
||||
resetTabIdForTests();
|
||||
expect(resolveTabId()).not.toBe(first);
|
||||
});
|
||||
|
||||
it("never touches storage", () => {
|
||||
// Deliberate: a stored id would drift into a tab-session id, silently
|
||||
// changing what every query means. This asserts the intent directly rather
|
||||
// than inferring it, so adding a read OR a write fails here.
|
||||
const seen: string[] = [];
|
||||
vi.stubGlobal("localStorage", {
|
||||
getItem: (k: string) => (seen.push(`get:${k}`), null),
|
||||
setItem: (k: string) => void seen.push(`set:${k}`),
|
||||
removeItem: (k: string) => void seen.push(`remove:${k}`),
|
||||
});
|
||||
resolveTabId();
|
||||
expect(seen).toEqual([]);
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,44 @@
|
||||
/**
|
||||
* A per-page-load identifier, attached to every Studio event from BOTH
|
||||
* transports (`studio:*` via utils/studioTelemetry.ts, `studio_*` via
|
||||
* telemetry/system.ts).
|
||||
*
|
||||
* Why it exists: `distinct_id` identifies a browser, not a page. Several
|
||||
* investigations have stalled on "two tabs or one tab switching?" — a burst of
|
||||
* events alternating between two project hashes is indistinguishable from a
|
||||
* user moving between projects, and `session_start` fires once per page LOAD
|
||||
* (main.tsx), so counting it cannot tell them apart either. Nothing in the
|
||||
* payload named the page. This does.
|
||||
*
|
||||
* Semantics, stated plainly because the name invites the wrong reading: one id
|
||||
* per PAGE LOAD. A hard reload mints a new one. It is not "the same tab across
|
||||
* reloads" — that is a different, harder question, and nobody has needed it yet.
|
||||
*
|
||||
* Deliberately not persisted. Storage would survive a reload and turn this into
|
||||
* a tab-session id, silently changing what every query means; and a blocked or
|
||||
* cleared storage would make the property intermittently absent, which reads as
|
||||
* "old client" rather than "storage unavailable".
|
||||
*
|
||||
* Memoized like agentRuntime.ts and distinctId.ts: fixed for the life of the
|
||||
* page, so a per-event caller does not pay for generation on every emit.
|
||||
*/
|
||||
|
||||
import { generateId } from "../utils/generateId";
|
||||
|
||||
let cached: string | null = null;
|
||||
|
||||
/** The page's id. Generated once, on first read. */
|
||||
export function resolveTabId(): string {
|
||||
if (cached === null) cached = generateId();
|
||||
return cached;
|
||||
}
|
||||
|
||||
/** The same value as a telemetry property, for the `studio_*` transport's meta. */
|
||||
export function tabIdProperty(): string {
|
||||
return resolveTabId();
|
||||
}
|
||||
|
||||
/** Test seam: the memo would otherwise leak between cases in one module load. */
|
||||
export function resetTabIdForTests(): void {
|
||||
cached = null;
|
||||
}
|
||||
@@ -66,6 +66,20 @@ describe("studioTelemetry — shared opt-out and canary properties", () => {
|
||||
});
|
||||
});
|
||||
|
||||
// The property is useless if the transport does not attach it: the module
|
||||
// has its own test, but nothing there fails if this line is deleted.
|
||||
it("stamps every event with a tab id that is stable within the page", async () => {
|
||||
trackStudioEvent("first");
|
||||
trackStudioEvent("second");
|
||||
const events = await sentEvents();
|
||||
expect(events).toHaveLength(2);
|
||||
const tabIds = events.map((e) => (e["properties"] as Record<string, unknown>)["tab_id"]);
|
||||
expect(typeof tabIds[0]).toBe("string");
|
||||
expect((tabIds[0] as string).length).toBeGreaterThan(0);
|
||||
// Same page, same id — otherwise a burst still reads as several sources.
|
||||
expect(tabIds[1]).toBe(tabIds[0]);
|
||||
});
|
||||
|
||||
it("lets an explicit property win over the canary mixin", async () => {
|
||||
trackStudioEvent("thing_happened", { "$feature/canary-test-one": "false" });
|
||||
const events = await sentEvents();
|
||||
|
||||
@@ -2,6 +2,7 @@ import { resolveStudioDistinctId } from "../telemetry/distinctId";
|
||||
import { browserTelemetryAllowed } from "../telemetry/policy";
|
||||
import { canaryEventProperties } from "../telemetry/canary";
|
||||
import { agentRuntimeProperty } from "../telemetry/agentRuntime";
|
||||
import { tabIdProperty } from "../telemetry/tabId";
|
||||
|
||||
// PostHog public ingest key — write-only, safe to ship in the client bundle
|
||||
const POSTHOG_API_KEY = "phc_zjjbX0PnWxERXrMHhkEJWj9A9BhGVLRReICgsfTMmpx";
|
||||
@@ -48,6 +49,9 @@ function getSessionProperties(): EventProperties {
|
||||
// and a property that only some events carry cannot answer it — the
|
||||
// breakdown silently reads as though the agent never used the rest.
|
||||
agent_runtime: agentRuntimeProperty(),
|
||||
// Which page load this came from — distinct_id identifies the browser, not
|
||||
// the page, so two tabs are otherwise one indistinguishable stream.
|
||||
tab_id: tabIdProperty(),
|
||||
screen_width: window.screen?.width,
|
||||
screen_height: window.screen?.height,
|
||||
viewport_width: window.innerWidth,
|
||||
|
||||
Reference in New Issue
Block a user