From 3d6f9627ecf93a30acbdcdda9e6b52a764b6017a Mon Sep 17 00:00:00 2001 From: Cuong Date: Wed, 27 May 2026 15:26:49 +0700 Subject: [PATCH 1/4] feat: [ENG-2969] M14.1 extend TASK_TYPE_VALUES with v4.0 tool-mode task types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds curate-tool-mode, query-tool-mode, dream-scan, dream-finalize to TaskTypes + TASK_TYPE_VALUES. After ENG-2925's rename the daemon dispatches these four types but the analytics enum still predates v4.0 — task_created / task_completed / task_failed silently rejected every tool-mode emit at the Zod boundary. - TaskTypes keeps the legacy 'curate' / 'query' / 'dream' values for back-compat with any constructor still building those payloads - per-event schemas (task_created / task_completed / task_failed) pick up the new types automatically via z.enum(TASK_TYPE_VALUES) - M12 per-flavor schemas (curate_run_completed / query_completed) still hardcode their own literals and continue to reject tool-mode types here — M14.2 migrates them to the canonical enum as a follow-up TDD: - new task-types tests assert TaskCreated/Completed/Failed accept all four new types - regression tests pin the M12 schemas' continued rejection so M14.2 has a clear flip point --- src/shared/analytics/task-types.ts | 8 ++ test/unit/shared/analytics/task-types.test.ts | 112 +++++++++++++++++- 2 files changed, 117 insertions(+), 3 deletions(-) diff --git a/src/shared/analytics/task-types.ts b/src/shared/analytics/task-types.ts index d14e60186..f8976f5ac 100644 --- a/src/shared/analytics/task-types.ts +++ b/src/shared/analytics/task-types.ts @@ -12,8 +12,12 @@ export const TaskTypes = { CURATE: 'curate', CURATE_FOLDER: 'curate-folder', + CURATE_TOOL_MODE: 'curate-tool-mode', DREAM: 'dream', + DREAM_FINALIZE: 'dream-finalize', + DREAM_SCAN: 'dream-scan', QUERY: 'query', + QUERY_TOOL_MODE: 'query-tool-mode', SEARCH: 'search', } as const @@ -27,7 +31,11 @@ export type TaskType = (typeof TaskTypes)[keyof typeof TaskTypes] export const TASK_TYPE_VALUES = [ TaskTypes.CURATE, TaskTypes.CURATE_FOLDER, + TaskTypes.CURATE_TOOL_MODE, TaskTypes.DREAM, + TaskTypes.DREAM_FINALIZE, + TaskTypes.DREAM_SCAN, TaskTypes.QUERY, + TaskTypes.QUERY_TOOL_MODE, TaskTypes.SEARCH, ] as const diff --git a/test/unit/shared/analytics/task-types.test.ts b/test/unit/shared/analytics/task-types.test.ts index 4126f1329..104a355cf 100644 --- a/test/unit/shared/analytics/task-types.test.ts +++ b/test/unit/shared/analytics/task-types.test.ts @@ -1,15 +1,25 @@ - + +/* eslint-disable camelcase */ import {expect} from 'chai' +import {CurateRunCompletedSchema} from '../../../../src/shared/analytics/events/curate-run-completed.js' +import {QueryCompletedSchema} from '../../../../src/shared/analytics/events/query-completed.js' +import {TaskCompletedSchema} from '../../../../src/shared/analytics/events/task-completed.js' +import {TaskCreatedSchema} from '../../../../src/shared/analytics/events/task-created.js' +import {TaskFailedSchema} from '../../../../src/shared/analytics/events/task-failed.js' import {TASK_TYPE_VALUES, type TaskType, TaskTypes} from '../../../../src/shared/analytics/task-types.js' describe('TaskTypes', () => { - it('should expose exactly the five daemon task types', () => { + it('should expose every v4.0 daemon task type', () => { expect(Object.keys(TaskTypes).sort()).to.deep.equal([ 'CURATE', 'CURATE_FOLDER', + 'CURATE_TOOL_MODE', 'DREAM', + 'DREAM_FINALIZE', + 'DREAM_SCAN', 'QUERY', + 'QUERY_TOOL_MODE', 'SEARCH', ]) }) @@ -17,9 +27,13 @@ describe('TaskTypes', () => { it('should map each key to the wire string used by the daemon TaskInfo.type', () => { expect(TaskTypes.CURATE).to.equal('curate') expect(TaskTypes.CURATE_FOLDER).to.equal('curate-folder') + expect(TaskTypes.CURATE_TOOL_MODE).to.equal('curate-tool-mode') + expect(TaskTypes.DREAM).to.equal('dream') + expect(TaskTypes.DREAM_FINALIZE).to.equal('dream-finalize') + expect(TaskTypes.DREAM_SCAN).to.equal('dream-scan') expect(TaskTypes.QUERY).to.equal('query') + expect(TaskTypes.QUERY_TOOL_MODE).to.equal('query-tool-mode') expect(TaskTypes.SEARCH).to.equal('search') - expect(TaskTypes.DREAM).to.equal('dream') }) it('should expose TaskType as the union of values', () => { @@ -39,4 +53,96 @@ describe('TaskTypes', () => { expect(TASK_TYPE_VALUES.length).to.be.greaterThan(0) }) }) + + describe('v4.0 tool-mode types validate through task_* schemas', () => { + const newTypes = [ + TaskTypes.CURATE_TOOL_MODE, + TaskTypes.QUERY_TOOL_MODE, + TaskTypes.DREAM_SCAN, + TaskTypes.DREAM_FINALIZE, + ] as const + + for (const taskType of newTypes) { + it(`TaskCreatedSchema accepts task_type='${taskType}'`, () => { + const parsed = TaskCreatedSchema.parse({ + has_files: false, + has_folder: false, + task_id: 't-1', + task_type: taskType, + }) + expect(parsed.task_type).to.equal(taskType) + }) + + it(`TaskCompletedSchema accepts task_type='${taskType}'`, () => { + const parsed = TaskCompletedSchema.parse({ + duration_ms: 100, + task_id: 't-1', + task_type: taskType, + }) + expect(parsed.task_type).to.equal(taskType) + }) + + it(`TaskFailedSchema accepts task_type='${taskType}'`, () => { + const parsed = TaskFailedSchema.parse({ + duration_ms: 100, + task_id: 't-1', + task_type: taskType, + }) + expect(parsed.task_type).to.equal(taskType) + }) + } + + it('rejects an unknown task_type on all three task_* schemas', () => { + const bad = { + duration_ms: 0, + has_files: false, + has_folder: false, + task_id: 't-1', + task_type: 'not-a-real-type' as unknown as TaskType, + } + expect(() => TaskCreatedSchema.parse(bad)).to.throw() + expect(() => TaskCompletedSchema.parse(bad)).to.throw() + expect(() => TaskFailedSchema.parse(bad)).to.throw() + }) + }) + + // The M12 schemas hardcode literal task_type values and still REJECT + // the new tool-mode types. M14.2 is the follow-up that migrates them + // to z.enum(TASK_TYPE_VALUES); until then these regressions are + // EXPECTED and documented as such in M14.1's TDD. + describe('M12 schemas still reject tool-mode types (M14.2 follow-up)', () => { + const curatePayload = { + duration_ms: 100, + operations_added: 0, + operations_deleted: 0, + operations_failed: 0, + operations_merged: 0, + operations_updated: 0, + outcome: 'completed' as const, + pending_review_count: 0, + task_id: 't-1', + } + + it('CurateRunCompletedSchema still rejects curate-tool-mode', () => { + expect(() => + CurateRunCompletedSchema.parse({...curatePayload, task_type: TaskTypes.CURATE_TOOL_MODE}), + ).to.throw() + }) + + it('QueryCompletedSchema still rejects query-tool-mode', () => { + const queryPayload = { + cache_hit: false, + duration_ms: 100, + matched_doc_count: 0, + outcome: 'completed' as const, + read_doc_count: 0, + read_tool_call_count: 0, + search_call_count: 0, + task_id: 't-1', + } + expect(() => + QueryCompletedSchema.parse({...queryPayload, task_type: TaskTypes.QUERY_TOOL_MODE}), + ).to.throw() + }) + }) }) From 6d7459659912145efd97b186ea91a8df659afec0 Mon Sep 17 00:00:00 2001 From: Cuong Date: Wed, 27 May 2026 15:30:04 +0700 Subject: [PATCH 2/4] fix: [ENG-2970] M14.2 relax curate_run_completed + query_completed task_type to TASK_TYPE_VALUES MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migrates the two M12 per-flavor schemas from hardcoded literal task_type values to the canonical TASK_TYPE_VALUES enum so v4.0 tool-mode tasks round-trip the wire boundary instead of being silently Zod-rejected inside AnalyticsHook. - curate_run_completed: z.enum(['curate', 'curate-folder']) → z.enum(TASK_TYPE_VALUES). curate-tool-mode payloads now validate. - query_completed: z.literal('query') → z.enum(TASK_TYPE_VALUES). query-tool-mode payloads now validate. The schemas no longer structurally constrain task_type to the curate or query family; the hook is trusted to only emit each event for the right flavor. Docblocks call out the widening for the next maintainer. TDD: - curate-run-completed.test asserts curate-tool-mode + legacy values both succeed; an unknown task_type still rejects - query-completed.test mirrors the same coverage for query-tool-mode - task-types.test M14.1 regression assertions flipped from rejection to acceptance for the M12 schemas; M14.1 docblock comments updated Without this, M14.3's hook code would land but the M12 emits that fire alongside the new generic task_* emits would silently disappear on every tool-mode task — that's the bug operators noticed in Mixpanel. --- .../analytics/events/curate-run-completed.ts | 10 +++- .../analytics/events/query-completed.ts | 10 +++- .../events/curate-run-completed.test.ts | 9 ++- .../analytics/events/query-completed.test.ts | 9 ++- test/unit/shared/analytics/task-types.test.ts | 56 +++++++++++-------- 5 files changed, 65 insertions(+), 29 deletions(-) diff --git a/src/shared/analytics/events/curate-run-completed.ts b/src/shared/analytics/events/curate-run-completed.ts index 8e1702f6c..57f223eb9 100644 --- a/src/shared/analytics/events/curate-run-completed.ts +++ b/src/shared/analytics/events/curate-run-completed.ts @@ -1,12 +1,20 @@ /* eslint-disable camelcase */ import {z} from 'zod' +import {TASK_TYPE_VALUES} from '../task-types.js' + /** * Per-event schema for `curate_run_completed`. * * Emitted by the daemon's `AnalyticsHook` (M12.2) at curate task terminal * states (completed / partial / cancelled / error). Carries per-task * operation counters so PMs can aggregate curate volume + outcome over time. + * + * M14.2 migrated `task_type` from a literal ['curate', 'curate-folder'] + * enum to the canonical `TASK_TYPE_VALUES` tuple so v4.0 tool-mode types + * (curate-tool-mode) round-trip the wire boundary. The hook is expected + * to only emit this event for curate flavors; the schema no longer + * structurally enforces that and trusts the caller. */ export const CurateRunCompletedSchema = z .object({ @@ -19,7 +27,7 @@ export const CurateRunCompletedSchema = z outcome: z.enum(['completed', 'partial', 'cancelled', 'error']), pending_review_count: z.number().int().nonnegative(), task_id: z.string().min(1), - task_type: z.enum(['curate', 'curate-folder']), + task_type: z.enum(TASK_TYPE_VALUES), }) .strict() diff --git a/src/shared/analytics/events/query-completed.ts b/src/shared/analytics/events/query-completed.ts index b861608f4..f6058897a 100644 --- a/src/shared/analytics/events/query-completed.ts +++ b/src/shared/analytics/events/query-completed.ts @@ -1,6 +1,8 @@ /* eslint-disable camelcase */ import {z} from 'zod' +import {TASK_TYPE_VALUES} from '../task-types.js' + /** * Per-file structure inside `query_completed.read_paths_with_metadata`. * Frontmatter arrays are optional and absent when the daemon cannot read @@ -23,6 +25,12 @@ const ReadPathWithMetadataSchema = z * states (completed / cancelled / error). Carries duration, retrieval * tier hit, doc counts, and (M12.3) the per-file structure for the top-N * (max 10) files the agent read during the query. + * + * M14.2 migrated `task_type` from `z.literal('query')` to the canonical + * `TASK_TYPE_VALUES` tuple so v4.0 tool-mode types (query-tool-mode) + * round-trip the wire boundary. The hook is expected to only emit this + * event for query flavors; the schema no longer structurally enforces + * that and trusts the caller. */ export const QueryCompletedSchema = z .object({ @@ -35,7 +43,7 @@ export const QueryCompletedSchema = z read_tool_call_count: z.number().int().nonnegative(), search_call_count: z.number().int().nonnegative(), task_id: z.string().min(1), - task_type: z.literal('query'), + task_type: z.enum(TASK_TYPE_VALUES), tier: z.union([z.literal(0), z.literal(1), z.literal(2), z.literal(3), z.literal(4)]).optional(), }) .strict() diff --git a/test/unit/shared/analytics/events/curate-run-completed.test.ts b/test/unit/shared/analytics/events/curate-run-completed.test.ts index d9e9e6728..5ec7cec2d 100644 --- a/test/unit/shared/analytics/events/curate-run-completed.test.ts +++ b/test/unit/shared/analytics/events/curate-run-completed.test.ts @@ -63,8 +63,13 @@ describe('CurateRunCompletedSchema', () => { expect(CurateRunCompletedSchema.safeParse({...baseValid, outcome: 'mystery'}).success).to.equal(false) }) - it('rejects out-of-enum task_type', () => { - expect(CurateRunCompletedSchema.safeParse({...baseValid, task_type: 'query'}).success).to.equal(false) + it('rejects an unknown task_type but accepts every canonical TASK_TYPE_VALUES entry', () => { + // M14.2 widened task_type from ['curate', 'curate-folder'] to the + // canonical TASK_TYPE_VALUES tuple so curate-tool-mode round-trips + // the wire boundary. Genuinely unknown values still reject. + expect(CurateRunCompletedSchema.safeParse({...baseValid, task_type: 'not-a-real-type'}).success).to.equal(false) + expect(CurateRunCompletedSchema.safeParse({...baseValid, task_type: 'curate-tool-mode'}).success).to.equal(true) + expect(CurateRunCompletedSchema.safeParse({...baseValid, task_type: 'query'}).success).to.equal(true) }) it('rejects negative counts and duration_ms', () => { diff --git a/test/unit/shared/analytics/events/query-completed.test.ts b/test/unit/shared/analytics/events/query-completed.test.ts index 69072357a..e80d8039f 100644 --- a/test/unit/shared/analytics/events/query-completed.test.ts +++ b/test/unit/shared/analytics/events/query-completed.test.ts @@ -90,8 +90,13 @@ describe('QueryCompletedSchema', () => { expect(QueryCompletedSchema.safeParse({...baseValid, tier: -1}).success).to.equal(false) }) - it('rejects task_type other than literal "query"', () => { - expect(QueryCompletedSchema.safeParse({...baseValid, task_type: 'curate'}).success).to.equal(false) + it('rejects an unknown task_type but accepts every canonical TASK_TYPE_VALUES entry', () => { + // M14.2 widened task_type from z.literal('query') to the canonical + // TASK_TYPE_VALUES tuple so query-tool-mode round-trips the wire + // boundary. Genuinely unknown values still reject. + expect(QueryCompletedSchema.safeParse({...baseValid, task_type: 'not-a-real-type'}).success).to.equal(false) + expect(QueryCompletedSchema.safeParse({...baseValid, task_type: 'query-tool-mode'}).success).to.equal(true) + expect(QueryCompletedSchema.safeParse({...baseValid, task_type: 'curate'}).success).to.equal(true) }) it('rejects negative or non-integer counts', () => { diff --git a/test/unit/shared/analytics/task-types.test.ts b/test/unit/shared/analytics/task-types.test.ts index 104a355cf..e0d5999ef 100644 --- a/test/unit/shared/analytics/task-types.test.ts +++ b/test/unit/shared/analytics/task-types.test.ts @@ -106,11 +106,7 @@ describe('TaskTypes', () => { }) }) - // The M12 schemas hardcode literal task_type values and still REJECT - // the new tool-mode types. M14.2 is the follow-up that migrates them - // to z.enum(TASK_TYPE_VALUES); until then these regressions are - // EXPECTED and documented as such in M14.1's TDD. - describe('M12 schemas still reject tool-mode types (M14.2 follow-up)', () => { + describe('M12 per-flavor schemas accept tool-mode types (M14.2)', () => { const curatePayload = { duration_ms: 100, operations_added: 0, @@ -123,26 +119,40 @@ describe('TaskTypes', () => { task_id: 't-1', } - it('CurateRunCompletedSchema still rejects curate-tool-mode', () => { - expect(() => - CurateRunCompletedSchema.parse({...curatePayload, task_type: TaskTypes.CURATE_TOOL_MODE}), - ).to.throw() + const queryPayload = { + cache_hit: false, + duration_ms: 100, + matched_doc_count: 0, + outcome: 'completed' as const, + read_doc_count: 0, + read_tool_call_count: 0, + search_call_count: 0, + task_id: 't-1', + } + + it('CurateRunCompletedSchema accepts curate-tool-mode', () => { + const parsed = CurateRunCompletedSchema.parse({ + ...curatePayload, + task_type: TaskTypes.CURATE_TOOL_MODE, + }) + expect(parsed.task_type).to.equal('curate-tool-mode') }) - it('QueryCompletedSchema still rejects query-tool-mode', () => { - const queryPayload = { - cache_hit: false, - duration_ms: 100, - matched_doc_count: 0, - outcome: 'completed' as const, - read_doc_count: 0, - read_tool_call_count: 0, - search_call_count: 0, - task_id: 't-1', - } - expect(() => - QueryCompletedSchema.parse({...queryPayload, task_type: TaskTypes.QUERY_TOOL_MODE}), - ).to.throw() + it('CurateRunCompletedSchema still accepts the legacy curate / curate-folder values', () => { + const curate = CurateRunCompletedSchema.parse({...curatePayload, task_type: TaskTypes.CURATE}) + expect(curate.task_type).to.equal('curate') + const folder = CurateRunCompletedSchema.parse({...curatePayload, task_type: TaskTypes.CURATE_FOLDER}) + expect(folder.task_type).to.equal('curate-folder') + }) + + it('QueryCompletedSchema accepts query-tool-mode', () => { + const parsed = QueryCompletedSchema.parse({...queryPayload, task_type: TaskTypes.QUERY_TOOL_MODE}) + expect(parsed.task_type).to.equal('query-tool-mode') + }) + + it('QueryCompletedSchema still accepts the legacy query value', () => { + const parsed = QueryCompletedSchema.parse({...queryPayload, task_type: TaskTypes.QUERY}) + expect(parsed.task_type).to.equal('query') }) }) }) From 1d4375ff281a70df9b98ea42fa0e87f5e654aee7 Mon Sep 17 00:00:00 2001 From: Cuong Date: Wed, 27 May 2026 15:56:30 +0700 Subject: [PATCH 3/4] feat: [ENG-2971] M14.3 wire task_created + task_completed + task_failed in AnalyticsHook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the three generic funnel-event emits described in M14's customer ask. Every daemon task (curate / curate-folder / curate-tool-mode (aliased from curate-html-direct) / query / query-tool-mode / dream-scan / dream-finalize / search) now produces, in order: task_created onTaskCreate (funnel entry — unconditional) ...optional per-op + M12 per-flavor terminal emit task_completed onTaskCompleted (terminal-event-last on success) task_failed onTaskError / onTaskCancelled (terminal-event-last) Three coupled changes shipped together so the wire stays consistent: 1. analytics-hook.ts grows three emits and a `toAnalyticsTaskType` alias-translator. The daemon still dispatches the pre-ENG-2925 name `curate-html-direct`; analytics canonicalises it to the post-rename `curate-tool-mode` so the wire enum matches TASK_TYPE_VALUES. Once ENG-2925 lands, the alias becomes a no-op identity. 2. CURATE_TASK_TYPES + QUERY_TASK_TYPES gain the tool-mode names so M12 per-flavor state init kicks in for tool-mode curates / queries. M12 counters stay all-zero on tool-mode today (no LLM tool calls fire) — that's a separate follow-up (FU-1 in plans/analytics-m14/follow-ups). 3. Both M12 payload builders route `task_type` through the same alias so curate-tool-mode tasks emit `task_type='curate-tool-mode'` on the curate_run_completed / query_completed events too. TDD: - new analytics-hook-m14.test.ts: 15 simulation tests covering every task type, the curate-html-direct → curate-tool-mode alias, has_files / has_folder semantics, both terminal paths, and the ordering invariant - existing analytics-hook.test.ts updated to filter out the new generic emits via `filterM12()` so M12-focused assertions keep their intent - integration stress tests updated for the new 13-event sequence (task_created → ops → curate_run_completed → task_completed) - repo full suite: 9121 passing, 0 failing --- src/server/infra/process/analytics-hook.ts | 87 +++++-- .../infra/process/curate-log-handler.ts | 7 +- src/server/infra/process/query-log-handler.ts | 5 +- .../analytics-hook-async-stress.test.ts | 24 +- .../infra/process/analytics-hook-m14.test.ts | 215 ++++++++++++++++++ .../infra/process/analytics-hook.test.ts | 119 ++++++---- 6 files changed, 380 insertions(+), 77 deletions(-) create mode 100644 test/unit/server/infra/process/analytics-hook-m14.test.ts diff --git a/src/server/infra/process/analytics-hook.ts b/src/server/infra/process/analytics-hook.ts index 6d886a3c3..307c7bba6 100644 --- a/src/server/infra/process/analytics-hook.ts +++ b/src/server/infra/process/analytics-hook.ts @@ -5,6 +5,7 @@ import type {AnalyticsEventName} from '../../../shared/analytics/event-names.js' import type {CurateRunCompletedProps} from '../../../shared/analytics/events/curate-run-completed.js' import type {PropsArg} from '../../../shared/analytics/events/index.js' import type {QueryCompletedProps} from '../../../shared/analytics/events/query-completed.js' +import type {TaskType} from '../../../shared/analytics/task-types.js' import type {LlmToolResultEvent} from '../../core/domain/transport/schemas.js' import type {TaskInfo} from '../../core/domain/transport/task-info.js' import type {IAnalyticsClient} from '../../core/interfaces/analytics/i-analytics-client.js' @@ -12,12 +13,25 @@ import type {ITaskLifecycleHook} from '../../core/interfaces/process/i-task-life import type {QueryResultMetadata} from './query-log-handler.js' import {AnalyticsEventNames} from '../../../shared/analytics/event-names.js' +import {TaskTypes} from '../../../shared/analytics/task-types.js' import {parseFrontmatter} from '../../core/domain/knowledge/markdown-writer.js' import {extractCurateOperations} from '../../utils/curate-result-parser.js' import {processLog} from '../../utils/process-logger.js' import {CURATE_TASK_TYPES} from './curate-log-handler.js' import {QUERY_TASK_TYPES} from './query-log-handler.js' +/** + * Translate the daemon's runtime task type string to the canonical + * analytics wire value. The daemon still dispatches the pre-ENG-2925 + * name `'curate-html-direct'`; analytics emits the post-rename + * `'curate-tool-mode'`. Once the rename PR lands, this becomes a + * no-op identity and can be inlined. + */ +function toAnalyticsTaskType(daemonType: string): TaskType { + if (daemonType === 'curate-html-direct') return TaskTypes.CURATE_TOOL_MODE + return daemonType as TaskType +} + // `CURATE_TASK_TYPES` is exported as a readonly tuple; wrap in a Set // for cast-free `.has()` lookups against TaskInfo.type (string). const CURATE_TASK_TYPE_SET: ReadonlySet = new Set(CURATE_TASK_TYPES) @@ -142,32 +156,52 @@ export class AnalyticsHook implements ITaskLifecycleHook { async onTaskCancelled(taskId: string, task: TaskInfo): Promise { await this.dispatchTerminal(taskId, task, 'cancelled') + this.emitTaskFailed(taskId, task) } async onTaskCompleted(taskId: string, _result: string, task: TaskInfo): Promise { const state = this.tasks.get(taskId) - if (!state) return + if (state) { + // Drain any in-flight per-op processing so CURATE_OPERATION_APPLIED emits + // land BEFORE the run-completion emit on the wire. The chain never + // rejects (see `onToolResult`), so this await is safe. + await this.pendingByTask.get(taskId) + + if (state.flavor === 'curate') { + const outcome = state.counters.failed > 0 ? 'partial' : 'completed' + this.emit( + AnalyticsEventNames.CURATE_RUN_COMPLETED, + this.buildCurateRunPayload({outcome, state, task, taskId}), + ) + } else { + this.emit( + AnalyticsEventNames.QUERY_COMPLETED, + await this.buildQueryCompletedPayload({outcome: 'completed', state, task, taskId}), + ) + } + } - // Drain any in-flight per-op processing so CURATE_OPERATION_APPLIED emits - // land BEFORE the run-completion emit on the wire. The chain never - // rejects (see `onToolResult`), so this await is safe. - await this.pendingByTask.get(taskId) - if (state.flavor === 'curate') { - const outcome = state.counters.failed > 0 ? 'partial' : 'completed' - this.emit( - AnalyticsEventNames.CURATE_RUN_COMPLETED, - this.buildCurateRunPayload({outcome, state, task, taskId}), - ) - } else { - this.emit( - AnalyticsEventNames.QUERY_COMPLETED, - await this.buildQueryCompletedPayload({outcome: 'completed', state, task, taskId}), - ) - } + // M14.3 generic funnel emit. Fires for EVERY task type AFTER any + // per-flavor M12 emit (terminal-event-last convention). + this.emit(AnalyticsEventNames.TASK_COMPLETED, { + duration_ms: this.durationMs(task), + task_id: taskId, + task_type: toAnalyticsTaskType(task.type), + }) } async onTaskCreate(task: TaskInfo): Promise { + // M14.3 generic funnel-entry emit. Fires for EVERY task type BEFORE + // the M12 per-flavor state init so the entry event lands even if + // state setup throws downstream. + this.emit(AnalyticsEventNames.TASK_CREATED, { + has_files: (task.files?.length ?? 0) > 0, + has_folder: typeof task.folderPath === 'string' && task.folderPath.length > 0, + task_id: task.taskId, + task_type: toAnalyticsTaskType(task.type), + }) + if (isCurateLiteral(task.type)) { this.tasks.set(task.taskId, { counters: {added: 0, deleted: 0, failed: 0, merged: 0, pendingReview: 0, updated: 0}, @@ -184,6 +218,7 @@ export class AnalyticsHook implements ITaskLifecycleHook { async onTaskError(taskId: string, _errorMessage: string, task: TaskInfo): Promise { await this.dispatchTerminal(taskId, task, 'error') + this.emitTaskFailed(taskId, task) } async onToolResult(taskId: string, payload: LlmToolResultEvent): Promise { @@ -246,7 +281,7 @@ export class AnalyticsHook implements ITaskLifecycleHook { outcome, pending_review_count: state.counters.pendingReview, task_id: taskId, - task_type: state.taskType, + task_type: toAnalyticsTaskType(state.taskType), } } @@ -331,7 +366,7 @@ export class AnalyticsHook implements ITaskLifecycleHook { read_tool_call_count: readToolCallCount, search_call_count: searchCallCount, task_id: taskId, - task_type: 'query', + task_type: toAnalyticsTaskType(task.type), ...(tier === undefined ? {} : {tier}), } } @@ -371,6 +406,20 @@ export class AnalyticsHook implements ITaskLifecycleHook { } } + /** + * M14.3 generic terminal-failure emit. Fired by both onTaskError and + * onTaskCancelled AFTER dispatchTerminal so M12 per-flavor failure + * emits land first on the wire. Cancellation maps to task_failed + * (not a distinct event) per the schema's docblock. + */ + private emitTaskFailed(taskId: string, task: TaskInfo): void { + this.emit(AnalyticsEventNames.TASK_FAILED, { + duration_ms: this.durationMs(task), + task_id: taskId, + task_type: toAnalyticsTaskType(task.type), + }) + } + private async processToolResult(taskId: string, payload: LlmToolResultEvent): Promise { const state = this.tasks.get(taskId) if (!state || state.flavor !== 'curate') return diff --git a/src/server/infra/process/curate-log-handler.ts b/src/server/infra/process/curate-log-handler.ts index 8f15e777d..aaf36ba59 100644 --- a/src/server/infra/process/curate-log-handler.ts +++ b/src/server/infra/process/curate-log-handler.ts @@ -50,7 +50,12 @@ function telemetryFields(record: CurateUsageRecord | undefined): { } } -export const CURATE_TASK_TYPES = ['curate', 'curate-folder'] as const +// `curate-html-direct` is the pre-ENG-2925 name still dispatched by the +// daemon; `curate-tool-mode` is the post-rename name. Both are listed +// so M12 state init in AnalyticsHook kicks in for tool-mode curates. +// The analytics wire canonicalizes both to `curate-tool-mode` via +// `toAnalyticsTaskType` in `analytics-hook.ts`. +export const CURATE_TASK_TYPES = ['curate', 'curate-folder', 'curate-html-direct', 'curate-tool-mode'] as const // ── Summary computation ─────────────────────────────────────────────────────── diff --git a/src/server/infra/process/query-log-handler.ts b/src/server/infra/process/query-log-handler.ts index 2fec074dd..16a561774 100644 --- a/src/server/infra/process/query-log-handler.ts +++ b/src/server/infra/process/query-log-handler.ts @@ -44,7 +44,10 @@ type TaskState = { queryResult?: QueryResultMetadata } -export const QUERY_TASK_TYPES: ReadonlySet = new Set(['query']) +// `query-tool-mode` is the v4.0 daemon dispatch name; legacy `query` is +// kept for back-compat. Both names enable M12 state init in AnalyticsHook +// (and matching query-log persistence here). +export const QUERY_TASK_TYPES: ReadonlySet = new Set(['query', 'query-tool-mode']) // ── QueryLogHandler ────────────────────────────────────────────────────────── diff --git a/test/integration/infra/process/analytics-hook-async-stress.test.ts b/test/integration/infra/process/analytics-hook-async-stress.test.ts index 669e9d336..417f1fa62 100644 --- a/test/integration/infra/process/analytics-hook-async-stress.test.ts +++ b/test/integration/infra/process/analytics-hook-async-stress.test.ts @@ -322,16 +322,20 @@ describe('AnalyticsHook async stress (integration through TaskRouter)', () => { const sequence = getEmitSequenceForTask(taskId) - // Exactly 50 per-op emits + 1 terminal emit, terminal LAST. + // M14.3: TASK_CREATED → 50 per-op → CURATE_RUN_COMPLETED → TASK_COMPLETED. expect( sequence.filter((s) => s === AnalyticsEventNames.CURATE_OPERATION_APPLIED), 'exactly 50 per-op emits', ).to.have.lengthOf(50) expect( sequence.filter((s) => s === AnalyticsEventNames.CURATE_RUN_COMPLETED), - 'exactly 1 terminal emit', + 'exactly 1 M12 terminal emit', ).to.have.lengthOf(1) - expect(sequence.at(-1), 'terminal is last in sequence').to.equal(AnalyticsEventNames.CURATE_RUN_COMPLETED) + expect(sequence.at(-1), 'M14.3 task_completed is last in sequence').to.equal(AnalyticsEventNames.TASK_COMPLETED) + expect( + sequence.at(-2), + 'M12 curate_run_completed lands immediately before the M14.3 terminal', + ).to.equal(AnalyticsEventNames.CURATE_RUN_COMPLETED) // And per-op emit order matches arrival order. const opEmits = getCurateOpEmits(taskId) @@ -375,15 +379,19 @@ describe('AnalyticsHook async stress (integration through TaskRouter)', () => { await Promise.all([...opPromises, ...terminalPromises]) - // Every task must end with CURATE_RUN_COMPLETED preceded by 10 per-op emits in arrival order. + // Every task: TASK_CREATED → 10 per-op → CURATE_RUN_COMPLETED → TASK_COMPLETED. for (const id of taskIds) { const sequence = getEmitSequenceForTask(id) - expect(sequence, `${id} sequence length`).to.have.lengthOf(11) + expect(sequence, `${id} sequence length`).to.have.lengthOf(13) + expect(sequence[0], `${id}: first is task_created`).to.equal(AnalyticsEventNames.TASK_CREATED) expect( - sequence.slice(0, 10).every((s) => s === AnalyticsEventNames.CURATE_OPERATION_APPLIED), - `${id}: first 10 are per-op emits`, + sequence.slice(1, 11).every((s) => s === AnalyticsEventNames.CURATE_OPERATION_APPLIED), + `${id}: 10 per-op emits between task_created and the terminals`, ).to.equal(true) - expect(sequence[10], `${id}: last is run-completed`).to.equal(AnalyticsEventNames.CURATE_RUN_COMPLETED) + expect(sequence[11], `${id}: M12 run-completed precedes the M14.3 terminal`).to.equal( + AnalyticsEventNames.CURATE_RUN_COMPLETED, + ) + expect(sequence[12], `${id}: M14.3 task_completed is last`).to.equal(AnalyticsEventNames.TASK_COMPLETED) const opEmits = getCurateOpEmits(id) for (let i = 0; i < 10; i++) { expect(opEmits[i].absolute_path, `${id} op #${i} arrival order`).to.equal(specsByTask[id][i].filePath) diff --git a/test/unit/server/infra/process/analytics-hook-m14.test.ts b/test/unit/server/infra/process/analytics-hook-m14.test.ts new file mode 100644 index 000000000..fdff97a29 --- /dev/null +++ b/test/unit/server/infra/process/analytics-hook-m14.test.ts @@ -0,0 +1,215 @@ +import {expect} from 'chai' +import sinon from 'sinon' + +import type {LlmToolResultEvent} from '../../../../../src/server/core/domain/transport/schemas.js' +import type {TaskInfo} from '../../../../../src/server/core/domain/transport/task-info.js' +import type {IAnalyticsClient} from '../../../../../src/server/core/interfaces/analytics/i-analytics-client.js' + +import {AnalyticsBatch} from '../../../../../src/server/core/domain/analytics/batch.js' +import {AnalyticsHook} from '../../../../../src/server/infra/process/analytics-hook.js' +import {AnalyticsEventNames} from '../../../../../src/shared/analytics/event-names.js' + +const FIXED_NOW = 1_700_000_000_000 + +const buildClient = (): {client: IAnalyticsClient; trackStub: sinon.SinonStub} => { + const trackStub = sinon.stub() + const client: IAnalyticsClient = { + abort() { + /* noop */ + }, + flush: sinon.stub().resolves(AnalyticsBatch.create([])), + getRuntimeState: () => Promise.resolve({droppedCount: 0, lastSuccessfulFlushAt: undefined, queueDepth: 0}), + onAuthTransition: sinon.stub().resolves(), + track: trackStub, + } + return {client, trackStub} +} + +const buildTask = (type: string, overrides: Partial = {}): TaskInfo => + ({ + clientId: 'c1', + completedAt: FIXED_NOW + 1234, + content: 'whatever', + createdAt: FIXED_NOW, + folderPath: undefined, + projectPath: '/project', + taskId: `task-${type}-1`, + toolCalls: [], + type, + ...overrides, + }) as TaskInfo + +const buildCurateOpToolResult = (): LlmToolResultEvent => + ({ + callId: 'call-1', + result: JSON.stringify({ + applied: [ + {filePath: '/a.md', needsReview: false, path: 'a', status: 'success', type: 'ADD'}, + ], + }), + sessionId: 's1', + taskId: 'task-curate-1', + timestamp: FIXED_NOW, + toolName: 'curate' as const, + }) as unknown as LlmToolResultEvent + +const eventSequence = (trackStub: sinon.SinonStub): string[] => + trackStub.getCalls().map((c) => c.args[0] as string) + +describe('AnalyticsHook M14.3 generic task_* emit simulation', () => { + let hook: AnalyticsHook + let trackStub: sinon.SinonStub + + beforeEach(() => { + const bundle = buildClient() + trackStub = bundle.trackStub + hook = new AnalyticsHook() + hook.setAnalyticsClient(bundle.client) + }) + + describe('curate task: full success lifecycle (curate-tool-mode rename simulated)', () => { + it('emits task_created on entry, then per-op + curate_run_completed + task_completed on terminal', async () => { + // Daemon dispatches the pre-ENG-2925 name 'curate-html-direct'; + // analytics is expected to alias-translate to 'curate-tool-mode'. + const task = buildTask('curate-html-direct', {taskId: 'task-curate-1'}) + + await hook.onTaskCreate(task) + await hook.onToolResult(task.taskId, buildCurateOpToolResult()) + await hook.onTaskCompleted(task.taskId, '', task) + + expect(eventSequence(trackStub)).to.deep.equal([ + AnalyticsEventNames.TASK_CREATED, + AnalyticsEventNames.CURATE_OPERATION_APPLIED, + AnalyticsEventNames.CURATE_RUN_COMPLETED, + AnalyticsEventNames.TASK_COMPLETED, + ]) + }) + + it('aliases curate-html-direct → curate-tool-mode on the wire (task_type field)', async () => { + const task = buildTask('curate-html-direct', {taskId: 'task-curate-1'}) + await hook.onTaskCreate(task) + await hook.onTaskCompleted(task.taskId, '', task) + + const created = trackStub.getCalls().find((c) => c.args[0] === AnalyticsEventNames.TASK_CREATED) + const completed = trackStub.getCalls().find((c) => c.args[0] === AnalyticsEventNames.TASK_COMPLETED) + expect((created?.args[1] as Record).task_type).to.equal('curate-tool-mode') + expect((completed?.args[1] as Record).task_type).to.equal('curate-tool-mode') + }) + + it('emits task_created has_files=true / has_folder=true when set on TaskInfo', async () => { + const task = buildTask('curate-folder', { + files: ['/a.ts', '/b.ts'], + folderPath: '/some/folder', + taskId: 'task-curate-2', + }) + await hook.onTaskCreate(task) + + const created = trackStub.getCalls().find((c) => c.args[0] === AnalyticsEventNames.TASK_CREATED) + const props = created?.args[1] as Record + expect(props.has_files).to.equal(true) + expect(props.has_folder).to.equal(true) + expect(props.task_type).to.equal('curate-folder') + }) + + it('emits task_created has_files=false / has_folder=false when both are unset', async () => { + const task = buildTask('curate', {files: undefined, folderPath: undefined, taskId: 'task-curate-3'}) + await hook.onTaskCreate(task) + + const created = trackStub.getCalls().find((c) => c.args[0] === AnalyticsEventNames.TASK_CREATED) + const props = created?.args[1] as Record + expect(props.has_files).to.equal(false) + expect(props.has_folder).to.equal(false) + }) + }) + + describe('query task: terminal emits include task_completed last', () => { + it('emits task_created → query_completed → task_completed in order', async () => { + const task = buildTask('query-tool-mode', {taskId: 'task-query-1'}) + + await hook.onTaskCreate(task) + await hook.onTaskCompleted(task.taskId, '', task) + + expect(eventSequence(trackStub)).to.deep.equal([ + AnalyticsEventNames.TASK_CREATED, + AnalyticsEventNames.QUERY_COMPLETED, + AnalyticsEventNames.TASK_COMPLETED, + ]) + }) + + it('emits the same query-tool-mode task_type across all three events', async () => { + const task = buildTask('query-tool-mode', {taskId: 'task-query-1'}) + await hook.onTaskCreate(task) + await hook.onTaskCompleted(task.taskId, '', task) + + for (const call of trackStub.getCalls()) { + const props = call.args[1] as Record + expect(props.task_type, `${call.args[0] as string} carried wrong task_type`).to.equal('query-tool-mode') + } + }) + }) + + describe('dream-scan / dream-finalize / search: only task_* emits fire (no M12 per-flavor)', () => { + for (const taskType of ['dream-scan', 'dream-finalize', 'search'] as const) { + it(`${taskType}: task_created → task_completed (no curate/query M12 emit)`, async () => { + const task = buildTask(taskType, {taskId: `task-${taskType}-1`}) + await hook.onTaskCreate(task) + await hook.onTaskCompleted(task.taskId, '', task) + + expect(eventSequence(trackStub)).to.deep.equal([ + AnalyticsEventNames.TASK_CREATED, + AnalyticsEventNames.TASK_COMPLETED, + ]) + }) + + it(`${taskType}: onTaskError emits task_created then task_failed (no curate/query M12 emit)`, async () => { + const task = buildTask(taskType, {taskId: `task-${taskType}-2`}) + await hook.onTaskCreate(task) + await hook.onTaskError(task.taskId, 'something blew up', task) + + expect(eventSequence(trackStub)).to.deep.equal([ + AnalyticsEventNames.TASK_CREATED, + AnalyticsEventNames.TASK_FAILED, + ]) + }) + } + }) + + describe('failure + cancellation both surface as task_failed', () => { + it('curate onTaskError emits curate_run_completed(outcome=error) then task_failed', async () => { + const task = buildTask('curate', {taskId: 'task-curate-err'}) + await hook.onTaskCreate(task) + trackStub.resetHistory() + await hook.onTaskError(task.taskId, 'kaboom', task) + + expect(eventSequence(trackStub)).to.deep.equal([ + AnalyticsEventNames.CURATE_RUN_COMPLETED, + AnalyticsEventNames.TASK_FAILED, + ]) + }) + + it('curate onTaskCancelled also emits task_failed (no distinct cancellation event)', async () => { + const task = buildTask('curate', {taskId: 'task-curate-cancel'}) + await hook.onTaskCreate(task) + trackStub.resetHistory() + await hook.onTaskCancelled(task.taskId, task) + + expect(eventSequence(trackStub)).to.deep.equal([ + AnalyticsEventNames.CURATE_RUN_COMPLETED, + AnalyticsEventNames.TASK_FAILED, + ]) + }) + + it('task_failed payload carries duration_ms + task_id + canonical task_type', async () => { + const task = buildTask('query', {taskId: 'task-query-err'}) + await hook.onTaskCreate(task) + trackStub.resetHistory() + await hook.onTaskError(task.taskId, 'kaboom', task) + + const failed = trackStub.getCalls().find((c) => c.args[0] === AnalyticsEventNames.TASK_FAILED) + const props = failed?.args[1] as Record + expect(props.task_id).to.equal('task-query-err') + expect(props.task_type).to.equal('query') + expect(props.duration_ms).to.equal(1234) + }) + }) +}) diff --git a/test/unit/server/infra/process/analytics-hook.test.ts b/test/unit/server/infra/process/analytics-hook.test.ts index 87bae7141..7f448c5e1 100644 --- a/test/unit/server/infra/process/analytics-hook.test.ts +++ b/test/unit/server/infra/process/analytics-hook.test.ts @@ -114,6 +114,22 @@ describe('AnalyticsHook', () => { hook.setAnalyticsClient(bundle.client) }) + // M14.3 added unconditional task_created / task_completed / task_failed + // emits on every lifecycle callback. Pre-M14.3 tests asserted only the + // M12 per-flavor curate_*/query_completed emits; filter the stub calls + // so existing assertions stay focused on M12 behavior. New M14.3 + // coverage lives in `analytics-hook-m14.test.ts`. + const filterM12 = (stub: sinon.SinonStub): sinon.SinonSpyCall[] => + stub.getCalls().filter((c) => { + const eventName = c.args[0] + return ( + eventName !== AnalyticsEventNames.TASK_CREATED && + eventName !== AnalyticsEventNames.TASK_COMPLETED && + eventName !== AnalyticsEventNames.TASK_FAILED + ) + }) + const m12Calls = (): sinon.SinonSpyCall[] => filterM12(trackStub) + describe('curate task flow', () => { it('emits curate_operation_applied per successful op + bumps matching counter; no event for failed op', async () => { const task = buildCurateTask() @@ -126,9 +142,9 @@ describe('AnalyticsHook', () => { ]) await hook.onToolResult(task.taskId, payload) - expect(trackStub.callCount).to.equal(2) - expect(trackStub.firstCall.args[0]).to.equal(AnalyticsEventNames.CURATE_OPERATION_APPLIED) - const firstProps = trackStub.firstCall.args[1] as Record + expect(m12Calls()).to.have.lengthOf(2) + expect(m12Calls()[0].args[0]).to.equal(AnalyticsEventNames.CURATE_OPERATION_APPLIED) + const firstProps = m12Calls()[0].args[1] as Record expect(firstProps.absolute_path).to.equal('/a.md') expect(firstProps.knowledge_path).to.equal('notes/a') expect(firstProps.operation_type).to.equal('ADD') @@ -137,7 +153,7 @@ describe('AnalyticsHook', () => { expect(firstProps).to.not.have.property('keywords') expect(firstProps).to.not.have.property('related') - const secondProps = trackStub.secondCall.args[1] as Record + const secondProps = m12Calls()[1].args[1] as Record expect(secondProps.needs_review).to.equal(true) expect(secondProps.operation_type).to.equal('UPDATE') }) @@ -157,9 +173,9 @@ describe('AnalyticsHook', () => { await hook.onTaskCompleted(task.taskId, '', task) - expect(trackStub.calledOnce).to.equal(true) - expect(trackStub.firstCall.args[0]).to.equal(AnalyticsEventNames.CURATE_RUN_COMPLETED) - const props = trackStub.firstCall.args[1] as Record + expect(m12Calls()).to.have.lengthOf(1) + expect(m12Calls()[0].args[0]).to.equal(AnalyticsEventNames.CURATE_RUN_COMPLETED) + const props = m12Calls()[0].args[1] as Record expect(props.task_id).to.equal(task.taskId) expect(props.task_type).to.equal('curate') expect(props.outcome).to.equal('completed') @@ -186,7 +202,7 @@ describe('AnalyticsHook', () => { await hook.onTaskCompleted(task.taskId, '', task) - const props = trackStub.firstCall.args[1] as Record + const props = m12Calls()[0].args[1] as Record expect(props.outcome).to.equal('partial') expect(props.operations_failed).to.equal(1) }) @@ -198,7 +214,7 @@ describe('AnalyticsHook', () => { await hook.onTaskError(task.taskId, 'boom', task) - const props = trackStub.firstCall.args[1] as Record + const props = m12Calls()[0].args[1] as Record expect(props.outcome).to.equal('error') }) @@ -209,7 +225,7 @@ describe('AnalyticsHook', () => { await hook.onTaskCancelled(task.taskId, task) - const props = trackStub.firstCall.args[1] as Record + const props = m12Calls()[0].args[1] as Record expect(props.outcome).to.equal('cancelled') }) @@ -227,7 +243,7 @@ describe('AnalyticsHook', () => { await hook.onTaskCompleted(task.taskId, '', task) - const props = trackStub.firstCall.args[1] as Record + const props = m12Calls()[0].args[1] as Record expect(props.operations_added).to.equal(1) expect(props.operations_updated).to.equal(1) }) @@ -246,7 +262,7 @@ describe('AnalyticsHook', () => { await hook.onTaskCompleted(task.taskId, '', task) - const props = trackStub.firstCall.args[1] as Record + const props = m12Calls()[0].args[1] as Record expect(props.pending_review_count).to.equal(2) }) @@ -255,7 +271,7 @@ describe('AnalyticsHook', () => { await hook.onTaskCreate(task) await hook.onTaskCompleted(task.taskId, '', task) - const props = trackStub.firstCall.args[1] as Record + const props = m12Calls()[0].args[1] as Record expect(props.task_type).to.equal('curate-folder') }) @@ -267,7 +283,9 @@ describe('AnalyticsHook', () => { buildToolResult([{needsReview: false, path: 'a', status: 'success', type: 'ADD'}]), ) - expect(trackStub.called).to.equal(false) + // No curate_operation_applied for an op missing filePath. (TASK_CREATED + // still fires from onTaskCreate — filtered out via m12Calls().) + expect(m12Calls()).to.have.lengthOf(0) }) }) @@ -298,9 +316,9 @@ describe('AnalyticsHook', () => { } as QueryResultMetadata) await hook.onTaskCompleted(task.taskId, '', task) - expect(trackStub.calledOnce).to.equal(true) - expect(trackStub.firstCall.args[0]).to.equal(AnalyticsEventNames.QUERY_COMPLETED) - const props = trackStub.firstCall.args[1] as Record + expect(m12Calls()).to.have.lengthOf(1) + expect(m12Calls()[0].args[0]).to.equal(AnalyticsEventNames.QUERY_COMPLETED) + const props = m12Calls()[0].args[1] as Record expect(props.task_id).to.equal(task.taskId) expect(props.task_type).to.equal('query') expect(props.outcome).to.equal('completed') @@ -336,7 +354,7 @@ describe('AnalyticsHook', () => { await hook.onTaskCreate(task) await hook.onTaskCompleted(task.taskId, '', task) - const props = trackStub.firstCall.args[1] as Record + const props = m12Calls()[0].args[1] as Record const paths = props.read_paths_with_metadata as Array> expect(paths).to.have.lengthOf(10) expect(props.read_doc_count).to.equal(15) // distinct count NOT capped @@ -357,7 +375,7 @@ describe('AnalyticsHook', () => { } as QueryResultMetadata) await localHook.onTaskCompleted(task.taskId, '', task) - const props = localBundle.trackStub.firstCall.args[1] as Record + const props = filterM12(localBundle.trackStub)[0].args[1] as Record expect(props.cache_hit).to.equal(true) }) } @@ -377,7 +395,7 @@ describe('AnalyticsHook', () => { } as QueryResultMetadata) await localHook.onTaskCompleted(task.taskId, '', task) - const props = localBundle.trackStub.firstCall.args[1] as Record + const props = filterM12(localBundle.trackStub)[0].args[1] as Record expect(props.cache_hit).to.equal(false) }) } @@ -387,7 +405,7 @@ describe('AnalyticsHook', () => { await hook.onTaskCreate(task) await hook.onTaskCompleted(task.taskId, '', task) - const props = trackStub.firstCall.args[1] as Record + const props = m12Calls()[0].args[1] as Record expect(props.tier).to.equal(undefined) expect(props.cache_hit).to.equal(false) expect(props.matched_doc_count).to.equal(0) @@ -398,7 +416,7 @@ describe('AnalyticsHook', () => { await hook.onTaskCreate(task) await hook.onTaskCompleted(task.taskId, '', task) - const props = trackStub.firstCall.args[1] as Record + const props = m12Calls()[0].args[1] as Record expect(props).to.not.have.property('read_paths_with_metadata') // Sanity: counts are zero, not omitted. expect(props.read_doc_count).to.equal(0) @@ -411,7 +429,7 @@ describe('AnalyticsHook', () => { await hook.onTaskError(task.taskId, 'boom', task) - const props = trackStub.firstCall.args[1] as Record + const props = m12Calls()[0].args[1] as Record expect(props.outcome).to.equal('error') }) @@ -421,7 +439,7 @@ describe('AnalyticsHook', () => { await hook.onTaskCancelled(task.taskId, task) - const props = trackStub.firstCall.args[1] as Record + const props = m12Calls()[0].args[1] as Record expect(props.outcome).to.equal('cancelled') }) }) @@ -435,18 +453,19 @@ describe('AnalyticsHook', () => { hook.cleanup(curate.taskId) hook.cleanup(query.taskId) - // After cleanup, terminal hooks should be no-ops + // After cleanup, M12 per-flavor emits must NOT fire (no state to read). + // M14.3 generic TASK_COMPLETED still fires unconditionally — filtered. trackStub.resetHistory() await hook.onTaskCompleted(curate.taskId, '', curate) await hook.onTaskCompleted(query.taskId, '', query) - expect(trackStub.called).to.equal(false) + expect(m12Calls()).to.have.lengthOf(0) }) - it('ignores unknown task types (no state created)', async () => { + it('ignores unknown task types (no M12 state created; only generic task_* emits fire)', async () => { const task = buildCurateTask({taskId: 'task-unknown', type: 'unknown' as TaskInfo['type']}) await hook.onTaskCreate(task) await hook.onTaskCompleted(task.taskId, '', task) - expect(trackStub.called).to.equal(false) + expect(m12Calls()).to.have.lengthOf(0) }) it('swallows analyticsClient.track throws (does not propagate)', async () => { @@ -497,7 +516,7 @@ describe('AnalyticsHook', () => { buildToolResult([{filePath, needsReview: false, path: 'a', status: 'success', type: 'ADD'}]), ) - const props = trackStub.firstCall.args[1] as Record + const props = m12Calls()[0].args[1] as Record expect(props.tags).to.deep.equal(['t1', 't2']) expect(props.keywords).to.deep.equal(['x', 'y']) expect(props.related).to.deep.equal(['z']) @@ -512,7 +531,7 @@ describe('AnalyticsHook', () => { buildToolResult([{filePath, needsReview: false, path: 'gone', status: 'success', type: 'DELETE'}]), ) - const props = trackStub.firstCall.args[1] as Record + const props = m12Calls()[0].args[1] as Record expect(props).to.not.have.property('tags') expect(props).to.not.have.property('keywords') expect(props).to.not.have.property('related') @@ -527,7 +546,7 @@ describe('AnalyticsHook', () => { buildToolResult([{filePath, needsReview: false, path: 'm', status: 'success', type: 'UPDATE'}]), ) - const props = trackStub.firstCall.args[1] as Record + const props = m12Calls()[0].args[1] as Record expect(props).to.not.have.property('tags') }) @@ -542,7 +561,7 @@ describe('AnalyticsHook', () => { buildToolResult([{filePath, needsReview: false, path: 'b', status: 'success', type: 'UPDATE'}]), ) - const props = trackStub.firstCall.args[1] as Record + const props = m12Calls()[0].args[1] as Record expect(props).to.not.have.property('tags') }) @@ -559,7 +578,7 @@ describe('AnalyticsHook', () => { buildToolResult([{filePath, needsReview: false, path: 'h', status: 'success', type: 'UPDATE'}]), ) - const props = trackStub.firstCall.args[1] as Record + const props = m12Calls()[0].args[1] as Record const tags = props.tags as string[] expect(tags).to.have.lengthOf(50) expect(tags[0]).to.have.lengthOf(256) @@ -580,7 +599,7 @@ describe('AnalyticsHook', () => { buildToolResult([{filePath, needsReview: false, path: 'g', status: 'success', type: 'UPDATE'}]), ) - const props = disabledBundle.trackStub.firstCall.args[1] as Record + const props = filterM12(disabledBundle.trackStub)[0].args[1] as Record expect(props).to.not.have.property('tags') }) }) @@ -602,7 +621,7 @@ describe('AnalyticsHook', () => { await hook.onTaskCreate(task) await hook.onTaskCompleted(task.taskId, '', task) - const props = trackStub.firstCall.args[1] as Record + const props = m12Calls()[0].args[1] as Record const paths = props.read_paths_with_metadata as Array> const byPath = Object.fromEntries(paths.map((p) => [p.absolute_path, p])) expect(byPath[a].tags).to.deep.equal(['ta']) @@ -626,7 +645,7 @@ describe('AnalyticsHook', () => { await hook.onTaskCreate(task) await hook.onTaskCompleted(task.taskId, '', task) - const props = trackStub.firstCall.args[1] as Record + const props = m12Calls()[0].args[1] as Record const paths = props.read_paths_with_metadata as Array> const byPath = Object.fromEntries(paths.map((p) => [p.absolute_path, p])) expect(byPath[real].tags).to.deep.equal(['ok']) @@ -651,7 +670,7 @@ describe('AnalyticsHook', () => { await disabledHook.onTaskCreate(task) await disabledHook.onTaskCompleted(task.taskId, '', task) - const props = disabledBundle.trackStub.firstCall.args[1] as Record + const props = filterM12(disabledBundle.trackStub)[0].args[1] as Record const paths = props.read_paths_with_metadata as Array> expect(paths[0]).to.not.have.property('tags') }) @@ -690,9 +709,9 @@ describe('AnalyticsHook', () => { await Promise.all([p1, p2]) - expect(bundle.trackStub.callCount).to.equal(2) - const first = bundle.trackStub.firstCall.args[1] as Record - const second = bundle.trackStub.secondCall.args[1] as Record + expect(filterM12(bundle.trackStub)).to.have.lengthOf(2) + const first = filterM12(bundle.trackStub)[0].args[1] as Record + const second = filterM12(bundle.trackStub)[1].args[1] as Record expect(first.absolute_path, 'first emit must be op1').to.equal('/op1.md') expect(second.absolute_path, 'second emit must be op2').to.equal('/op2.md') }) @@ -717,15 +736,16 @@ describe('AnalyticsHook', () => { const opPromise = orderHook.onToolResult(task.taskId, payload) const completePromise = orderHook.onTaskCompleted(task.taskId, '', task) - // Neither emit can have fired yet — read is still pending. - expect(bundle.trackStub.called, 'no emit before read settles').to.equal(false) + // Neither M12 emit can have fired yet — read is still pending. (TASK_CREATED + // from M14.3 already fired during onTaskCreate but doesn't gate on the read.) + expect(filterM12(bundle.trackStub), 'no M12 emit before read settles').to.have.lengthOf(0) d.resolve(buildFrontmatterDoc('tag-x')) await Promise.all([opPromise, completePromise]) - expect(bundle.trackStub.callCount).to.equal(2) - expect(bundle.trackStub.firstCall.args[0]).to.equal(AnalyticsEventNames.CURATE_OPERATION_APPLIED) - expect(bundle.trackStub.secondCall.args[0]).to.equal(AnalyticsEventNames.CURATE_RUN_COMPLETED) + expect(filterM12(bundle.trackStub)).to.have.lengthOf(2) + expect(filterM12(bundle.trackStub)[0].args[0]).to.equal(AnalyticsEventNames.CURATE_OPERATION_APPLIED) + expect(filterM12(bundle.trackStub)[1].args[0]).to.equal(AnalyticsEventNames.CURATE_RUN_COMPLETED) }) it('readFile rejection is swallowed: emit fires with frontmatter fields omitted; daemon does not crash', async () => { @@ -743,8 +763,8 @@ describe('AnalyticsHook', () => { await errorHook.onToolResult(task.taskId, payload) - expect(bundle.trackStub.calledOnce).to.equal(true) - const props = bundle.trackStub.firstCall.args[1] as Record + expect(filterM12(bundle.trackStub)).to.have.lengthOf(1) + const props = filterM12(bundle.trackStub)[0].args[1] as Record expect(props.absolute_path).to.equal('/missing.md') expect(props).to.not.have.property('keywords') expect(props).to.not.have.property('tags') @@ -769,7 +789,10 @@ describe('AnalyticsHook', () => { // directly, but the assertion below catches the leak: a new task with the same // id observes a fresh in-memory state. await cleanupHook.onTaskCreate(task) - expect(bundle.trackStub.callCount, 'no replay after cleanup').to.equal(2) + // M12 emits: 1 curate_operation_applied + 1 curate_run_completed = 2. + // Re-creating the task after cleanup must NOT replay either; it only + // adds another TASK_CREATED (filtered out below). + expect(filterM12(bundle.trackStub), 'no replay after cleanup').to.have.lengthOf(2) }) }) }) From a943aacbabb40f9ed26cb54f3dc5ab15c8c235ac Mon Sep 17 00:00:00 2001 From: Cuong Date: Wed, 27 May 2026 17:26:58 +0700 Subject: [PATCH 4/4] =?UTF-8?q?refactor:=20[ENG-2971]=20M14.3=20review=20f?= =?UTF-8?q?ix=20=E2=80=94=20relative=20paths,=20required=20keywords/tags,?= =?UTF-8?q?=20structured=20related=5Fpaths?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tightens the curate / query M12 payloads per code review: curate_operation_applied: - rename absolute_path → relative_path (project-relative via path.relative against the task's projectPath; falls back to identity when projectPath is unset so search tasks at the daemon root still emit a usable string) - keywords / tags promoted from optional to required arrays (default []) so the wire shape stays uniform regardless of frontmatter read success query_completed.read_paths_with_metadata: - same absolute_path → relative_path rename - same keywords / tags promotion (required arrays, default []) - flat related: string[] → structured related_paths: [{relative_path, keywords, tags}] so each linked topic carries its own metadata slot. Keywords/tags default to [] until a future FU cascade-reads each linked file's frontmatter — the wire shape doesn't need to change when that lands. Hook implementation: - new `toRelativePath(filePath, projectPath)` helper using node:path.relative - CurateTaskAnalyticsState stores projectPath captured at onTaskCreate so per-op emits can relativize without threading task through processToolResult - all four payload sites (curate-op, curate-run-completed, query-completed read_paths_with_metadata) route through the helper Inspection test added at test/unit/.../analytics-hook-toolmode-inspection that pretty-prints every event + payload for curate-tool-mode / query / query-tool-mode flows — gives PMs a single place to verify the wire shape end-to-end. Privacy win: relative paths drop the /Users/{name} prefix from every file-touched event, keeping host-identifiable PII off the analytics wire while still letting PMs reason about which file inside a project an operation touched. Tests: full repo 9132 passing. --- src/server/infra/process/analytics-hook.ts | 50 +++- .../events/curate-operation-applied.ts | 20 +- .../analytics/events/query-completed.ts | 37 ++- test/integration/analytics/transport.test.ts | 6 +- .../analytics-hook-async-stress.test.ts | 16 +- .../infra/analytics/analytics-client.test.ts | 8 +- .../analytics/no-op-analytics-client.test.ts | 4 +- ...analytics-hook-toolmode-inspection.test.ts | 260 ++++++++++++++++++ .../infra/process/analytics-hook.test.ts | 78 +++--- .../handlers/analytics-handler.test.ts | 10 +- .../events/curate-operation-applied.test.ts | 21 +- .../analytics/events/query-completed.test.ts | 84 ++++-- 12 files changed, 499 insertions(+), 95 deletions(-) create mode 100644 test/unit/server/infra/process/analytics-hook-toolmode-inspection.test.ts diff --git a/src/server/infra/process/analytics-hook.ts b/src/server/infra/process/analytics-hook.ts index 307c7bba6..551cf8340 100644 --- a/src/server/infra/process/analytics-hook.ts +++ b/src/server/infra/process/analytics-hook.ts @@ -1,5 +1,6 @@ /* eslint-disable camelcase */ import {readFile as readFileAsync} from 'node:fs/promises' +import {relative as relativePath} from 'node:path' import type {AnalyticsEventName} from '../../../shared/analytics/event-names.js' import type {CurateRunCompletedProps} from '../../../shared/analytics/events/curate-run-completed.js' @@ -32,6 +33,22 @@ function toAnalyticsTaskType(daemonType: string): TaskType { return daemonType as TaskType } +/** + * Convert an absolute filesystem path to a project-relative path for the + * analytics wire. Falls back to the input unchanged when projectPath is + * unset (e.g., search tasks scoped to the daemon root). Keeps emits free + * of `/Users/{name}` PII while still letting PMs reason about which file + * inside a project an operation touched. + */ +function toRelativePath(filePath: string, projectPath?: string): string { + if (!projectPath) return filePath + const rel = relativePath(projectPath, filePath) + // `path.relative` returns '' when paths are identical — defensively + // surface a leaf token rather than emit a zero-length wire string that + // would fail `z.string().min(1)`. + return rel === '' ? '.' : rel +} + // `CURATE_TASK_TYPES` is exported as a readonly tuple; wrap in a Set // for cast-free `.has()` lookups against TaskInfo.type (string). const CURATE_TASK_TYPE_SET: ReadonlySet = new Set(CURATE_TASK_TYPES) @@ -81,6 +98,8 @@ type CurateCounters = { type CurateTaskAnalyticsState = { counters: CurateCounters flavor: 'curate' + /** Captured at onTaskCreate so onToolResult emits can relativize op.filePath. */ + projectPath?: string taskType: CurateTaskTypeLiteral } @@ -206,6 +225,7 @@ export class AnalyticsHook implements ITaskLifecycleHook { this.tasks.set(task.taskId, { counters: {added: 0, deleted: 0, failed: 0, merged: 0, pendingReview: 0, updated: 0}, flavor: 'curate', + projectPath: task.projectPath, taskType: task.type, }) return @@ -338,17 +358,24 @@ export class AnalyticsHook implements ITaskLifecycleHook { // M12.3: harvest per-path frontmatter on the same async read path used // for curate emits. Entries whose file is unreadable / has no frontmatter - // carry `absolute_path` alone (the three array fields stay absent). - // `Promise.all` preserves input-array order in the result regardless of - // which read settles first. + // carry empty keywords / tags / related_paths arrays — the wire shape + // is uniform regardless of read success. `Promise.all` preserves + // input-array order in the result regardless of which read settles first. const readPathsWithMetadata = await Promise.all( cappedPaths.map(async (p) => { const fm = await this.readFrontmatterFields(p) return { - absolute_path: p, - ...(fm.keywords ? {keywords: fm.keywords} : {}), - ...(fm.related ? {related: fm.related} : {}), - ...(fm.tags ? {tags: fm.tags} : {}), + keywords: fm.keywords ?? [], + // M14 review tightening: each related entry is structured so a + // later FU can populate the linked file's own keywords/tags + // without changing the wire shape. + related_paths: (fm.related ?? []).map((r) => ({ + keywords: [], + relative_path: r, + tags: [], + })), + relative_path: toRelativePath(p, task.projectPath), + tags: fm.tags ?? [], } }), ) @@ -472,20 +499,21 @@ export class AnalyticsHook implements ITaskLifecycleHook { // M12.3: read post-op frontmatter for ADD / UPDATE / MERGE-target / // UPSERT. DELETE skips the read (file is gone). Frontmatter fields - // stay absent when the read fails (ENOENT, EACCES, malformed YAML). + // default to empty arrays when the read fails (ENOENT, EACCES, + // malformed YAML) so the wire shape stays uniform. // eslint-disable-next-line no-await-in-loop -- emit order MUST match op order const frontmatter = op.type === 'DELETE' ? {} : await this.readFrontmatterFields(op.filePath) this.emit(AnalyticsEventNames.CURATE_OPERATION_APPLIED, { - absolute_path: op.filePath, ...(op.confidence ? {confidence: op.confidence} : {}), ...(op.impact ? {impact: op.impact} : {}), - ...(frontmatter.keywords ? {keywords: frontmatter.keywords} : {}), + keywords: frontmatter.keywords ?? [], knowledge_path: op.path, needs_review: op.needsReview ?? false, operation_type: op.type, ...(frontmatter.related ? {related: frontmatter.related} : {}), - ...(frontmatter.tags ? {tags: frontmatter.tags} : {}), + relative_path: toRelativePath(op.filePath, state.projectPath), + tags: frontmatter.tags ?? [], task_id: taskId, }) } diff --git a/src/shared/analytics/events/curate-operation-applied.ts b/src/shared/analytics/events/curate-operation-applied.ts index 12f4a0564..c7e9839b0 100644 --- a/src/shared/analytics/events/curate-operation-applied.ts +++ b/src/shared/analytics/events/curate-operation-applied.ts @@ -5,24 +5,28 @@ import {z} from 'zod' * Per-event schema for `curate_operation_applied`. * * Emitted by the daemon's `AnalyticsHook` (M12.2) once per successful curate - * operation. Each operation carries the affected file's absolute path, its - * knowledge-tree address, review/impact metadata, and (M12.3) the file's - * current-state frontmatter values for tags / keywords / related. + * operation. Each operation carries the affected file's project-relative + * path, its knowledge-tree address, review/impact metadata, and (M12.3) the + * file's current-state frontmatter values for tags / keywords / related. * - * All three frontmatter arrays are optional and absent on DELETE operations - * (the file is gone post-op) and on read failures (defensive). + * Review tightening (M14 follow-up): + * - `absolute_path` → `relative_path` for privacy + portability across hosts + * - `keywords` / `tags` are now required arrays (default empty) so consumers + * don't have to special-case the "field absent" shape + * - `related` stays optional and absent on DELETE / read-failure (file is + * gone or unreadable, no related-link source to harvest from) */ export const CurateOperationAppliedSchema = z .object({ - absolute_path: z.string().min(1), confidence: z.enum(['high', 'low']).optional(), impact: z.enum(['high', 'low']).optional(), - keywords: z.array(z.string().max(256)).max(50).optional(), + keywords: z.array(z.string().max(256)).max(50), knowledge_path: z.string().min(1), needs_review: z.boolean(), operation_type: z.enum(['ADD', 'UPDATE', 'DELETE', 'MERGE', 'UPSERT']), related: z.array(z.string().max(256)).max(50).optional(), - tags: z.array(z.string().max(256)).max(50).optional(), + relative_path: z.string().min(1), + tags: z.array(z.string().max(256)).max(50), task_id: z.string().min(1), }) .strict() diff --git a/src/shared/analytics/events/query-completed.ts b/src/shared/analytics/events/query-completed.ts index f6058897a..d7bc2865e 100644 --- a/src/shared/analytics/events/query-completed.ts +++ b/src/shared/analytics/events/query-completed.ts @@ -3,18 +3,41 @@ import {z} from 'zod' import {TASK_TYPE_VALUES} from '../task-types.js' +/** + * Per related-path metadata. Each related entry is a project-relative + * knowledge path captured from a read file's frontmatter `related` list, + * carrying its own keywords / tags so PMs can see what the linked-from + * topics actually cover. + * + * keywords / tags default to `[]` when the related file isn't on disk or + * when analytics is disabled (no enrichment read happens). The shape is + * structured here so a later FU can fill keywords/tags without a wire + * format change. + */ +const RelatedPathWithMetadataSchema = z + .object({ + keywords: z.array(z.string().max(256)).max(50), + relative_path: z.string().min(1), + tags: z.array(z.string().max(256)).max(50), + }) + .strict() + /** * Per-file structure inside `query_completed.read_paths_with_metadata`. - * Frontmatter arrays are optional and absent when the daemon cannot read - * the file (ENOENT, parse failure) — `absolute_path` alone still tells - * PMs which file the agent touched. + * + * Review tightening (M14 follow-up): + * - `absolute_path` → `relative_path` for privacy + portability + * - `keywords` / `tags` are now required arrays (default `[]`) so the + * "field absent" wire shape goes away + * - flat `related: string[]` → structured `related_paths: [{relative_path, + * keywords, tags}]` so each linked topic carries its own metadata */ const ReadPathWithMetadataSchema = z .object({ - absolute_path: z.string().min(1), - keywords: z.array(z.string().max(256)).max(50).optional(), - related: z.array(z.string().max(256)).max(50).optional(), - tags: z.array(z.string().max(256)).max(50).optional(), + keywords: z.array(z.string().max(256)).max(50), + related_paths: z.array(RelatedPathWithMetadataSchema).max(50), + relative_path: z.string().min(1), + tags: z.array(z.string().max(256)).max(50), }) .strict() diff --git a/test/integration/analytics/transport.test.ts b/test/integration/analytics/transport.test.ts index 7e8bba997..157cc90f6 100644 --- a/test/integration/analytics/transport.test.ts +++ b/test/integration/analytics/transport.test.ts @@ -99,10 +99,12 @@ describe('analytics:track transport round-trip integration (M2.6)', () => { { event: AnalyticsEventNames.CURATE_OPERATION_APPLIED, properties: { - absolute_path: '/tmp/x.md', + keywords: [], knowledge_path: 'kg/x.md', needs_review: false, operation_type: 'ADD', + relative_path: 'tmp/x.md', + tags: [], task_id: 't-1', }, }, @@ -118,7 +120,7 @@ describe('analytics:track transport round-trip integration (M2.6)', () => { expect(event.identity).to.deep.equal({device_id: validDeviceId}) // User-supplied properties preserved end-to-end - expect(event.properties.absolute_path).to.equal('/tmp/x.md') + expect(event.properties.relative_path).to.equal('tmp/x.md') expect(event.properties.operation_type).to.equal('ADD') // All five super-properties stamped on receipt diff --git a/test/integration/infra/process/analytics-hook-async-stress.test.ts b/test/integration/infra/process/analytics-hook-async-stress.test.ts index 417f1fa62..59951c20b 100644 --- a/test/integration/infra/process/analytics-hook-async-stress.test.ts +++ b/test/integration/infra/process/analytics-hook-async-stress.test.ts @@ -15,6 +15,7 @@ import {expect} from 'chai' import {randomUUID} from 'node:crypto' +import {relative as relativePath} from 'node:path' import {createSandbox, type SinonSandbox, type SinonStub} from 'sinon' import type {LlmToolResultEvent} from '../../../../src/server/core/domain/transport/schemas.js' @@ -260,11 +261,12 @@ describe('AnalyticsHook async stress (integration through TaskRouter)', () => { // readFile call order must match arrival order (proves per-task queue). expect(readFileCallOrder, 'readFile call order = arrival order').to.deep.equal(opSpecs.map((s) => s.filePath)) - // Emit order must match arrival order. + // Emit order must match arrival order. M14 review: relative_path is + // relativized against the curate task's projectPath ('/proj'). const emits = getCurateOpEmits(taskId) expect(emits).to.have.lengthOf(20) for (const [i, emit] of emits.entries()) { - expect(emit.absolute_path, `emit #${i} arrival order`).to.equal(opSpecs[i].filePath) + expect(emit.relative_path, `emit #${i} arrival order`).to.equal(relativePath('/proj', opSpecs[i].filePath)) } }) @@ -297,8 +299,8 @@ describe('AnalyticsHook async stress (integration through TaskRouter)', () => { expect(xEmits).to.have.lengthOf(15) expect(yEmits).to.have.lengthOf(15) for (let i = 0; i < 15; i++) { - expect(xEmits[i].absolute_path, `X emit #${i}`).to.equal(xSpecs[i].filePath) - expect(yEmits[i].absolute_path, `Y emit #${i}`).to.equal(ySpecs[i].filePath) + expect(xEmits[i].relative_path, `X emit #${i}`).to.equal(relativePath('/proj', xSpecs[i].filePath)) + expect(yEmits[i].relative_path, `Y emit #${i}`).to.equal(relativePath('/proj', ySpecs[i].filePath)) } }) @@ -340,7 +342,7 @@ describe('AnalyticsHook async stress (integration through TaskRouter)', () => { // And per-op emit order matches arrival order. const opEmits = getCurateOpEmits(taskId) for (let i = 0; i < 50; i++) { - expect(opEmits[i].absolute_path, `op #${i} arrival order`).to.equal(specs[i].filePath) + expect(opEmits[i].relative_path, `op #${i} arrival order`).to.equal(relativePath('/proj', specs[i].filePath)) } }) @@ -394,7 +396,9 @@ describe('AnalyticsHook async stress (integration through TaskRouter)', () => { expect(sequence[12], `${id}: M14.3 task_completed is last`).to.equal(AnalyticsEventNames.TASK_COMPLETED) const opEmits = getCurateOpEmits(id) for (let i = 0; i < 10; i++) { - expect(opEmits[i].absolute_path, `${id} op #${i} arrival order`).to.equal(specsByTask[id][i].filePath) + expect(opEmits[i].relative_path, `${id} op #${i} arrival order`).to.equal( + relativePath('/proj', specsByTask[id][i].filePath), + ) } } }) diff --git a/test/unit/server/infra/analytics/analytics-client.test.ts b/test/unit/server/infra/analytics/analytics-client.test.ts index 023bdeaf6..906614ec0 100644 --- a/test/unit/server/infra/analytics/analytics-client.test.ts +++ b/test/unit/server/infra/analytics/analytics-client.test.ts @@ -23,10 +23,12 @@ import {AnalyticsEventNames} from '../../../../../src/shared/analytics/event-nam */ function makeCurateOpProps(overrides: Partial = {}): CurateOperationAppliedProps { return { - absolute_path: '/tmp/file.md', + keywords: [], knowledge_path: 'cli_architecture/test.md', needs_review: false, operation_type: 'ADD', + relative_path: 'tmp/file.md', + tags: [], task_id: 'task-1', ...overrides, } @@ -233,7 +235,7 @@ describe('AnalyticsClient', () => { }) const before = Date.now() - const opProps = makeCurateOpProps({absolute_path: '/tmp/merge-fixture.md'}) + const opProps = makeCurateOpProps({relative_path: 'tmp/merge-fixture.md'}) client.track(AnalyticsEventNames.CURATE_OPERATION_APPLIED, opProps) await flushMicrotasks() const after = Date.now() @@ -248,7 +250,7 @@ describe('AnalyticsClient', () => { expect(event.timestamp).to.be.at.most(after) // user properties merged through - expect(event.properties.absolute_path).to.equal('/tmp/merge-fixture.md') + expect(event.properties.relative_path).to.equal('tmp/merge-fixture.md') expect(event.properties.operation_type).to.equal('ADD') // all 5 super properties stamped expect(event.properties.cli_version).to.equal('3.10.3') diff --git a/test/unit/server/infra/analytics/no-op-analytics-client.test.ts b/test/unit/server/infra/analytics/no-op-analytics-client.test.ts index dd88cb076..f178efa2b 100644 --- a/test/unit/server/infra/analytics/no-op-analytics-client.test.ts +++ b/test/unit/server/infra/analytics/no-op-analytics-client.test.ts @@ -15,10 +15,12 @@ describe('NoOpAnalyticsClient', () => { // CURATE_OPERATION_APPLIED with a minimal valid payload. expect(() => client.track(AnalyticsEventNames.CURATE_OPERATION_APPLIED, { - absolute_path: '/tmp/x.md', + keywords: [], knowledge_path: 'kg/x.md', needs_review: false, operation_type: 'ADD', + relative_path: 'tmp/x.md', + tags: [], task_id: 't-1', }), ).to.not.throw() diff --git a/test/unit/server/infra/process/analytics-hook-toolmode-inspection.test.ts b/test/unit/server/infra/process/analytics-hook-toolmode-inspection.test.ts new file mode 100644 index 000000000..57ae6b75b --- /dev/null +++ b/test/unit/server/infra/process/analytics-hook-toolmode-inspection.test.ts @@ -0,0 +1,260 @@ +/* eslint-disable camelcase */ +import {expect} from 'chai' +import sinon from 'sinon' + +import type {LlmToolResultEvent} from '../../../../../src/server/core/domain/transport/schemas.js' +import type {TaskInfo} from '../../../../../src/server/core/domain/transport/task-info.js' +import type {IAnalyticsClient} from '../../../../../src/server/core/interfaces/analytics/i-analytics-client.js' +import type {QueryResultMetadata} from '../../../../../src/server/infra/process/query-log-handler.js' + +import {AnalyticsBatch} from '../../../../../src/server/core/domain/analytics/batch.js' +import {AnalyticsHook} from '../../../../../src/server/infra/process/analytics-hook.js' + +const NOW = 1_700_000_000_000 + +const buildClient = (): {client: IAnalyticsClient; trackStub: sinon.SinonStub} => { + const trackStub = sinon.stub() + const client: IAnalyticsClient = { + abort() { + /* noop */ + }, + flush: sinon.stub().resolves(AnalyticsBatch.create([])), + getRuntimeState: () => Promise.resolve({droppedCount: 0, lastSuccessfulFlushAt: undefined, queueDepth: 0}), + onAuthTransition: sinon.stub().resolves(), + track: trackStub, + } + return {client, trackStub} +} + +const buildToolModeCurateTask = (overrides: Partial = {}): TaskInfo => + ({ + clientId: 'agent-1', + completedAt: NOW + 4321, + content: 'JSON envelope describing the html the calling agent already wrote', + createdAt: NOW, + // Daemon today still dispatches the pre-ENG-2925 name; analytics + // aliases it to 'curate-tool-mode' on the wire via toAnalyticsTaskType. + projectPath: '/Users/dev/example-project', + taskId: 'task-curate-tm-1', + type: 'curate-html-direct', + ...overrides, + }) as TaskInfo + +const buildToolModeQueryTask = (overrides: Partial = {}): TaskInfo => + ({ + clientId: 'agent-1', + completedAt: NOW + 987, + content: 'how does the auth middleware work', + createdAt: NOW, + projectPath: '/Users/dev/example-project', + taskId: 'task-query-tm-1', + toolCalls: [], + type: 'query-tool-mode', + ...overrides, + }) as TaskInfo + +async function fakeReadFileForInspection(filePath: string): Promise { + if (filePath === '/Users/dev/example-project/.brv/notes/auth.md') { + return '---\nkeywords: ["jwt", "session"]\nrelated: ["auth/middleware", "users"]\ntags: ["security"]\n---\nbody\n' + } + + return '---\n---\nempty\n' +} + +const dumpEvents = (label: string, trackStub: sinon.SinonStub): void => { + console.log(`\n┌─ ${label} ${'─'.repeat(Math.max(0, 70 - label.length))}`) + for (const [i, call] of trackStub.getCalls().entries()) { + const eventName = call.args[0] as string + const props = call.args[1] as Record + console.log(`│ [${i}] ${eventName}`) + console.log(`│ ${JSON.stringify(props, null, 2).replaceAll('\n', '\n│ ')}`) + } + + console.log(`└${'─'.repeat(72)}\n`) +} + +describe('analytics-hook tool-mode event inspection (M14)', () => { + it('curate-tool-mode: prints every event + payload the daemon emits to analytics', async () => { + const {client, trackStub} = buildClient() + const hook = new AnalyticsHook() + hook.setAnalyticsClient(client) + + const task = buildToolModeCurateTask() + await hook.onTaskCreate(task) + await hook.onTaskCompleted(task.taskId, '', task) + + dumpEvents('curate-tool-mode — success path', trackStub) + + // Sanity: every event carries the canonical post-rename task_type + for (const call of trackStub.getCalls()) { + const props = call.args[1] as Record + expect(props.task_type, `${call.args[0] as string} task_type`).to.equal('curate-tool-mode') + } + + // Counters all-zero today because onToolResult never fires for + // tool-mode (no LLM tool calls) — that's the FU-1 follow-up. + const runCompleted = trackStub.getCalls().find((c) => c.args[0] === 'curate_run_completed') + const counters = runCompleted?.args[1] as Record + expect(counters.operations_added).to.equal(0) + expect(counters.operations_updated).to.equal(0) + expect(counters.operations_deleted).to.equal(0) + expect(counters.operations_merged).to.equal(0) + expect(counters.operations_failed).to.equal(0) + expect(counters.pending_review_count).to.equal(0) + }) + + it('curate-tool-mode: error path — prints curate_run_completed(outcome=error) + task_failed', async () => { + const {client, trackStub} = buildClient() + const hook = new AnalyticsHook() + hook.setAnalyticsClient(client) + + const task = buildToolModeCurateTask({taskId: 'task-curate-tm-err'}) + await hook.onTaskCreate(task) + await hook.onTaskError(task.taskId, 'writer rejected: path-exists', task) + + dumpEvents('curate-tool-mode — error path', trackStub) + }) + + it('curate-tool-mode: with a successful tool-result op (FU-1 forward-look — what counters WOULD look like)', async () => { + const {client, trackStub} = buildClient() + const hook = new AnalyticsHook() + hook.setAnalyticsClient(client) + + const task = buildToolModeCurateTask({taskId: 'task-curate-tm-fu1'}) + await hook.onTaskCreate(task) + + // Simulate a curate-op as if FU-1 had synthesised one from task.result + // (today's tool-mode path doesn't fire onToolResult — FU-1 fixes that). + const simulatedOp: LlmToolResultEvent = { + callId: 'sim-1', + result: JSON.stringify({ + applied: [ + { + filePath: '/Users/dev/example-project/.brv/notes/auth.md', + needsReview: false, + path: 'auth', + status: 'success', + type: 'ADD', + }, + ], + }), + sessionId: 'sim-session', + taskId: task.taskId, + timestamp: NOW, + toolName: 'curate' as const, + } as unknown as LlmToolResultEvent + await hook.onToolResult(task.taskId, simulatedOp) + await hook.onTaskCompleted(task.taskId, '', task) + + dumpEvents('curate-tool-mode — FU-1 forward-look (single synthetic op)', trackStub) + }) + + it('query-tool-mode: prints every event + payload the daemon emits to analytics', async () => { + const {client, trackStub} = buildClient() + const hook = new AnalyticsHook() + hook.setAnalyticsClient(client) + + const task = buildToolModeQueryTask() + await hook.onTaskCreate(task) + await hook.onTaskCompleted(task.taskId, '', task) + + dumpEvents('query-tool-mode — success path (no setQueryResult)', trackStub) + + for (const call of trackStub.getCalls()) { + const props = call.args[1] as Record + expect(props.task_type, `${call.args[0] as string} task_type`).to.equal('query-tool-mode') + } + + // No setQueryResult call → matched_doc_count + read_doc_count both 0, + // tier omitted, read_paths_with_metadata omitted. That's the + // empty-metadata state FU-1's query half closes. + const queryCompleted = trackStub.getCalls().find((c) => c.args[0] === 'query_completed') + const props = queryCompleted?.args[1] as Record + expect(props.matched_doc_count).to.equal(0) + expect(props.read_doc_count).to.equal(0) + expect(props.cache_hit).to.equal(false) + expect(props.tier).to.equal(undefined) + expect(props.read_paths_with_metadata).to.equal(undefined) + }) + + it('query-tool-mode: with setQueryResult (forward-look from FU-1) — populated metadata', async () => { + const {client, trackStub} = buildClient() + const hook = new AnalyticsHook() + hook.setAnalyticsClient(client) + + const task = buildToolModeQueryTask({taskId: 'task-query-tm-fu1'}) + await hook.onTaskCreate(task) + + const metadata: QueryResultMetadata = { + matchedDocs: [], + searchMetadata: {resultCount: 4, topScore: 0.82, totalFound: 4}, + tier: 2, + timing: {durationMs: 987}, + } as QueryResultMetadata + hook.setQueryResult(task.taskId, metadata) + + await hook.onTaskCompleted(task.taskId, '', task) + + dumpEvents('query-tool-mode — FU-1 forward-look (setQueryResult populated)', trackStub) + }) + + it('query (legacy): read_paths_with_metadata carries structured related_paths + relative_path + keywords/tags arrays', async () => { + const trackStub = sinon.stub() + const client: IAnalyticsClient = { + abort() { + /* noop */ + }, + flush: sinon.stub().resolves(AnalyticsBatch.create([])), + getRuntimeState: () => Promise.resolve({droppedCount: 0, lastSuccessfulFlushAt: undefined, queueDepth: 0}), + onAuthTransition: sinon.stub().resolves(), + track: trackStub, + } + const hook = new AnalyticsHook({readFile: fakeReadFileForInspection}) + hook.setAnalyticsClient(client) + + const task = buildToolModeQueryTask({ + taskId: 'task-query-paths-1', + toolCalls: [ + { + args: {filePath: '/Users/dev/example-project/.brv/notes/auth.md'}, + sessionId: 's1', + status: 'completed', + timestamp: NOW, + toolName: 'read_file', + }, + ], + type: 'query', + } as Partial) + + await hook.onTaskCreate(task) + await hook.onTaskCompleted(task.taskId, '', task) + + dumpEvents('query (legacy) — read_paths_with_metadata + related_paths structure', trackStub) + + const queryCompleted = trackStub.getCalls().find((c) => c.args[0] === 'query_completed') + const props = queryCompleted?.args[1] as Record + const paths = props.read_paths_with_metadata as Array> + expect(paths).to.have.lengthOf(1) + + const entry = paths[0] + expect(entry.relative_path).to.equal('.brv/notes/auth.md') + expect(entry.keywords).to.deep.equal(['jwt', 'session']) + expect(entry.tags).to.deep.equal(['security']) + expect(entry.related_paths).to.deep.equal([ + {keywords: [], relative_path: 'auth/middleware', tags: []}, + {keywords: [], relative_path: 'users', tags: []}, + ]) + }) + + it('query-tool-mode: error path', async () => { + const {client, trackStub} = buildClient() + const hook = new AnalyticsHook() + hook.setAnalyticsClient(client) + + const task = buildToolModeQueryTask({taskId: 'task-query-tm-err'}) + await hook.onTaskCreate(task) + await hook.onTaskError(task.taskId, 'connector unreachable', task) + + dumpEvents('query-tool-mode — error path', trackStub) + }) +}) diff --git a/test/unit/server/infra/process/analytics-hook.test.ts b/test/unit/server/infra/process/analytics-hook.test.ts index 7f448c5e1..48a566ecb 100644 --- a/test/unit/server/infra/process/analytics-hook.test.ts +++ b/test/unit/server/infra/process/analytics-hook.test.ts @@ -145,12 +145,13 @@ describe('AnalyticsHook', () => { expect(m12Calls()).to.have.lengthOf(2) expect(m12Calls()[0].args[0]).to.equal(AnalyticsEventNames.CURATE_OPERATION_APPLIED) const firstProps = m12Calls()[0].args[1] as Record - expect(firstProps.absolute_path).to.equal('/a.md') + // buildCurateTask sets projectPath:'/project'; path.relative('/project','/a.md') = '../a.md' + expect(firstProps.relative_path).to.equal('../a.md') expect(firstProps.knowledge_path).to.equal('notes/a') expect(firstProps.operation_type).to.equal('ADD') expect(firstProps.needs_review).to.equal(false) - expect(firstProps).to.not.have.property('tags') - expect(firstProps).to.not.have.property('keywords') + expect(firstProps.tags).to.deep.equal([]) + expect(firstProps.keywords).to.deep.equal([]) expect(firstProps).to.not.have.property('related') const secondProps = m12Calls()[1].args[1] as Record @@ -331,13 +332,14 @@ describe('AnalyticsHook', () => { expect(props.matched_doc_count).to.equal(7) const paths = props.read_paths_with_metadata as Array> expect(paths).to.have.lengthOf(3) - // sorted lexicographically - expect(paths.map((p) => p.absolute_path)).to.deep.equal(['/a.md', '/b.md', '/c.md']) - // each entry has only absolute_path, no metadata in M12.2 + // sorted lexicographically; relativized against projectPath:'/project' + expect(paths.map((p) => p.relative_path)).to.deep.equal(['../a.md', '../b.md', '../c.md']) + // each entry has empty keywords/tags arrays and an empty related_paths + // list — no frontmatter source files exist in this in-memory test. for (const entry of paths) { - expect(entry).to.not.have.property('tags') - expect(entry).to.not.have.property('keywords') - expect(entry).to.not.have.property('related') + expect(entry.tags).to.deep.equal([]) + expect(entry.keywords).to.deep.equal([]) + expect(entry.related_paths).to.deep.equal([]) } }) @@ -522,7 +524,7 @@ describe('AnalyticsHook', () => { expect(props.related).to.deep.equal(['z']) }) - it('omits tags/keywords/related on DELETE ops (file gone post-op)', async () => { + it('keywords/tags default to empty arrays on DELETE ops (file gone post-op); related stays omitted', async () => { const filePath = join(tmpDir, 'gone.md') const task = buildCurateTask() await hook.onTaskCreate(task) @@ -532,12 +534,12 @@ describe('AnalyticsHook', () => { ) const props = m12Calls()[0].args[1] as Record - expect(props).to.not.have.property('tags') - expect(props).to.not.have.property('keywords') + expect(props.tags).to.deep.equal([]) + expect(props.keywords).to.deep.equal([]) expect(props).to.not.have.property('related') }) - it('omits tags/keywords/related when filePath cannot be read (ENOENT)', async () => { + it('keywords/tags default to empty arrays when filePath cannot be read (ENOENT)', async () => { const filePath = join(tmpDir, 'missing.md') const task = buildCurateTask() await hook.onTaskCreate(task) @@ -547,10 +549,11 @@ describe('AnalyticsHook', () => { ) const props = m12Calls()[0].args[1] as Record - expect(props).to.not.have.property('tags') + expect(props.tags).to.deep.equal([]) + expect(props.keywords).to.deep.equal([]) }) - it('omits tags/keywords/related on malformed YAML (no throw)', async () => { + it('keywords/tags default to empty arrays on malformed YAML (no throw)', async () => { const filePath = join(tmpDir, 'bad.md') writeFileSync(filePath, '---\nthis is: not [valid YAML\n---\nbody', 'utf8') @@ -562,7 +565,8 @@ describe('AnalyticsHook', () => { ) const props = m12Calls()[0].args[1] as Record - expect(props).to.not.have.property('tags') + expect(props.tags).to.deep.equal([]) + expect(props.keywords).to.deep.equal([]) }) it('caps arrays at 50 entries and strings at 256 chars per entry', async () => { @@ -584,7 +588,7 @@ describe('AnalyticsHook', () => { expect(tags[0]).to.have.lengthOf(256) }) - it('skips file reads entirely when isEnabled() returns false', async () => { + it('skips file reads entirely when isEnabled() returns false; keywords/tags fall back to []', async () => { const filePath = join(tmpDir, 'gated.md') writeMarkdown(filePath, {tags: ['should-not-appear']}) @@ -600,7 +604,8 @@ describe('AnalyticsHook', () => { ) const props = filterM12(disabledBundle.trackStub)[0].args[1] as Record - expect(props).to.not.have.property('tags') + expect(props.tags).to.deep.equal([]) + expect(props.keywords).to.deep.equal([]) }) }) @@ -611,7 +616,9 @@ describe('AnalyticsHook', () => { writeMarkdown(a, {tags: ['ta']}) writeMarkdown(b, {keywords: ['kb']}) + // Pin projectPath to tmpDir so relative_path == 'a.md' / 'b.md'. const task = buildQueryTask({ + projectPath: tmpDir, toolCalls: [ {args: {filePath: a}, sessionId: 's', status: 'completed', timestamp: 1, toolName: 'read_file'}, {args: {filePath: b}, sessionId: 's', status: 'completed', timestamp: 2, toolName: 'read_file'}, @@ -623,19 +630,20 @@ describe('AnalyticsHook', () => { const props = m12Calls()[0].args[1] as Record const paths = props.read_paths_with_metadata as Array> - const byPath = Object.fromEntries(paths.map((p) => [p.absolute_path, p])) - expect(byPath[a].tags).to.deep.equal(['ta']) - expect(byPath[a]).to.not.have.property('keywords') - expect(byPath[b].keywords).to.deep.equal(['kb']) - expect(byPath[b]).to.not.have.property('tags') + const byPath = Object.fromEntries(paths.map((p) => [p.relative_path, p])) + expect(byPath['a.md'].tags).to.deep.equal(['ta']) + expect(byPath['a.md'].keywords).to.deep.equal([]) + expect(byPath['b.md'].keywords).to.deep.equal(['kb']) + expect(byPath['b.md'].tags).to.deep.equal([]) }) - it('mixed readable + ENOENT paths: each entry independently has/omits metadata', async () => { + it('mixed readable + ENOENT paths: each entry has keywords/tags arrays (populated or empty)', async () => { const real = join(tmpDir, 'real.md') const missing = join(tmpDir, 'missing.md') writeMarkdown(real, {tags: ['ok']}) const task = buildQueryTask({ + projectPath: tmpDir, toolCalls: [ {args: {filePath: real}, sessionId: 's', status: 'completed', timestamp: 1, toolName: 'read_file'}, {args: {filePath: missing}, sessionId: 's', status: 'completed', timestamp: 2, toolName: 'read_file'}, @@ -647,9 +655,10 @@ describe('AnalyticsHook', () => { const props = m12Calls()[0].args[1] as Record const paths = props.read_paths_with_metadata as Array> - const byPath = Object.fromEntries(paths.map((p) => [p.absolute_path, p])) - expect(byPath[real].tags).to.deep.equal(['ok']) - expect(byPath[missing]).to.not.have.property('tags') + const byPath = Object.fromEntries(paths.map((p) => [p.relative_path, p])) + expect(byPath['real.md'].tags).to.deep.equal(['ok']) + expect(byPath['missing.md'].tags).to.deep.equal([]) + expect(byPath['missing.md'].keywords).to.deep.equal([]) }) it('skips per-path file reads when isEnabled() returns false', async () => { @@ -672,7 +681,8 @@ describe('AnalyticsHook', () => { const props = filterM12(disabledBundle.trackStub)[0].args[1] as Record const paths = props.read_paths_with_metadata as Array> - expect(paths[0]).to.not.have.property('tags') + expect(paths[0].tags).to.deep.equal([]) + expect(paths[0].keywords).to.deep.equal([]) }) }) }) @@ -712,8 +722,9 @@ describe('AnalyticsHook', () => { expect(filterM12(bundle.trackStub)).to.have.lengthOf(2) const first = filterM12(bundle.trackStub)[0].args[1] as Record const second = filterM12(bundle.trackStub)[1].args[1] as Record - expect(first.absolute_path, 'first emit must be op1').to.equal('/op1.md') - expect(second.absolute_path, 'second emit must be op2').to.equal('/op2.md') + // buildCurateTask projectPath:'/project'; absolute paths relativize with '../' prefix + expect(first.relative_path, 'first emit must be op1').to.equal('../op1.md') + expect(second.relative_path, 'second emit must be op2').to.equal('../op2.md') }) it('onTaskCompleted waits for in-flight onToolResult work before emitting CURATE_RUN_COMPLETED', async () => { @@ -765,9 +776,10 @@ describe('AnalyticsHook', () => { expect(filterM12(bundle.trackStub)).to.have.lengthOf(1) const props = filterM12(bundle.trackStub)[0].args[1] as Record - expect(props.absolute_path).to.equal('/missing.md') - expect(props).to.not.have.property('keywords') - expect(props).to.not.have.property('tags') + // buildCurateTask sets projectPath:'/project'; '/missing.md' relativizes to '../missing.md' + expect(props.relative_path).to.equal('../missing.md') + expect(props.keywords).to.deep.equal([]) + expect(props.tags).to.deep.equal([]) expect(props).to.not.have.property('related') }) diff --git a/test/unit/server/infra/transport/handlers/analytics-handler.test.ts b/test/unit/server/infra/transport/handlers/analytics-handler.test.ts index e469d22eb..ca2461497 100644 --- a/test/unit/server/infra/transport/handlers/analytics-handler.test.ts +++ b/test/unit/server/infra/transport/handlers/analytics-handler.test.ts @@ -64,10 +64,12 @@ describe('AnalyticsHandler', () => { const payload: AnalyticsTrackPayload = { event: AnalyticsEventNames.CURATE_OPERATION_APPLIED, properties: { - absolute_path: '/tmp/a.md', + keywords: [], knowledge_path: 'kg/a.md', needs_review: false, operation_type: 'ADD', + relative_path: 'tmp/a.md', + tags: [], task_id: 't-1', }, } @@ -76,10 +78,12 @@ describe('AnalyticsHandler', () => { expect(analyticsClient.trackCalls).to.have.lengthOf(1) expect(analyticsClient.trackCalls[0].event).to.equal(AnalyticsEventNames.CURATE_OPERATION_APPLIED) expect(analyticsClient.trackCalls[0].properties).to.deep.equal({ - absolute_path: '/tmp/a.md', + keywords: [], knowledge_path: 'kg/a.md', needs_review: false, operation_type: 'ADD', + relative_path: 'tmp/a.md', + tags: [], task_id: 't-1', }) }) @@ -116,7 +120,7 @@ describe('AnalyticsHandler', () => { new AnalyticsHandler({analyticsClient, transport}).setup() const handler = transport._handlers.get(AnalyticsEvents.TRACK) as AnalyticsTrackHandler - // CURATE_OPERATION_APPLIED requires absolute_path / knowledge_path / etc. + // CURATE_OPERATION_APPLIED requires relative_path / knowledge_path / etc. await handler({event: AnalyticsEventNames.CURATE_OPERATION_APPLIED, properties: {wrong: 'shape'}}, 'client-1') // QUERY_COMPLETED requires duration_ms / outcome / etc. await handler({event: AnalyticsEventNames.QUERY_COMPLETED, properties: {}}, 'client-1') diff --git a/test/unit/shared/analytics/events/curate-operation-applied.test.ts b/test/unit/shared/analytics/events/curate-operation-applied.test.ts index b273200d4..9d2915ef1 100644 --- a/test/unit/shared/analytics/events/curate-operation-applied.test.ts +++ b/test/unit/shared/analytics/events/curate-operation-applied.test.ts @@ -4,10 +4,12 @@ import {expect} from 'chai' import {CurateOperationAppliedSchema} from '../../../../../src/shared/analytics/events/curate-operation-applied.js' const baseValid = { - absolute_path: '/Users/dev/project/.brv/context-tree/notes/test.md', + keywords: [], knowledge_path: 'notes/test', needs_review: false, operation_type: 'ADD' as const, + relative_path: '.brv/context-tree/notes/test.md', + tags: [], task_id: 'task-uuid-123', } @@ -33,7 +35,9 @@ describe('CurateOperationAppliedSchema', () => { expect(CurateOperationAppliedSchema.safeParse({...baseValid, needs_review: true}).success).to.equal(true) }) - it('accepts payloads omitting any/all of tags, keywords, related', () => { + it('accepts payloads with keywords / tags as required arrays (default empty) and optional related', () => { + // keywords/tags are required after the M14 review tightening; the + // base payload already carries them as []. expect(CurateOperationAppliedSchema.safeParse({...baseValid}).success).to.equal(true) expect(CurateOperationAppliedSchema.safeParse({...baseValid, tags: ['a']}).success).to.equal(true) expect(CurateOperationAppliedSchema.safeParse({...baseValid, keywords: ['k']}).success).to.equal(true) @@ -43,6 +47,15 @@ describe('CurateOperationAppliedSchema', () => { ).to.equal(true) }) + it('rejects payloads missing the required keywords / tags arrays', () => { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const {keywords: _k, ...withoutKeywords} = baseValid + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const {tags: _t, ...withoutTags} = baseValid + expect(CurateOperationAppliedSchema.safeParse(withoutKeywords).success).to.equal(false) + expect(CurateOperationAppliedSchema.safeParse(withoutTags).success).to.equal(false) + }) + it('accepts tags / keywords / related with exactly 50 entries each', () => { const fifty = Array.from({length: 50}, (_, i) => `entry-${i}`) expect( @@ -75,8 +88,8 @@ describe('CurateOperationAppliedSchema', () => { expect(CurateOperationAppliedSchema.safeParse({...baseValid, confidence: 'maybe'}).success).to.equal(false) }) - it('rejects empty absolute_path / knowledge_path / task_id', () => { - expect(CurateOperationAppliedSchema.safeParse({...baseValid, absolute_path: ''}).success).to.equal(false) + it('rejects empty relative_path / knowledge_path / task_id', () => { + expect(CurateOperationAppliedSchema.safeParse({...baseValid, relative_path: ''}).success).to.equal(false) expect(CurateOperationAppliedSchema.safeParse({...baseValid, knowledge_path: ''}).success).to.equal(false) expect(CurateOperationAppliedSchema.safeParse({...baseValid, task_id: ''}).success).to.equal(false) }) diff --git a/test/unit/shared/analytics/events/query-completed.test.ts b/test/unit/shared/analytics/events/query-completed.test.ts index e80d8039f..197df52ef 100644 --- a/test/unit/shared/analytics/events/query-completed.test.ts +++ b/test/unit/shared/analytics/events/query-completed.test.ts @@ -16,6 +16,13 @@ const baseValid = { task_type: 'query' as const, } +const baseEntry = { + keywords: [], + related_paths: [], + relative_path: '.brv/notes/a.md', + tags: [], +} + describe('QueryCompletedSchema', () => { describe('valid payloads', () => { it('accepts the minimal required payload with empty read_paths_with_metadata', () => { @@ -48,25 +55,52 @@ describe('QueryCompletedSchema', () => { expect(QueryCompletedSchema.safeParse({...baseValid, cache_hit: true}).success).to.equal(true) }) - it('accepts read_paths_with_metadata entries with no metadata', () => { - const entries = [{absolute_path: '/a.md'}, {absolute_path: '/b.md'}] + it('accepts read_paths_with_metadata entries with empty metadata arrays', () => { + const entries = [ + {...baseEntry, relative_path: '.brv/a.md'}, + {...baseEntry, relative_path: '.brv/b.md'}, + ] expect(QueryCompletedSchema.safeParse({...baseValid, read_paths_with_metadata: entries}).success).to.equal(true) }) - it('accepts entries with full optional metadata', () => { - const entries = [{absolute_path: '/a.md', keywords: ['k1'], related: ['r1'], tags: ['t1']}] + it('accepts entries with populated keywords, tags, and structured related_paths', () => { + const entries = [ + { + keywords: ['k1'], + related_paths: [{keywords: [], relative_path: 'r1', tags: []}], + relative_path: '.brv/a.md', + tags: ['t1'], + }, + ] expect(QueryCompletedSchema.safeParse({...baseValid, read_paths_with_metadata: entries}).success).to.equal(true) }) it('accepts read_paths_with_metadata with exactly 10 entries', () => { - const entries = Array.from({length: 10}, (_, i) => ({absolute_path: `/file-${i}.md`})) + const entries = Array.from({length: 10}, (_, i) => ({...baseEntry, relative_path: `.brv/file-${i}.md`})) expect(QueryCompletedSchema.safeParse({...baseValid, read_paths_with_metadata: entries}).success).to.equal(true) }) - it('accepts entries with tags / keywords / related at the 50-entry cap and 256-char cap', () => { + it('accepts entries with keywords / tags at the 50-entry cap and 256-char strings', () => { const fifty = Array.from({length: 50}, (_, i) => `entry-${i}`) const at256 = 'x'.repeat(256) - const entries = [{absolute_path: '/a.md', keywords: fifty, related: [at256], tags: fifty}] + const entries = [ + { + keywords: fifty, + related_paths: [{keywords: [], relative_path: at256, tags: []}], + relative_path: '.brv/a.md', + tags: fifty, + }, + ] + expect(QueryCompletedSchema.safeParse({...baseValid, read_paths_with_metadata: entries}).success).to.equal(true) + }) + + it('accepts related_paths with up to 50 structured entries', () => { + const fifty = Array.from({length: 50}, (_, i) => ({ + keywords: [], + relative_path: `notes/related-${i}`, + tags: [], + })) + const entries = [{...baseEntry, related_paths: fifty}] expect(QueryCompletedSchema.safeParse({...baseValid, read_paths_with_metadata: entries}).success).to.equal(true) }) }) @@ -105,26 +139,42 @@ describe('QueryCompletedSchema', () => { }) it('rejects read_paths_with_metadata with more than 10 entries', () => { - const entries = Array.from({length: 11}, (_, i) => ({absolute_path: `/file-${i}.md`})) + const entries = Array.from({length: 11}, (_, i) => ({...baseEntry, relative_path: `.brv/file-${i}.md`})) expect(QueryCompletedSchema.safeParse({...baseValid, read_paths_with_metadata: entries}).success).to.equal(false) }) - it('rejects entries with empty absolute_path', () => { - const entries = [{absolute_path: ''}] + it('rejects entries with empty relative_path', () => { + const entries = [{...baseEntry, relative_path: ''}] expect(QueryCompletedSchema.safeParse({...baseValid, read_paths_with_metadata: entries}).success).to.equal(false) }) - it('rejects entries with more than 50 tags / keywords / related', () => { - const fiftyOne = Array.from({length: 51}, (_, i) => `entry-${i}`) - const tagsEntry = [{absolute_path: '/a.md', tags: fiftyOne}] - expect(QueryCompletedSchema.safeParse({...baseValid, read_paths_with_metadata: tagsEntry}).success).to.equal( + it('rejects entries missing required keywords / tags arrays', () => { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const {keywords: _k, ...withoutKeywords} = baseEntry + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const {tags: _t, ...withoutTags} = baseEntry + expect( + QueryCompletedSchema.safeParse({...baseValid, read_paths_with_metadata: [withoutKeywords]}).success, + ).to.equal(false) + expect(QueryCompletedSchema.safeParse({...baseValid, read_paths_with_metadata: [withoutTags]}).success).to.equal( false, ) }) - it('rejects entries with tag / keyword / related string longer than 256 chars', () => { + it('rejects entries with more than 50 tags / keywords', () => { + const fiftyOne = Array.from({length: 51}, (_, i) => `entry-${i}`) + const tagsEntry = [{...baseEntry, tags: fiftyOne}] + expect(QueryCompletedSchema.safeParse({...baseValid, read_paths_with_metadata: tagsEntry}).success).to.equal(false) + }) + + it('rejects entries with tag / keyword string longer than 256 chars', () => { const at257 = 'x'.repeat(257) - const entries = [{absolute_path: '/a.md', keywords: [at257]}] + const entries = [{...baseEntry, keywords: [at257]}] + expect(QueryCompletedSchema.safeParse({...baseValid, read_paths_with_metadata: entries}).success).to.equal(false) + }) + + it('rejects related_paths entries missing keywords / tags / relative_path', () => { + const entries = [{...baseEntry, related_paths: [{relative_path: 'r1'}]}] expect(QueryCompletedSchema.safeParse({...baseValid, read_paths_with_metadata: entries}).success).to.equal(false) }) @@ -133,7 +183,7 @@ describe('QueryCompletedSchema', () => { }) it('rejects unknown extra fields inside an entry (strict)', () => { - const entries = [{absolute_path: '/a.md', mystery: 'oops'}] + const entries = [{...baseEntry, mystery: 'oops'}] expect(QueryCompletedSchema.safeParse({...baseValid, read_paths_with_metadata: entries}).success).to.equal(false) }) })