mirror of
https://github.com/nexu-io/open-design.git
synced 2026-10-27 09:51:12 +01:00
fix(analytics): generation_progress page_view + artifact_count
Two P0 data bugs spot-checked on PostHog after PR #2390 landed: 1. `page_view page_name=onboarding area=generation_progress` was wired but never actually fired in production (PostHog shows 0 events despite hundreds of `area=runtime` / `about_you` / `design_system` peers). Cause: `OnboardingView` cleared `onboardingSessionId` on unmount. The Generate path unmounts OnboardingView *before* DesignSystemDetailView mounts, so by the time the generation effect calls `peekOnboardingSessionId()`, sessionStorage is empty and the 4th-step emission is skipped. Fix: drop the unmount-clear. Add `onFinishWithCleanup` and route the Skip / Back-from-step-0 / last-step-Continue buttons through it so those paths still clear. The Generate path now relies on `DesignSystemDetailView` itself clearing after the `generation_progress` event lands (already there in `DesignSystemFlow.tsx`). Abandoned sessions clear on tab close. 2. `run_finished.artifact_count` was hard-coded `0` at `server.ts:11061`. Every run on PostHog reported zero artifacts, making the "generation success → artifact produced" funnel useless. Fix: count incremental `.html` paths the run wrote or edited, with path-level dedup so a Write-then-Edit cycle on the same file counts as one artifact. Implemented as a pure helper in `apps/daemon/src/run-artifacts.ts` with 10 unit tests covering: - empty / no-html runs (returns 0) - single Write on a .html path - dedup across Write+Edit+MultiEdit on same path - distinct .html paths each count - Codex aliases (create_file, str_replace_edit) - both `file_path` and `path` input shapes - case-insensitive extension (.HTML / .Html) - non-agent events / malformed payloads ignored - Read/Grep/Bash never count Read-only ops never count; Write/Edit/MultiEdit + their Codex aliases (create_file, str_replace_edit, multi_edit) all count.
This commit is contained in:
@@ -0,0 +1,66 @@
|
||||
// Daemon-side helper that counts how many distinct `.html` files this
|
||||
// run produced or modified. Fed into v2 `run_finished.artifact_count`.
|
||||
//
|
||||
// Semantics (per product spec, 2026-05-21):
|
||||
// - Count is incremental for THIS run only, not cumulative across the
|
||||
// project. If the run touched no `.html` files, the count is 0.
|
||||
// - A file written multiple times within the same run counts once
|
||||
// (dedup by path) so a Write-then-Edit cycle on the same file
|
||||
// reports one artifact, not two.
|
||||
// - Both Write (create_file) and Edit / MultiEdit count, because the
|
||||
// agent often writes a skeleton then edits to fill it in; both end
|
||||
// in a new file state at run end.
|
||||
// - Read-only ops never count.
|
||||
//
|
||||
// Earlier `server.ts:11061` hard-coded `artifact_count: 0`, which
|
||||
// produced uniform zero on PostHog and made the v2 "generation
|
||||
// success → produced artifact" funnel useless.
|
||||
|
||||
// Tool names cover Claude-style, Codex-style, and the ACP/MCP shapes
|
||||
// the daemon proxies. Keep aligned with the web-side `WRITE_NAMES` /
|
||||
// `EDIT_NAMES` sets in `apps/web/src/runtime/file-ops.ts`.
|
||||
const WRITE_OR_EDIT_TOOL_NAMES: ReadonlySet<string> = new Set([
|
||||
'Write',
|
||||
'create_file',
|
||||
'Edit',
|
||||
'str_replace_edit',
|
||||
'MultiEdit',
|
||||
'multi_edit',
|
||||
]);
|
||||
|
||||
function extractToolFilePath(input: unknown): string | null {
|
||||
if (!input || typeof input !== 'object') return null;
|
||||
const obj = input as { file_path?: unknown; path?: unknown };
|
||||
if (typeof obj.file_path === 'string' && obj.file_path) return obj.file_path;
|
||||
if (typeof obj.path === 'string' && obj.path) return obj.path;
|
||||
return null;
|
||||
}
|
||||
|
||||
function isHtmlPath(path: string): boolean {
|
||||
return path.toLowerCase().endsWith('.html');
|
||||
}
|
||||
|
||||
export interface RunEventLike {
|
||||
event?: string;
|
||||
data?: unknown;
|
||||
}
|
||||
|
||||
export function countNewHtmlArtifacts(events: readonly RunEventLike[]): number {
|
||||
if (!events || events.length === 0) return 0;
|
||||
const writtenPaths = new Set<string>();
|
||||
for (const rec of events) {
|
||||
if (rec?.event !== 'agent') continue;
|
||||
const data = rec.data as
|
||||
| { type?: string; name?: unknown; input?: unknown }
|
||||
| null
|
||||
| undefined;
|
||||
if (data?.type !== 'tool_use') continue;
|
||||
if (typeof data.name !== 'string') continue;
|
||||
if (!WRITE_OR_EDIT_TOOL_NAMES.has(data.name)) continue;
|
||||
const path = extractToolFilePath(data.input);
|
||||
if (!path) continue;
|
||||
if (!isHtmlPath(path)) continue;
|
||||
writtenPaths.add(path);
|
||||
}
|
||||
return writtenPaths.size;
|
||||
}
|
||||
@@ -184,6 +184,7 @@ import { renderDesignSystemPreview } from './design-system-preview.js';
|
||||
import { renderDesignSystemShowcase } from './design-system-showcase.js';
|
||||
import { createChatRunService } from './runs.js';
|
||||
import { deriveRunErrorCode, runResultFromStatus } from './run-result.js';
|
||||
import { countNewHtmlArtifacts } from './run-artifacts.js';
|
||||
import { reportRunCompletedFromDaemon } from './langfuse-bridge.js';
|
||||
import {
|
||||
createAnalyticsService,
|
||||
@@ -10835,7 +10836,13 @@ export async function startServer({
|
||||
...baseProps,
|
||||
area: 'chat_panel',
|
||||
result,
|
||||
artifact_count: 0,
|
||||
// Incremental count of `.html` paths the run produced or
|
||||
// modified, deduped per file. Replaces the hard-coded `0`
|
||||
// that masked the "did this run actually generate an
|
||||
// artifact?" funnel on PostHog. See `run-artifacts.ts`
|
||||
// for the dedup semantics; tested in
|
||||
// `tests/run-artifacts.test.ts`.
|
||||
artifact_count: countNewHtmlArtifacts(run.events),
|
||||
total_duration_ms: Date.now() - runStartedAt,
|
||||
...(errorCode ? { error_code: errorCode } : {}),
|
||||
...(inputTokens !== undefined ? { input_tokens: inputTokens } : {}),
|
||||
|
||||
@@ -0,0 +1,124 @@
|
||||
// Unit coverage for `countNewHtmlArtifacts`. Pins the v2
|
||||
// `run_finished.artifact_count` invariant: incremental count of
|
||||
// distinct `.html` paths the run produced or modified, deduped by
|
||||
// path, with Read ops never counted.
|
||||
//
|
||||
// `server.ts` previously emitted `artifact_count: 0` literally, which
|
||||
// suppressed every dashboard tile that breaks "generation success" by
|
||||
// whether an artifact landed. These tests keep the new helper honest
|
||||
// for the shapes the daemon actually sees on the wire (claude-stream,
|
||||
// codex, ACP/MCP proxies).
|
||||
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import { countNewHtmlArtifacts } from '../src/run-artifacts.js';
|
||||
|
||||
function toolUse(name: string, filePath: string, id = 'tool-1') {
|
||||
return {
|
||||
event: 'agent',
|
||||
data: {
|
||||
type: 'tool_use',
|
||||
id,
|
||||
name,
|
||||
input: { file_path: filePath },
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe('countNewHtmlArtifacts', () => {
|
||||
it('returns 0 when the run produced no events', () => {
|
||||
expect(countNewHtmlArtifacts([])).toBe(0);
|
||||
});
|
||||
|
||||
it('returns 0 when no tool_use targets a .html file', () => {
|
||||
expect(
|
||||
countNewHtmlArtifacts([
|
||||
toolUse('Write', '/proj/notes.md'),
|
||||
toolUse('Edit', '/proj/styles.css'),
|
||||
toolUse('Read', '/proj/index.html'), // Read doesn't count
|
||||
]),
|
||||
).toBe(0);
|
||||
});
|
||||
|
||||
it('counts a single Write on a .html path', () => {
|
||||
expect(
|
||||
countNewHtmlArtifacts([toolUse('Write', '/proj/index.html')]),
|
||||
).toBe(1);
|
||||
});
|
||||
|
||||
it('dedupes multiple Write/Edit ops on the same path (one artifact per file)', () => {
|
||||
expect(
|
||||
countNewHtmlArtifacts([
|
||||
toolUse('Write', '/proj/index.html', 't1'),
|
||||
toolUse('Edit', '/proj/index.html', 't2'),
|
||||
toolUse('MultiEdit', '/proj/index.html', 't3'),
|
||||
]),
|
||||
).toBe(1);
|
||||
});
|
||||
|
||||
it('counts distinct .html paths separately', () => {
|
||||
expect(
|
||||
countNewHtmlArtifacts([
|
||||
toolUse('Write', '/proj/index.html'),
|
||||
toolUse('Write', '/proj/about.html'),
|
||||
toolUse('Write', '/proj/contact.html'),
|
||||
]),
|
||||
).toBe(3);
|
||||
});
|
||||
|
||||
it('handles the Codex `create_file` / `str_replace_edit` aliases', () => {
|
||||
expect(
|
||||
countNewHtmlArtifacts([
|
||||
toolUse('create_file', '/proj/a.html'),
|
||||
toolUse('str_replace_edit', '/proj/b.html'),
|
||||
]),
|
||||
).toBe(2);
|
||||
});
|
||||
|
||||
it('accepts both `file_path` and `path` input shapes', () => {
|
||||
expect(
|
||||
countNewHtmlArtifacts([
|
||||
{
|
||||
event: 'agent',
|
||||
data: {
|
||||
type: 'tool_use',
|
||||
id: 't1',
|
||||
name: 'Write',
|
||||
input: { path: '/proj/page.html' },
|
||||
},
|
||||
},
|
||||
]),
|
||||
).toBe(1);
|
||||
});
|
||||
|
||||
it('treats .HTML / .Html case-insensitively', () => {
|
||||
expect(
|
||||
countNewHtmlArtifacts([
|
||||
toolUse('Write', '/proj/Page.HTML'),
|
||||
toolUse('Write', '/proj/Other.Html'),
|
||||
]),
|
||||
).toBe(2);
|
||||
});
|
||||
|
||||
it('ignores non-agent events and malformed payloads', () => {
|
||||
expect(
|
||||
countNewHtmlArtifacts([
|
||||
{ event: 'start', data: { runId: 'r1' } },
|
||||
{ event: 'stderr', data: { chunk: 'log' } },
|
||||
{ event: 'agent', data: null },
|
||||
{ event: 'agent', data: { type: 'text_delta', text: 'hi' } },
|
||||
toolUse('Write', '/proj/index.html'),
|
||||
]),
|
||||
).toBe(1);
|
||||
});
|
||||
|
||||
it('ignores Read / Grep / Bash even when their input names a .html file', () => {
|
||||
expect(
|
||||
countNewHtmlArtifacts([
|
||||
toolUse('Read', '/proj/index.html'),
|
||||
toolUse('Grep', '/proj/index.html'),
|
||||
toolUse('Bash', '/proj/index.html'),
|
||||
]),
|
||||
).toBe(0);
|
||||
});
|
||||
});
|
||||
@@ -8,7 +8,7 @@
|
||||
// can be rebased without touching this file. `EntryView` becomes a
|
||||
// thin wrapper that passes data and callbacks through to this shell.
|
||||
|
||||
import { useEffect, useMemo, useRef, useState, type ReactNode } from 'react';
|
||||
import { useCallback, useEffect, useMemo, useRef, useState, type ReactNode } from 'react';
|
||||
import {
|
||||
defaultScenarioPluginIdForKind,
|
||||
type ConnectorDetail,
|
||||
@@ -862,18 +862,26 @@ function OnboardingView({
|
||||
// exposure. The fourth step (`generation`) lives in
|
||||
// `DesignSystemDetailView` because the user navigates out of this
|
||||
// component once the design system project opens; that emission
|
||||
// reads the same `onboarding_session_id` from sessionStorage.
|
||||
// `clearOnboardingSessionId` runs on `onFinish` / unmount so a
|
||||
// later DS visit unrelated to onboarding doesn't inherit the id.
|
||||
// reads the same `onboarding_session_id` from sessionStorage and
|
||||
// clears it once it fires.
|
||||
//
|
||||
// We do NOT clear on unmount: the Generate path unmounts
|
||||
// OnboardingView *before* DesignSystemDetailView mounts, so an
|
||||
// unmount-clear would race the 4th-step emission and consistently
|
||||
// wipe the id before it could be read (observed on PostHog —
|
||||
// `area=generation_progress` had zero events after PR #2390).
|
||||
// The Skip path clears via `onFinishWithCleanup` below; the
|
||||
// Generate path clears from `DesignSystemDetailView` after the
|
||||
// generation_progress event lands; abandoned sessions clear on
|
||||
// sessionStorage tab close.
|
||||
const onboardingSessionIdRef = useRef<string>('');
|
||||
if (!onboardingSessionIdRef.current) {
|
||||
onboardingSessionIdRef.current = getOrCreateOnboardingSessionId();
|
||||
}
|
||||
useEffect(() => {
|
||||
return () => {
|
||||
clearOnboardingSessionId();
|
||||
};
|
||||
}, []);
|
||||
const onFinishWithCleanup = useCallback(() => {
|
||||
clearOnboardingSessionId();
|
||||
onFinish();
|
||||
}, [onFinish]);
|
||||
useEffect(() => {
|
||||
const onboardingSessionId = onboardingSessionIdRef.current;
|
||||
if (!onboardingSessionId) return;
|
||||
@@ -1068,7 +1076,10 @@ function OnboardingView({
|
||||
|
||||
function handlePrimaryAction() {
|
||||
if (isLastStep) {
|
||||
onFinish();
|
||||
// Last-step "Continue" without generating a design system: same
|
||||
// shape as Skip — clear the session id so a later unrelated DS
|
||||
// visit doesn't claim onboarding attribution.
|
||||
onFinishWithCleanup();
|
||||
return;
|
||||
}
|
||||
setStep((current) => current + 1);
|
||||
@@ -1353,7 +1364,7 @@ function OnboardingView({
|
||||
<span>{t('settings.onboardingDesignIntroReuseBody')}</span>
|
||||
</div>
|
||||
</div>
|
||||
<button type="button" className="onboarding-view__ds-skip" onClick={onFinish}>
|
||||
<button type="button" className="onboarding-view__ds-skip" onClick={onFinishWithCleanup}>
|
||||
{t('settings.onboardingSkip')}
|
||||
</button>
|
||||
</div>
|
||||
@@ -1387,7 +1398,7 @@ function OnboardingView({
|
||||
<button
|
||||
type="button"
|
||||
className="onboarding-view__secondary"
|
||||
onClick={() => (step === 0 ? onFinish() : setStep((current) => current - 1))}
|
||||
onClick={() => (step === 0 ? onFinishWithCleanup() : setStep((current) => current - 1))}
|
||||
>
|
||||
{step === 0 ? t('settings.onboardingSkip') : t('settings.onboardingBack')}
|
||||
</button>
|
||||
|
||||
Reference in New Issue
Block a user