From 53401530d8531c49e6e9f69321bed25ce0e20804 Mon Sep 17 00:00:00 2001 From: wxr <51250936+wxrbyte@users.noreply.github.com> Date: Tue, 29 Sep 2026 09:41:45 +0800 Subject: [PATCH] =?UTF-8?q?fix(eval):=20=E5=8F=AF=E8=A7=82=E6=B5=8B?= =?UTF-8?q?=E6=80=A7=E9=93=BE=E8=B7=AF=E6=8A=8A=E7=94=A8=E6=88=B7=E5=8F=96?= =?UTF-8?q?=E6=B6=88=E7=9A=84=E8=BF=90=E8=A1=8C=E8=AE=B0=E6=88=90=20cancel?= =?UTF-8?q?led=EF=BC=8C=E4=B8=8D=E5=86=8D=E8=AE=B0=E6=88=90=E5=A4=B1?= =?UTF-8?q?=E8=B4=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit kernelHost.observeRunEvent() 把 run_completed / run_failed 先压成一个布尔量， settleEvaluationRun() 便只能落 completed / failed 两种状态。而用户主动取消恰恰 是以 run_failed + error.code = 'RUN_CANCELLED' 落地的（RunManager 的取消分支， stream-event-adapter 再把它归一成流事件的 cancelled），于是「取消」在这条可观测性 链路上被记成了「任务失败」。 后果在统计口径上可见：packages/shared/src/evaluation/aggregate.ts 的 isSkippedRun() 以 status === 'cancelled' 判定跳过，记成 failed 之后 isQualityRun() 反而为 true ⇒ 取消的运行被计入 evaluated、skipped 恒为 0， verdictForRun() 还会把该 case 判成 'fail'。core 的 ExperimentExecutionCounts 对 skipped 的注释写得很清楚：「Cases never run: cancelled mid-flight or skipped after an abort.」 同层的另一个生产者 experiment-service 早已按 RUN_CANCELLED 落 'cancelled' （:911），本改动让两个生产者对 EvaluationRunStatus 的口径一致。 修法：observeRunEvent() 直接推导 EvaluationRunStatus；settleEvaluationRun() 的形参 由 completed: boolean 换成 status: EvaluationRunStatus；EvaluationRun 字面量本就是 status 简写无需改动，exportAgentTrace({ completed }) 改写为 completed: status === 'completed'，取值与修复前等价，trace 侧行为不变。预算/护栏终止 （BUDGET_EXHAUSTED 等）仍记 failed，未被一并归入 cancelled。 Closes #254 --- apps/electron/src/main/kernelHost.test.ts | 97 ++++++++++++++++++++++- apps/electron/src/main/kernelHost.ts | 23 ++++-- 2 files changed, 114 insertions(+), 6 deletions(-) diff --git a/apps/electron/src/main/kernelHost.test.ts b/apps/electron/src/main/kernelHost.test.ts index 0a5684e..925249b 100644 --- a/apps/electron/src/main/kernelHost.test.ts +++ b/apps/electron/src/main/kernelHost.test.ts @@ -6,8 +6,19 @@ let lastKernelOptions: Record | null = null; let lastMarketData: FakeMarketDataService | null = null; let lastAutomationContext: unknown = null; let forwardedEvents: unknown[] = []; +/** Evaluation runs handed to EvaluationStore.addRun (observability path). */ +const savedEvalRuns: Array> = []; const routerFetchers = { getQuote: async () => ({ symbol: 'AAPL.US' }) }; +/** Poll until `predicate` holds; the settle path is fire-and-forget (`void`). */ +async function waitFor(predicate: () => boolean, timeoutMs = 1000): Promise { + const deadline = Date.now() + timeoutMs; + while (!predicate()) { + if (Date.now() > deadline) throw new Error('waitFor timed out'); + await new Promise((resolve) => setTimeout(resolve, 5)); + } +} + class FakeMarketDataService { quoteSymbols: string[] = []; constructor(readonly options?: { fetchers?: unknown }) {} @@ -54,6 +65,7 @@ const fakeSessions = { deleteSession: async () => undefined, listMessages: async (sessionId: string) => [{ id: 'm1', role: 'user', content: sessionId, timestamp: 1 }], listRuns: async () => [], + getSession: async () => undefined, }; const fakeRuns = { @@ -288,7 +300,9 @@ mock.module('@finagent/shared', () => ({ }); saveSettings = async (settings: unknown) => settings; getSettings = async () => ({}); - addRun = async () => undefined; + addRun = async (run: Record) => { + savedEvalRuns.push(run); + }; listExperiments = async () => []; getExperiment = async () => undefined; listRuns = async () => []; @@ -352,6 +366,7 @@ beforeEach(() => { lastMarketData = null; lastAutomationContext = null; forwardedEvents = []; + savedEvalRuns.length = 0; }); afterEach(() => { @@ -521,4 +536,84 @@ describe('AgentKernelHost', () => { }); host.dispose(); }); + + it('records a user-cancelled run as cancelled, not failed', async () => { + const originalSubscribe = fakeRuns.subscribe; + let subscriber: ((event: AgentEvent) => void) | null = null; + fakeRuns.subscribe = (listener) => { + subscriber = listener; + return () => undefined; + }; + const host = new AgentKernelHost(); + const emit = subscriber as ((event: AgentEvent) => void) | null; + + emit?.({ + id: 'e1', + sessionId: 's1', + runId: 'r1', + type: 'run_started', + timestamp: 10, + sequence: 1, + payload: { + run: { id: 'r1', sessionId: 's1', status: 'running', input: 'x', startedAt: 10 }, + userMessage: { id: 'm1', role: 'user', content: 'x', timestamp: 10 }, + }, + }); + // A user cancel settles the run as run_failed/RUN_CANCELLED (RunManager + // normalizes it to `cancelled` on the stream channel). + emit?.({ + id: 'e2', + sessionId: 's1', + runId: 'r1', + type: 'run_failed', + timestamp: 20, + sequence: 2, + payload: { error: { code: 'RUN_CANCELLED', message: 'Run cancelled by user.' } }, + }); + + await waitFor(() => savedEvalRuns.length === 1); + expect(savedEvalRuns[0]).toMatchObject({ id: 'r1', status: 'cancelled' }); + + fakeRuns.subscribe = originalSubscribe; + host.dispose(); + }); + + it('keeps a genuine runtime failure recorded as failed', async () => { + const originalSubscribe = fakeRuns.subscribe; + let subscriber: ((event: AgentEvent) => void) | null = null; + fakeRuns.subscribe = (listener) => { + subscriber = listener; + return () => undefined; + }; + const host = new AgentKernelHost(); + const emit = subscriber as ((event: AgentEvent) => void) | null; + + emit?.({ + id: 'e1', + sessionId: 's1', + runId: 'r2', + type: 'run_started', + timestamp: 10, + sequence: 1, + payload: { + run: { id: 'r2', sessionId: 's1', status: 'running', input: 'x', startedAt: 10 }, + userMessage: { id: 'm1', role: 'user', content: 'x', timestamp: 10 }, + }, + }); + emit?.({ + id: 'e2', + sessionId: 's1', + runId: 'r2', + type: 'run_failed', + timestamp: 20, + sequence: 2, + payload: { error: { code: 'PROVIDER_ERROR', message: 'Upstream failed.' } }, + }); + + await waitFor(() => savedEvalRuns.length === 1); + expect(savedEvalRuns[0]).toMatchObject({ id: 'r2', status: 'failed' }); + + fakeRuns.subscribe = originalSubscribe; + host.dispose(); + }); }); diff --git a/apps/electron/src/main/kernelHost.ts b/apps/electron/src/main/kernelHost.ts index 63bea45..a84e6ab 100644 --- a/apps/electron/src/main/kernelHost.ts +++ b/apps/electron/src/main/kernelHost.ts @@ -237,6 +237,15 @@ interface StartRunRequest { /** Experiment id for runs observed outside explicit evaluation experiments. */ const OBSERVABILITY_EXPERIMENT_ID = '__observability__'; +/** + * `RunManager` reports a user cancel as `run_failed` carrying this code and + * normalizes it to `cancelled` on the stream channel. The observability store + * has to read the same code, otherwise a deliberate cancel is recorded as a + * task failure and lands in the evaluated-quality bucket instead of `skipped` + * (`isSkippedRun` keys off `status === 'cancelled'`). + */ +const RUN_CANCELLED_CODE = 'RUN_CANCELLED'; + interface PendingEvalRun { sessionId: string; startedAt: number; @@ -1326,21 +1335,25 @@ export class AgentKernelHost { pending.answer = event.payload.answer; } else if (event.type === 'run_completed' || event.type === 'run_failed') { if (event.type === 'run_failed') pending.error = event.payload.error; - const completed = event.type === 'run_completed'; - void this.settleEvaluationRun(event.runId, event.sessionId, completed, event.timestamp); + const status: EvaluationRunStatus = + event.type === 'run_completed' + ? 'completed' + : event.payload.error?.code === RUN_CANCELLED_CODE + ? 'cancelled' + : 'failed'; + void this.settleEvaluationRun(event.runId, event.sessionId, status, event.timestamp); } } private async settleEvaluationRun( runId: string, sessionId: string, - completed: boolean, + status: EvaluationRunStatus, endedAt: number ): Promise { const pending = this.evalRuns.get(runId); this.evalRuns.delete(runId); if (!pending) return; - const status: EvaluationRunStatus = completed ? 'completed' : 'failed'; const toolCalls: ToolCallRecord[] = pending.toolCalls.map((toolCall) => ({ id: toolCall.id, toolName: toolCall.toolName, @@ -1387,7 +1400,7 @@ export class AgentKernelHost { answer: run.answer, toolCalls: run.toolCalls, error: pending.error, - completed, + completed: status === 'completed', }); if (ref) { await this.persistTraceLink(runId, ref);