Skip to content

Commit e80cec3

Browse files
Merge pull request #1787 from kirill-markin/claude/agent-followups-p20b
feat(analytics): attribute AI chat reviews to the device that started the run
2 parents cf4e9cb + 71de515 commit e80cec3

34 files changed

Lines changed: 147 additions & 54 deletions

apps/admin/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ The old dashboard showed nothing for them, but that was not its replica join: th
9292

9393
The email that the `%@example.com` exclusion needs is not in the events table, so it is joined from `actor_id` to `org.user_settings`. `actor_id` is a UUID and renders as canonical lowercase hex, while `org.user_settings.user_id` is an unconstrained `TEXT` primary key, so that join folds the stored side with `pg_catalog.lower`. Comparing as stored would silently miss an uppercase-hex row, and a test account with no matched email is counted rather than excluded.
9494

95-
Platform is read off the event row and never derived. The buckets are `web`, `android`, `ios`, `agent`, and `unattributed`, and they are always split, never summed: an agent-API client merged into `web` would read as a person using the site. `agent` is an upper bound on human agent use rather than a count of people, because a scheduled or polling machine client files activity on a timer, and `db/migrations/0121_backfill_synthetic_app_opened_days.sql` states this in full. A `review_answered` row carries the platform the backend resolved from the replica that recorded the review, and migration `0122` filled the same value on the reconstructed history, `0123` on the live rows the producer wrote before it could resolve one, so both platform charts colour real device activity. A device value appears only for a `client_installation` replica on `ios`, `android`, or `web`, and a machine-API replica resolves to `agent`; an AI-chat and a seed/reset replica both leave the column NULL, as does a review whose replica row is gone. `unattributed` therefore means the row carries no resolved device fact - either the actor behind it is not a device, or no device could be resolved for it - and it stays its own bucket rather than being guessed at or summed into a device.
95+
Platform is read off the event row and never derived. The buckets are `web`, `android`, `ios`, `agent`, and `unattributed`, and they are always split, never summed: an agent-API client merged into `web` would read as a person using the site. `agent` is an upper bound on human agent use rather than a count of people, because a scheduled or polling machine client files activity on a timer, and `db/migrations/0121_backfill_synthetic_app_opened_days.sql` states this in full. A `review_answered` row carries the platform the backend resolved from the replica that recorded the review, and migration `0122` filled the same value on the reconstructed history, `0123` on the live rows the producer wrote before it could resolve one, so both platform charts colour real device activity. A device value appears for a `client_installation` replica on `ios`, `android`, or `web`, and for an AI-chat review whose chat run was started by a request that named its device; a machine-API replica resolves to `agent`, while any other AI-chat review and a seed/reset replica leave the column NULL, as does a review whose replica row is gone. `unattributed` therefore means the row carries no resolved device fact - either the actor behind it is not a device, or no device could be resolved for it - and it stays its own bucket rather than being guessed at or summed into a device.
9696

9797
One thing to read the two platform charts with: a bulk review-history import ensures a replica from the importing request, and every imported review event stores that replica, so one import files its whole batch under the device that performed the import rather than under the device that originally answered. Those rows already land on the import day, because `occurred_at` falls back to the server anchor outside the 30-day window. A large import therefore shows as a single-platform spike on a single day, and that is the import showing through rather than a defect.
9898

apps/admin/src/reports/reviewEventsByDate/query.ts

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -517,19 +517,22 @@ export function buildReviewEventsByDateAvailableRangeSql(): string {
517517
// Keeping the anonymized history is intended - the reviews really happened - and `identity_state` on
518518
// `analytics.product_events_resolved` is the handle if they ever need filtering out.
519519
//
520-
// PLATFORM IS READ OFF THE ROW AND NEVER DERIVED. The producer derives it once per drain from the
521-
// replica that recorded the review (`apps/backend/src/productAnalytics/serverFacts/reviewAnswers.ts`), and
522-
// migration `0122` filled the same value on the history `0120` reconstructed, `0123` on the live
523-
// rows the producer wrote before it could resolve one. That derivation reads
520+
// PLATFORM IS READ OFF THE ROW AND NEVER DERIVED. The producer derives it once per drain
521+
// (`apps/backend/src/productAnalytics/serverFacts/reviewAnswers.ts`), from the replica that recorded
522+
// the review for every review except an AI-chat one, and migrations `0122` and `0123` filled the
523+
// same replica-derived value on the history `0120` reconstructed and on the live rows the producer
524+
// wrote before it could resolve one. That derivation reads
524525
// `sync.workspace_replicas.platform` only together with `actor_kind` on the same row, so a value
525526
// read off that column appears only for a `client_installation` replica on 'ios', 'android' or
526527
// 'web': an `agent_connection` replica stores 'web' for the machine API, an `ai_chat` replica
527528
// stores a hard-coded 'web' that describes no device, and seed/reset replicas store 'system'. An
528529
// `agent_connection` replica resolves to `agent` from its actor kind instead of from that column.
529-
// An `ai_chat` or seed/reset replica, and a review whose replica row is gone or whose resolution
530-
// failed, stays NULL and lands in the `unattributed` bucket, which means no resolved device fact -
531-
// either the actor behind the row is not a device or no device could be resolved for it - rather
532-
// than either case alone.
530+
// An AI-chat review takes the device platform stored on its chat run instead, so a device value
531+
// also appears for an AI-chat review whose run was started by a request that named its device, and
532+
// any other AI-chat review stays NULL. A seed/reset replica, and a review whose replica row is gone
533+
// or whose resolution failed, stays NULL too. Every NULL lands in the `unattributed` bucket, which
534+
// means no resolved device fact - either the actor behind the row is not a device or no device
535+
// could be resolved for it - rather than either case alone.
533536
export function buildReviewEventsByDateSql(from: string, to: string): string {
534537
assertValidDateRange({ from, to }, "Review events report");
535538

apps/backend/scripts/postgresIntegrations/boundaries.mjs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,14 @@ export const createdRolesByMigration = new Map([
7070
["0044_reporting_readonly_role.sql", Object.freeze(["reporting_readonly"])],
7171
]);
7272
export const boundaryDefinitions = Object.freeze([
73+
Object.freeze({
74+
migrationFileName: "0136_ai_chat_run_client_platform.sql",
75+
expectedMigrationCount: 138,
76+
testFiles: Object.freeze([
77+
"src/chat/cardImages/operation.postgres.integration.ts",
78+
"src/chat/runs/generatedImageAttemptBudget.postgres.integration.ts",
79+
]),
80+
}),
7381
Object.freeze({
7482
migrationFileName: "0135_generated_media_promotion_job_created_at_select.sql",
7583
expectedMigrationCount: 137,
@@ -127,14 +135,6 @@ export const boundaryDefinitions = Object.freeze([
127135
"src/mediaAssets/blobLifecycle/cleanup/sharedProvenance.postgres.integration.ts",
128136
]),
129137
}),
130-
Object.freeze({
131-
migrationFileName: "0127_ai_chat_run_live_attach_ownership.sql",
132-
expectedMigrationCount: 129,
133-
testFiles: Object.freeze([
134-
"src/chat/cardImages/operation.postgres.integration.ts",
135-
"src/chat/runs/generatedImageAttemptBudget.postgres.integration.ts",
136-
]),
137-
}),
138138
Object.freeze({
139139
migrationFileName: "0123_backfill_live_review_answered_platform.sql",
140140
expectedMigrationCount: 125,

apps/backend/src/agent/reviews.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
type DatabaseExecutor,
99
} from "../database";
1010
import { getDeck } from "../decks";
11+
import type { ProductAnalyticsClientReportablePlatform } from "../productAnalytics/catalog";
1112
import { createPostCommitAnalyticsBudget } from "../productAnalytics/serverFacts/postCommitBudget";
1213
import { runTransactionReportingReviewAnswers } from "../productAnalytics/serverFacts/reviewAnswers";
1314
import type { FsrsCardState, ReviewRating } from "../scheduling";
@@ -357,11 +358,13 @@ export async function resolveAgentConnectionReviewReplica(
357358
* content.review_events: a repeated reviewId dedupes there and never advances the schedule, and
358359
* a reviewId the caller reused on another card is refused outright rather than read as that retry.
359360
* The replica half of that key comes from resolveReplicaId, so each surface dedupes within its own
360-
* sync actor. */
361+
* sync actor. aiChatClientPlatform is the device the in-app chat run submitting this review was
362+
* started from; the surfaces that write through an agent connection pass null. */
361363
export async function submitAgentReview(
362364
context: AgentReviewContext,
363365
request: AgentReviewInput,
364366
resolveReplicaId: AgentReviewReplicaResolver,
367+
aiChatClientPlatform: ProductAnalyticsClientReportablePlatform | null,
365368
): Promise<AgentReviewResult> {
366369
const input = parseReviewRequest(submitReviewSchema, request);
367370
if (
@@ -378,6 +381,7 @@ export async function submitAgentReview(
378381
const clientEventId = `agent-review:${input.reviewId}`;
379382
return runTransactionReportingReviewAnswers(
380383
createPostCommitAnalyticsBudget(),
384+
aiChatClientPlatform,
381385
(runInTransaction) =>
382386
transactionWithWorkspaceScope(context, runInTransaction),
383387
async (executor) => {

apps/backend/src/cards/review/reviews.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,7 @@ export async function submitReview(
272272
// and microseconds in practice.
273273
return runTransactionReportingReviewAnswers<ReviewResult>(
274274
createPostCommitAnalyticsBudget(),
275+
null,
275276
(runInTransaction) => transactionWithWorkspaceScope({ userId, workspaceId }, runInTransaction),
276277
async (executor) => {
277278
const hotChangeWriteLock = await lockWorkspaceSyncMetadataForHotChangesInExecutor(executor, workspaceId);

apps/backend/src/chat/cardImages/operation.postgres.integration.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ async function createClaimedImageRun(fixture: PostgresIntegrationFixture): Promi
103103
const prepared = await prepareChatRun(
104104
fixture.userId, fixture.workspaceId, undefined,
105105
[{ type: "text", text: "Generate an image for this card." }],
106-
randomUUID(), "Europe/Madrid", null, true,
106+
randomUUID(), "Europe/Madrid", null, true, null,
107107
);
108108
const claimed = await claimChatRun(fixture.userId, fixture.workspaceId, prepared.runId);
109109
if (claimed === null) {

apps/backend/src/chat/http/handlers.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
getBackendTraceCarrier,
1212
startBackendSpan,
1313
} from "../../observability/sentry";
14+
import { readProductAnalyticsClientPlatform } from "../../productAnalytics/catalog";
1415
import { parseOptionalWorkspaceIdParam } from "../../server/requestContext";
1516
import {
1617
parseJsonBody,
@@ -178,6 +179,7 @@ export function createPostChatHandler(dependencies: ChatRouteDependencies): Hand
178179
// version is greater than 1.5.0.
179180
body.uiLocale ?? null,
180181
requestContext.transport === "bearer" || requestContext.transport === "session",
182+
readProductAnalyticsClientPlatform(context.get("clientPlatform") ?? null),
181183
));
182184
} catch (error) {
183185
return mapStoreError(error);

apps/backend/src/chat/openai/loop/loop.testSupport.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ export function createParams(
3232
sessionId: "session-1",
3333
generatedImageEligible: false,
3434
generatedImageOperationDeadlineMs: Date.now() + 600_000,
35+
clientPlatform: null,
3536
modelId: "gpt-5.6-terra",
3637
reasoningEffort: "xhigh",
3738
timezone: "Europe/Madrid",

apps/backend/src/chat/openai/loop/loop.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
type ChatRuntimeReasoningEffort,
2727
} from "../../config";
2828
import type { ChatRunClaimToken } from "../../runs";
29+
import type { ProductAnalyticsClientReportablePlatform } from "../../../productAnalytics/catalog";
2930
import {
3031
buildOpenAIResponsesRequest,
3132
buildPromptCacheKey,
@@ -87,6 +88,7 @@ export type StartOpenAILoopParams = Readonly<{
8788
sessionId: string;
8889
generatedImageEligible: boolean;
8990
generatedImageOperationDeadlineMs: number;
91+
clientPlatform: ProductAnalyticsClientReportablePlatform | null;
9092
modelId: ChatRuntimeModelId;
9193
reasoningEffort: ChatRuntimeReasoningEffort;
9294
timezone: string;
@@ -111,6 +113,7 @@ async function runOneToolCall(
111113
workspaceId: string;
112114
signal: AbortSignal | null;
113115
generatedImageOperationDeadlineMs: number;
116+
clientPlatform: ProductAnalyticsClientReportablePlatform | null;
114117
rootObservation: LangfuseObservation | null;
115118
}>,
116119
): Promise<ExecutedChatToolCall> {
@@ -340,6 +343,7 @@ async function runLoopWithDeps(
340343
workspaceId: params.workspaceId,
341344
signal: params.signal,
342345
generatedImageOperationDeadlineMs: params.generatedImageOperationDeadlineMs,
346+
clientPlatform: params.clientPlatform,
343347
rootObservation: params.rootObservation,
344348
onExecutionPhaseChanged: params.onExecutionPhaseChanged,
345349
shouldStopBeforeNextStep: params.shouldStopBeforeNextStep,

apps/backend/src/chat/openai/loop/modelCall.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
type ChatRuntimeReasoningEffort,
1717
} from "../../config";
1818
import type { ChatRunClaimToken } from "../../runs";
19+
import type { ProductAnalyticsClientReportablePlatform } from "../../../productAnalytics/catalog";
1920
import { createGeneratedImageOperationKey } from "../../generatedImageOperationIdentity";
2021
import { GENERATED_IMAGE_TOOL_NAME } from "../tools/generatedImageToolContract";
2122
import {
@@ -77,6 +78,7 @@ export type RunOneToolCall = (params: Readonly<{
7778
workspaceId: string;
7879
signal: AbortSignal | null;
7980
generatedImageOperationDeadlineMs: number;
81+
clientPlatform: ProductAnalyticsClientReportablePlatform | null;
8082
rootObservation: LangfuseObservation | null;
8183
}>) => Promise<ExecutedChatToolCall>;
8284

@@ -93,6 +95,7 @@ type ExecuteToolCallsParams = Readonly<{
9395
workspaceId: string;
9496
signal: AbortSignal | undefined;
9597
generatedImageOperationDeadlineMs: number;
98+
clientPlatform: ProductAnalyticsClientReportablePlatform | null;
9699
rootObservation: LangfuseObservation | null;
97100
onExecutionPhaseChanged: ((phase: "idle" | "model" | "tool") => void) | undefined;
98101
shouldStopBeforeNextStep: (() => boolean) | undefined;
@@ -282,6 +285,7 @@ export async function executeToolCalls(
282285
workspaceId: params.workspaceId,
283286
signal: params.signal ?? null,
284287
generatedImageOperationDeadlineMs: params.generatedImageOperationDeadlineMs,
288+
clientPlatform: params.clientPlatform,
285289
rootObservation: params.rootObservation,
286290
});
287291
if (output.stopReason !== null) {

0 commit comments

Comments
 (0)