From c8d9ec64c3191b81da7d4119354c1c5e281f139e Mon Sep 17 00:00:00 2001 From: Jesse_Chen Date: Mon, 17 Aug 2026 15:14:36 +0800 Subject: [PATCH] fix(consult): stop the model spending its step budget on invalid tool params A staging consultation calculated the chart and then returned nothing but the ensureFinalResponseText fallback. The model had made four calls to run-jyotish-consultation, and two of them never reached a calculation: they set both domains and theme, which canonicalDomainPlan rejects at execution. The schema declared those two fields as independent optionals, the description never mentioned the constraint, and the instructions actively told the model to use theme for a single-domain retry. Each attempt therefore bought a rule the contract never stated, and because the throw happens before the step-recording try/catch, it left no trace in the receipt either. Make the constraint unrepresentable instead of enforced. The model-facing schema keeps only question and domains, so Mastra refuses the pair before the tool body runs; the description states the single-array contract, and the instruction that advertised theme is gone. canonicalDomainPlan still resolves the single-value form for callers that build a plan without that schema, and is now exported so that path has its own tests. maxSteps and the abort timeout bound the same run but were hard-coded apart. One calculation takes about 20s against a 110s budget, so time is the binding constraint and three failed calculations exhaust it whatever the step count. The budget only has to cover the longest useful shape, so it moves to 8 beside the timeout with that reasoning recorded, and the recorded step list is sized to match so an exhausted run cannot truncate its own evidence. Step exhaustion was only ever inferable by counting events, since finishReason was recorded nowhere and progressive-disclosure reads never reach the public stream. Capture it as a closed enum plus a step count, normalizing anything unrecognized, and log both as controlled fields. Neither may enter the client receipt, whose step schema is strict and would fail a successful run. Co-authored-by: Cursor --- docs/BUG_HISTORY.md | 15 ++ frontend/src/app/api/consult/route.ts | 21 +- frontend/src/lib/agent-observability.ts | 35 ++++ frontend/src/lib/stream-agent-response.ts | 29 +++ frontend/src/mastra/consultation-tools.ts | 34 +++- frontend/src/mastra/index.ts | 2 +- frontend/tests/agent-observability.test.ts | 39 ++++ .../consultation-agentic-runtime.test.ts | 192 ++++++++++++++++-- .../consultation-stream-recovery.test.ts | 4 +- .../consultation-workflow-contract.test.ts | 9 + 10 files changed, 355 insertions(+), 25 deletions(-) diff --git a/docs/BUG_HISTORY.md b/docs/BUG_HISTORY.md index d6e91cb7..4b54f783 100644 --- a/docs/BUG_HISTORY.md +++ b/docs/BUG_HISTORY.md @@ -3757,3 +3757,18 @@ - 待跟进:`BUG-163` 的 `状态`/`最近更新`/`根因`/`验证`/`修复版本` 各出现两次,疑为同一记录两轮更新直接拼接。该缺陷早于本轮、不属正文错位,未处理。 - 相关记录:BUG-253 - 修复版本:本地未提交候选 +## BUG-255 | 模型把步数预算耗在无效工具参数上,个人咨询只返回兜底文案 + +- 状态:resolved(本地修复,未提交、未发布) +- 首次发现:2026-08-17 +- 最近更新:2026-08-17 +- 影响面:`/api/consult` 个人咨询的 Agent 步数预算、`run-jyotish-consultation` 的模型可见参数契约,以及模型运行结束原因的可观测性。 +- 用户现象:staging 事业类咨询运行至星盘计算成功,但模型没有产出任何回答文本,用户只看到 `ensureFinalResponseText()` 的兜底句“本次计算已完成,但暂时没有生成可展示的回答”。事件流显示 Skill 已加载、四次 `run-jyotish-consultation`(三次失败、一次在 20278ms 后成功),此后再无 `answer.delta`。 +- 触发条件:模型在同一次运行内多次调用排盘工具,其中至少两次同时传入 `domains` 与兼容字段 `theme`;叠加渐进式披露的 Skill 参考读取后,`maxSteps: 6` 在写回答之前耗尽。 +- 根因:三层叠加。其一,`consultationToolInputSchema` 把 `domains` 与 `theme` 声明为两个彼此独立的可选字段,互斥关系只在 `canonicalDomainPlan()` 里以 `invalid_consultation_domain_plan` 运行期抛出,工具 `description` 也从未提到该约束;更直接的是 `jyotishInstructions` 明确写着“Use the legacy theme field only for a single-domain compatibility retry”,等于主动引导模型去用一个会被拒绝的组合。其二,这类无效调用发生在步骤记录 try/catch 之前,既不产生 `chart-calculation` 活动也不追加运行步骤,因此每次都白耗一个模型步骤且在回执里不留痕迹。其三,`maxSteps: 6` 与 `AbortSignal.timeout(110_000)` 约束同一次运行却分别硬编码:1 次 Skill 加载 + 4 次工具调用已占 5 步,`skill_read` / `skill_search` 这类渐进式披露工具未映射进公开事件流,第 6 步一旦被一次不可见的参考读取拿走,运行就在没有任何回答的情况下结束。`finishReason` 在整个仓库中没有任何记录点,因此“步数耗尽”只能靠事后数事件推断,无法证实。 +- 修复:把互斥关系改为不可表达而非运行期拒绝——模型可见的 `inputSchema` 只保留 `question` 与 `domains`,`theme` 从模型契约中移除,`.strict()` 保持不变,使 `theme` 在进入工具体之前即被 Mastra 的入参校验拒绝;`description` 补齐“只用一个有序 `domains` 数组,省略即接受服务端已选领域,出生资料服务端绑定”的显式契约;`jyotishInstructions` 同步删除引导模型使用 `theme` 的那句。`canonicalDomainPlan()` 继续处理单值 `theme` 形态并保留 `invalid_consultation_domain_plan`,导出后由直接单元测试覆盖,供不经模型 schema 构造计划的调用方使用。步数预算与时钟预算改为相邻声明的 `AGENT_MAX_STEPS = 8` 与 `AGENT_TIMEOUT_MS = 110_000`,并注明二者约束同一次运行、必须一起考虑;运行步骤记录预算随之对齐,避免耗尽步数的运行同时截断自身证据。新增受控观测字段 `modelFinishReason`(封闭枚举,未知取值一律归一为 `unknown`)与 `modelStepCount`,在流中按 `step-finish` 计数、以终止 `finish` 携带的步骤列表为准,跨重试累计。未放宽单次计算边界,未放宽运行合同门禁,未把任何模型原文或 provider payload 写入日志。 +- 验证:新增修复前失败的回归 6 项——模型可见 schema 必须拒绝 `theme`(单独出现与与 `domains` 同时出现)、被拒调用不得推进任何运行状态(`consultationToolStarted` / `consultationToolCallCount` / `steps` 全部不变)、完成运行必须记录 `finishReason` 与权威步数、以 `tool-calls` 结束且无回答的运行必须记下耗尽的步数、重试必须累计步数并归一化未识别的 provider 取值、步数与时钟预算必须成对声明。回退任一源改动可确认对应回归失败。另新增公开回执守卫:`agentExecutionReceiptSchema` 是 strict,`modelFinishReason` / `modelStepCount` 一旦透出会让成功运行在序列化自身回答时报错,故断言按白名单构建的回执不含这两个字段、直接透出则必须抛错。`canonicalDomainPlan()` 补 8 项直接断言,锁定“单值 theme 不得覆盖路由选定领域”。BUG-205 的缓存不被污染性质改由 schema 合法但注册表非法的输入(`domains: ["career", "unknown"]`)复验,因其原始触发条件已不可达。全量非数据库套件 1586/1586 通过,`npx tsc --noEmit` 0 错误,改动文件 `npx eslint` 0 错误。 +- 待跟进:Mastra 的 `createTool` 会在调用业务 `execute` 之前完成入参校验,校验失败时**返回**错误对象而不是抛出,因此模型若仍误传 `theme`,本次运行仍会消耗一个步骤,只是拿到的是明确可纠正的提示,而不再是不透明的 `invalid_consultation_domain_plan`,且不会进入计算缓存。该类校验失败同样不追加运行步骤,回执中依旧看不到;是否为“入参被 schema 拒绝”单独记一条受控失败码,留待与 `failureCode` 分类一并评估。另记 `maxSteps` 取 8 而非更大值的依据是时钟而非步数:单次排盘约 20s,路由 `maxDuration` 为 120s、Agent 超时 110s,三次失败计算即会先耗尽时钟;8 步刚好覆盖最长有用形态——Skill 加载、两次渐进式披露参考读取、一次计算加一次重试、一次写回答,继续放大只会在注定失败的运行上多花 token,不会换来更多计算机会。 +- 防复发:模型可见的工具参数不得存在两个语义重叠的字段,互斥关系必须由 schema 表达而不是运行期抛出;任何在模型契约中被移除的字段,必须同时从 Agent instructions 中删除,否则提示词会继续引导模型踩坑。步数预算与时钟预算必须相邻声明并在同一处说明彼此关系,不得分散硬编码。凡以“模型没写回答”为现象的问题,必须先能读到 `finishReason` 与实际步数再下结论;新增观测字段只能是封闭枚举或计数,且必须同时验证其不会进入 strict 的对外回执。 +- 相关记录:BUG-214、BUG-205、BUG-186 +- 修复版本:本地未提交候选 diff --git a/frontend/src/app/api/consult/route.ts b/frontend/src/app/api/consult/route.ts index d27923aa..d145f2ed 100644 --- a/frontend/src/app/api/consult/route.ts +++ b/frontend/src/app/api/consult/route.ts @@ -36,6 +36,7 @@ import { streamAgentResponse } from "@/lib/stream-agent-response"; import type { AgentExecutionReceipt, WorkflowReceipt } from "@/lib/consultation-agent-events"; import { createConsultationAgentContext, + consultationModelStepTelemetry, consultationStepBudgetReceipt, createConsultationRuntimeHooks, createConsultationRuntimeState, @@ -62,6 +63,16 @@ import { z } from "zod"; export const runtime = "nodejs"; export const maxDuration = 120; +// These two limits bound the same Agent run and must be changed together. One +// chart calculation takes about 20s and maxDuration caps the request near 120s, +// so wall-clock time, not steps, is the binding constraint: three failed +// calculations exhaust the timeout no matter how many steps remain. The step +// budget therefore only has to cover the longest useful shape—skill load, a +// couple of progressive-disclosure reference reads, one calculation plus one +// retry, and the answer turn—since a larger budget cannot buy more time. +const AGENT_MAX_STEPS = 8; +const AGENT_TIMEOUT_MS = 110_000; + const chatRequestMetadataSchema = z.object({ requestId: z.string().uuid(), sessionId: z.string().uuid(), @@ -513,7 +524,10 @@ export async function POST(request: Request) { name: string, generalDailyContext: GeneralDailyPanchangaContext | null, ) { - const state = createConsultationRuntimeState(); + // The recorded step list has to be able to hold everything the model loop + // can produce, otherwise a run that exhausts its steps also truncates the + // evidence of having done so. + const state = createConsultationRuntimeState({ plannedSteps: AGENT_MAX_STEPS }); const hooks = createConsultationRuntimeHooks(state); const usages: Promise[] = []; const agentStartedAt = Date.now(); @@ -581,6 +595,7 @@ export async function POST(request: Request) { }, ], retryCount: Math.max(0, usages.length - 1), + ...consultationModelStepTelemetry(state), ...(errorCode === undefined ? {} : { errorCode }), inputTokens, outputTokens, @@ -621,10 +636,10 @@ export async function POST(request: Request) { ].filter(Boolean).join("\n"), }, ]; - const agentAbortSignal = AbortSignal.timeout(110_000); + const agentAbortSignal = AbortSignal.timeout(AGENT_TIMEOUT_MS); const streamOptions = { runId: requestId, - maxSteps: 6, + maxSteps: AGENT_MAX_STEPS, abortSignal: agentAbortSignal, hooks, }; diff --git a/frontend/src/lib/agent-observability.ts b/frontend/src/lib/agent-observability.ts index f880cebb..a3ac4ac2 100644 --- a/frontend/src/lib/agent-observability.ts +++ b/frontend/src/lib/agent-observability.ts @@ -47,6 +47,37 @@ export const billingSettlementResults = [ export type AgentBillingSettlementResult = (typeof billingSettlementResults)[number]; export type AgentSettlementResult = Exclude; +/** + * Closed vocabulary for why the model stopped stepping. These are the provider + * finish reasons plus the two Mastra adds; anything else normalizes to + * `unknown` so an unrecognized provider string can never become a log field. + * + * `tool-calls` on a run that produced no answer means the step budget ran out + * while the model still wanted to call a tool, which is otherwise only + * inferable from the recorded step list. + */ +export const agentModelFinishReasons = [ + "stop", + "length", + "content-filter", + "tool-calls", + "error", + "other", + "tripwire", + "retry", + "unknown", +] as const; + +export type AgentModelFinishReason = (typeof agentModelFinishReasons)[number]; + +const knownModelFinishReasons = new Set(agentModelFinishReasons); + +export function toAgentModelFinishReason(value: unknown): AgentModelFinishReason { + return typeof value === "string" && knownModelFinishReasons.has(value) + ? value as AgentModelFinishReason + : "unknown"; +} + export type AgentSettlementTelemetryOutcome = Readonly<{ billingSettlementResult: AgentSettlementResult; errorCode?: string; @@ -97,6 +128,10 @@ export const agentObservabilityEventSchema = z.object({ contractPhases: z.array(agentObservabilityContractPhaseSchema).max(64).optional(), retryCount: z.number().int().min(0).max(100).optional(), errorCode: machineCodeSchema.optional(), + // Why the model stopped, and how many model steps the run consumed across + // every attempt. Both are enum-like machine values, never provider text. + modelFinishReason: z.enum(agentModelFinishReasons).optional(), + modelStepCount: countSchema.optional(), inputTokens: tokenCountSchema.optional(), outputTokens: tokenCountSchema.optional(), diff --git a/frontend/src/lib/stream-agent-response.ts b/frontend/src/lib/stream-agent-response.ts index 04d6e823..81298395 100644 --- a/frontend/src/lib/stream-agent-response.ts +++ b/frontend/src/lib/stream-agent-response.ts @@ -9,6 +9,7 @@ import { type AgentExecutionReceipt, type ConsultationAgentPublicEvent, } from "./consultation-agent-events.ts"; +import { toAgentModelFinishReason } from "./agent-observability.ts"; import { createVisibleTextTransformer } from "./stream-text-response.ts"; type Chunk = { type?: string; payload?: Record; data?: unknown }; @@ -58,6 +59,25 @@ function activity(value: unknown): ConsultationAgentPublicEvent | null { return { type: "activity", phase: phase.data, label: data.label.slice(0, 120) }; } +/** + * The runtime only reveals how the model loop ended through the stream: one + * `step-finish` per model step, then a terminal `finish` carrying the reason + * the model stopped and the authoritative step list. Without this, a run that + * exhausted its step budget is indistinguishable from one that chose to stop, + * because progressive-disclosure reads never reach the public event stream. + */ +function finishTelemetry(chunk: Chunk) { + const payload = chunk.payload as { + stepResult?: { reason?: unknown }; + output?: { steps?: unknown }; + } | undefined; + const steps = payload?.output?.steps; + return { + reason: toAgentModelFinishReason(payload?.stepResult?.reason), + stepCount: Array.isArray(steps) ? steps.length : null, + }; +} + function safeToolError(error: unknown) { if (error instanceof DOMException && error.name === "AbortError") return "cancelled" as const; if (error instanceof DOMException && error.name === "TimeoutError") return "timeout" as const; @@ -193,8 +213,17 @@ export function streamAgentResponse(options: StreamAgentResponseOptions) { if (/\S/.test(held)) emitted = true; held = ""; }; + // A retry runs a second model loop under the same step budget, so the run + // total accumulates while the finish reason describes the latest attempt. + const stepCountBeforeAttempt = options.state.modelStepCount; for await (const chunk of readChunks(stream)) { for (const event of mapChunk(chunk, options, startedAt, jyotishSkillCallIds)) send(controller, event); + if (chunk.type === "step-finish") options.state.modelStepCount += 1; + if (chunk.type === "finish") { + const finish = finishTelemetry(chunk); + options.state.modelFinishReason = finish.reason; + if (finish.stepCount !== null) options.state.modelStepCount = stepCountBeforeAttempt + finish.stepCount; + } if (chunk.type === "text-delta" && typeof chunk.payload?.text === "string") { await outputText(visible.push(chunk.payload.text)); } diff --git a/frontend/src/mastra/consultation-tools.ts b/frontend/src/mastra/consultation-tools.ts index 7a8741bb..b4a09fa7 100644 --- a/frontend/src/mastra/consultation-tools.ts +++ b/frontend/src/mastra/consultation-tools.ts @@ -8,6 +8,7 @@ import { applyBirthTimeModeToWorkflowContext, type ConsultationBirthTimeMode } f import type { ServerChartConsultation } from "../lib/consultation-route-service.ts"; import { createConsultationPlan, type ConsultationPlan } from "../lib/consultation-plan.ts"; import type { WorkflowReceipt } from "../lib/consultation-agent-events.ts"; +import type { AgentModelFinishReason } from "../lib/agent-observability.ts"; import { consultationInputSchema, consultationWorkflowFailureCode, @@ -20,11 +21,13 @@ import { const MAX_CONSULTATION_DOMAINS = 6; const domainPlanValueSchema = z.string().trim().min(1).max(64); +// The model may only express a domain plan one way. A second, mutually +// exclusive field was representable here but rejected at execution, so every +// call that set both spent a model step to learn a rule the schema never +// stated. Internal callers keep the single-value form; see canonicalDomainPlan. const consultationToolInputSchema = z.object({ question: z.string().trim().min(1).max(500), domains: z.array(domainPlanValueSchema).min(1).max(MAX_CONSULTATION_DOMAINS).optional(), - // Transitional compatibility for older Agent calls. New calls must use domains. - theme: domainPlanValueSchema.optional(), }).strict(); const MAX_RECORDED_STEPS = 32; @@ -57,6 +60,10 @@ export type ConsultationRuntimeState = { steps: ConsultationRuntimeStep[]; stepBudget: ConsultationStepBudget; stepsTruncated: boolean; + // Diagnostics for the model step budget. Internal only: the public receipt is + // strict and would reject them, so they never enter it. + modelStepCount: number; + modelFinishReason?: AgentModelFinishReason; }; export function createConsultationRuntimeState(options: { plannedSteps?: number; reservedValidationSteps?: number } = {}): ConsultationRuntimeState { @@ -72,6 +79,19 @@ export function createConsultationRuntimeState(options: { plannedSteps?: number; steps: [], stepBudget: { planned, reservedValidation, total: planned + reservedValidation }, stepsTruncated: false, + modelStepCount: 0, + }; +} + +/** + * The controlled fields that report how the model loop ended. Kept beside the + * public allowlist so both directions of the boundary are visible: these go to + * the observability log only, never to the client receipt. + */ +export function consultationModelStepTelemetry(state: ConsultationRuntimeState) { + return { + modelStepCount: state.modelStepCount, + ...(state.modelFinishReason === undefined ? {} : { modelFinishReason: state.modelFinishReason }), }; } @@ -128,7 +148,13 @@ export function createConsultationAgentContext(context: ConsultationAgentContext return Object.freeze(context); } -function canonicalDomainPlan( +/** + * Resolves the one domain plan a call may express. The model-facing schema + * declares only `domains` and rejects anything else before execute() runs, so + * the single-value `theme` form and its mutual exclusion remain the contract + * for callers that build a plan without that schema. + */ +export function canonicalDomainPlan( input: { domains?: readonly unknown[]; theme?: unknown }, context: Pick, ): ConsultationDomain[] { @@ -202,7 +228,7 @@ export function createConsultationTools(ctx: ConsultationAgentContext) { let calculation: Promise> | null = null; const consultationTool = createTool({ id: "run-jyotish-consultation", - description: "Run one server-validated plan of up to six allowlisted personal Jyotish consultation domains. Use domains in priority order; birth data is server-bound and must never be supplied by the model.", + description: "Run one server-validated plan of up to six allowlisted personal Jyotish consultation domains. Send only question and domains. The single ordered domains array is the only way to select domains: list them in priority order, or omit it entirely to accept the domain the server already selected for this consultation. Birth data is server-bound and must never be supplied. One calculation is executed per request and reused, so repeating the call with different parameters cannot change the result.", inputSchema: consultationToolInputSchema, execute: async (input, context) => { const domains = canonicalDomainPlan(input, ctx); diff --git a/frontend/src/mastra/index.ts b/frontend/src/mastra/index.ts index ec0b7175..c057534c 100644 --- a/frontend/src/mastra/index.ts +++ b/frontend/src/mastra/index.ts @@ -35,7 +35,7 @@ const jyotishInstructions = `You are the guide for a conversational Vedic astrol Write in concise Simplified Chinese as a natural conversation, not a report or fixed template. Use Markdown only when it improves scanning; tables are allowed only for genuinely comparative information. For Vedic astrology questions, load the jyotish-vedic-astrology skill before deciding which calculation tool or workflow to use. Follow the skill's method and truth boundaries, but use run-jyotish-consultation for actual chart calculations instead of inventing results. For questions that require a new chart claim, call run-jyotish-consultation before answering. Simple conversational follow-ups may use the existing context. -When a question spans multiple supported consultation domains, submit one ordered domains plan to run-jyotish-consultation. The server canonicalizes aliases, rejects unsupported/product domains, executes each accepted domain, and returns the actual domains in the tool context and receipt. Use the legacy theme field only for a single-domain compatibility retry. +Select consultation domains only through the single ordered domains array of run-jyotish-consultation, whether the question covers one domain or several; omit it to accept the domain the server already selected. The server canonicalizes aliases, rejects unsupported/product domains, executes each accepted domain, and returns the actual domains in the tool context and receipt. A rejected domain plan is final for this run: correct the domains once, and never re-send the same call with extra parameters. Activity, progress, tool status, and execution receipts are server-owned. Never imitate data-jyotish-activity, activity events, tool-started/tool-completed messages, or receipts in the answer text. Treat the server-provided current time as authoritative for words such as today, now, this year, and the next few months. Never infer the current date from model knowledge or the birth date. Treat consumer_context as the authoritative answer policy: diff --git a/frontend/tests/agent-observability.test.ts b/frontend/tests/agent-observability.test.ts index 0c2bd8b2..b646391a 100644 --- a/frontend/tests/agent-observability.test.ts +++ b/frontend/tests/agent-observability.test.ts @@ -5,9 +5,11 @@ import test from "node:test"; import { ZodError } from "zod"; import { + agentModelFinishReasons, agentObservabilityEventSchema, createAgentObservabilityLogger, settlementTelemetryOutcome, + toAgentModelFinishReason, toAgentObservabilityErrorCode, } from "../src/lib/agent-observability.ts"; @@ -30,6 +32,8 @@ const baseEvent = { ], retryCount: 1, errorCode: "runtime_contract_incomplete", + modelFinishReason: "tool-calls", + modelStepCount: 8, inputTokens: 1200, outputTokens: 345, evidenceCount: 4, @@ -151,6 +155,40 @@ test("error normalization never records arbitrary exception messages", () => { ); }); +test("the model finish reason is a closed vocabulary, never a provider string", () => { + for (const reason of agentModelFinishReasons) { + assert.equal(toAgentModelFinishReason(reason), reason); + } + assert.equal(toAgentModelFinishReason("max-steps-exceeded"), "unknown"); + assert.equal(toAgentModelFinishReason("provider said user@example.com"), "unknown"); + assert.equal(toAgentModelFinishReason(undefined), "unknown"); + assert.equal(toAgentModelFinishReason({ reason: "stop" }), "unknown"); + assert.throws( + () => agentObservabilityEventSchema.parse({ + runId: baseEvent.runId, + modelFinishReason: "stopped because the provider said so", + }), + ZodError, + ); +}); + +test("step exhaustion is recorded rather than inferred from the step list", () => { + const emitted: unknown[] = []; + const logger = createAgentObservabilityLogger((event) => emitted.push(event)); + + logger({ + runId: baseEvent.runId, + modelFinishReason: toAgentModelFinishReason("tool-calls"), + modelStepCount: 8, + }); + + assert.deepEqual(emitted, [{ + runId: baseEvent.runId, + modelFinishReason: "tool-calls", + modelStepCount: 8, + }]); +}); + test("settlement telemetry reports successful cancellation as cancelled", () => { assert.deepEqual( settlementTelemetryOutcome("cancelled", "cancelled"), @@ -188,5 +226,6 @@ test("ordinary consultation logRun uses the strict logger and aggregated usage", assert.match(route, /inputTokens/); assert.match(route, /outputTokens/); assert.match(route, /billingSettlementResult/); + assert.match(route, /\.\.\.consultationModelStepTelemetry\(state\),/); assert.doesNotMatch(route, /\[consult-agentic\]/); }); diff --git a/frontend/tests/consultation-agentic-runtime.test.ts b/frontend/tests/consultation-agentic-runtime.test.ts index e2cffbb4..f912ae9d 100644 --- a/frontend/tests/consultation-agentic-runtime.test.ts +++ b/frontend/tests/consultation-agentic-runtime.test.ts @@ -2,6 +2,8 @@ import assert from "node:assert/strict"; import test from "node:test"; import { appendConsultationRuntimeStep, + canonicalDomainPlan, + consultationModelStepTelemetry, consultationStepBudgetReceipt, createConsultationTools, createConsultationRuntimeState, @@ -29,6 +31,13 @@ const serverChart = { }, }; +// Mastra validates against the model-facing schema before execute() runs, so a +// test can only send what the model can send. The cast keeps the argument +// checked against that shape without depending on Mastra's inferred type. +type ModelConsultationToolInput = { question: string; domains?: string[] }; +const modelInput = (input: ModelConsultationToolInput) => input as never; +const rejectedByInputSchema = (input: { question: string; theme?: string; domains?: string[] }) => input as never; + function workflow( theme = "career", options: { status?: "ready" | "degraded" | "blocked"; missingLayers?: string[]; preciseTiming?: boolean } = {}, @@ -45,7 +54,7 @@ function workflow( }; } -test("context-bound tool keeps legacy single theme compatibility and calculates once", async () => { +test("the server-selected domain stays authoritative when the model omits domains", async () => { let calls = 0; let captured: unknown; let capturedPlan: unknown; @@ -66,12 +75,14 @@ test("context-bound tool keeps legacy single theme compatibility and calculates }, }); const tool = tools["run-jyotish-consultation"]; - assert.deepEqual(Object.keys((tool.inputSchema as unknown as { shape: object }).shape), ["question", "domains", "theme"]); + // A single-value theme is deliberately absent: two mutually exclusive ways to + // name a domain cost the model a step per call to discover the rule. + assert.deepEqual(Object.keys((tool.inputSchema as unknown as { shape: object }).shape), ["question", "domains"]); const execute = tool.execute!; const context = { observe: { span: async (_n: string, fn: () => Promise) => fn(), log() {} } } as never; const [first, second] = await Promise.all([ - execute({ question: "尝试改成精确应期", theme: "timing" }, context), - execute({ question: "尝试改成婚恋", theme: "marriage" }, context), + execute(modelInput({ question: "尝试改成精确应期" }), context), + execute(modelInput({ question: "尝试改成婚恋" }), context), ]); assert.equal(calls, 1); assert.deepEqual(first, second); @@ -97,7 +108,7 @@ test("multi-domain plan canonicalizes aliases, de-duplicates, preserves order, a }, }); const result = await tools["run-jyotish-consultation"].execute!( - { question: "事业、财富和迁居怎么一起规划", domains: ["career", "finance", "career", "home"] }, + modelInput({ question: "事业、财富和迁居怎么一起规划", domains: ["career", "finance", "career", "home"] }), { observe: { span: async (_n: string, fn: () => Promise) => fn(), log() {} } } as never, ) as { domains: string[]; consultations: Array<{ domain: string }> }; @@ -128,7 +139,7 @@ test("domain plan rejects unknown and product domains before any workflow runs", })["run-jyotish-consultation"]; await assert.rejects( tool.execute!( - { question: "测试", domains: ["career", domain] }, + modelInput({ question: "测试", domains: ["career", domain] }), { observe: { span: async (_n: string, fn: () => Promise) => fn(), log() {} } } as never, ), /unsupported_consultation_domain/, @@ -154,16 +165,51 @@ test("domain plan enforces the raw plan upper bound and one input mode", async ( }).success, false); assert.equal(calls, 0); + // The model can only express a domain plan one way. The pair that used to be + // representable, and cost a model step to be told was invalid, is now refused + // by the schema before the tool body runs. + assert.equal(inputSchema.safeParse({ question: "测试", domains: ["career"] }).success, true); + assert.equal(inputSchema.safeParse({ question: "测试" }).success, true); + assert.equal(inputSchema.safeParse({ question: "测试", theme: "career" }).success, false); + assert.equal(inputSchema.safeParse({ question: "测试", domains: ["career"], theme: "career" }).success, false); + + const rejectedState = createConsultationRuntimeState(); const secondTool = createConsultationTools({ userId: "u", sessionId: "s", requestId: "r-modes", consultationMode: "verified_chart", - serverChart, state: createConsultationRuntimeState(), + serverChart, state: rejectedState, runWorkflow: async () => { calls += 1; return workflow(); }, })["run-jyotish-consultation"]; - await assert.rejects( - secondTool.execute!({ question: "测试", domains: ["career"], theme: "career" }, context), + const refused = await secondTool.execute!( + rejectedByInputSchema({ question: "测试", domains: ["career"], theme: "career" }), + context, + ) as Record; + + // Nothing about the run advances: no calculation, no counted attempt, and no + // consultation payload the model could mistake for a result. + assert.equal(calls, 0); + assert.equal("domains" in refused, false); + assert.equal(rejectedState.consultationToolStarted, false); + assert.equal(rejectedState.consultationToolCallCount, 0); + assert.deepEqual(rejectedState.steps, []); +}); + +test("the single-value domain form stays available to callers without the model schema", () => { + const plan = createConsultationPlan({ + userIntent: "事业如何", theme: "career", consultationMode: "verified_chart", modelCreditCost: 1, + }); + + // A single-value theme must never override the route-selected server domain. + assert.deepEqual(canonicalDomainPlan({ theme: "timing" }, { plan, theme: "career" }), ["career"]); + assert.deepEqual(canonicalDomainPlan({}, { plan, theme: "career" }), ["career"]); + assert.deepEqual(canonicalDomainPlan({ theme: "marriage" }, {}), ["marriage"]); + assert.deepEqual(canonicalDomainPlan({ domains: ["career", "finance"] }, {}), ["career", "wealth"]); + assert.deepEqual(canonicalDomainPlan({ domains: ["timing"] }, { plan, theme: "career" }), ["timing"]); + assert.throws( + () => canonicalDomainPlan({ domains: ["career"], theme: "career" }, {}), /invalid_consultation_domain_plan/, ); - assert.equal(calls, 0); + assert.throws(() => canonicalDomainPlan({}, {}), /invalid_consultation_domain_plan/); + assert.throws(() => canonicalDomainPlan({ theme: "prashna" }, {}), /unsupported_consultation_domain/); }); test("invalid model input does not poison a later valid contract retry", async () => { @@ -176,15 +222,17 @@ test("invalid model input does not poison a later valid contract retry", async ( })["run-jyotish-consultation"]; const context = { observe: { span: async (_n: string, fn: () => Promise) => fn(), log() {} } } as never; + // BUG-205: an input the schema accepts but the domain registry rejects must + // still be refused before the request-scoped calculation cache is written. await assert.rejects( - tool.execute!({ question: "先给出错误参数", domains: ["career"], theme: "career" }, context), - /invalid_consultation_domain_plan/, + tool.execute!(modelInput({ question: "先给出错误参数", domains: ["career", "unknown"] }), context), + /unsupported_consultation_domain/, ); assert.equal(calls, 0); assert.equal(state.consultationToolCallCount, 0); assert.equal(state.consultationToolSuccessCount, 0); - const result = await tool.execute!({ question: "改用合法参数", theme: "timing" }, context) as { domains: string[] }; + const result = await tool.execute!(modelInput({ question: "改用合法参数", domains: ["timing"] }), context) as { domains: string[] }; assert.equal(calls, 1); assert.deepEqual(result.domains, ["timing"]); assert.equal(state.consultationToolCallCount, 1); @@ -207,10 +255,10 @@ test("a rejected workflow promise is cleared before a later tool call", async () const context = { observe: { span: async (_n: string, fn: () => Promise) => fn(), log() {} } } as never; await assert.rejects( - tool.execute!({ question: "第一次计算", theme: "career" }, context), + tool.execute!(modelInput({ question: "第一次计算", domains: ["career"] }), context), /workflow_temporarily_failed/, ); - const result = await tool.execute!({ question: "重新计算", theme: "timing" }, context) as { domains: string[] }; + const result = await tool.execute!(modelInput({ question: "重新计算", domains: ["timing"] }), context) as { domains: string[] }; assert.equal(calls, 2); assert.deepEqual(result.domains, ["timing"]); @@ -231,7 +279,7 @@ test("a failed calculation records why it failed and forwards the request id", a })["run-jyotish-consultation"]; await assert.rejects( - tool.execute!({ question: "队列满时的表现", theme: "career" }, { observe: { span: async (_n: string, fn: () => Promise) => fn(), log() {} } } as never), + tool.execute!(modelInput({ question: "队列满时的表现", domains: ["career"] }), { observe: { span: async (_n: string, fn: () => Promise) => fn(), log() {} } } as never), /Async job queue is full/, ); @@ -276,6 +324,37 @@ test("the public receipt never carries the internal failure classification", () })); }); +test("the public receipt never carries the model step budget diagnostics", () => { + const state = createConsultationRuntimeState(); + state.modelStepCount = 8; + state.modelFinishReason = "tool-calls"; + appendConsultationRuntimeStep(state, { kind: "skill", name: "jyotish-vedic-astrology", status: "completed" }); + + assert.deepEqual(consultationModelStepTelemetry(state), { modelStepCount: 8, modelFinishReason: "tool-calls" }); + assert.deepEqual( + consultationModelStepTelemetry(createConsultationRuntimeState()), + { modelStepCount: 0 }, + ); + + // The client receipt schema is strict, so leaking either field would make a + // successful run fail while serializing its own answer. + const receipt = agentExecutionReceiptSchema.parse({ + runId: "run", runtime: "mastra-agentic", + skill: { name: "jyotish-vedic-astrology", loaded: true }, + steps: publicConsultationRuntimeSteps(state), + stepBudget: consultationStepBudgetReceipt(state), + workflow: { route: "career", status: "ready", preciseTiming: "blocked", missingLayers: [] }, + }); + assert.doesNotMatch(JSON.stringify(receipt), /modelStepCount|modelFinishReason|tool-calls/); + assert.throws(() => agentExecutionReceiptSchema.parse({ + runId: "run", runtime: "mastra-agentic", + skill: { name: "jyotish-vedic-astrology", loaded: true }, + steps: publicConsultationRuntimeSteps(state), + workflow: { route: "career", status: "ready", preciseTiming: "blocked", missingLayers: [] }, + ...consultationModelStepTelemetry(state), + })); +}); + test("personal Agent exposes the Jyotish Skill and named server tool", async () => { const state = createConsultationRuntimeState(); const agent = getJyotishAgent({ @@ -502,6 +581,87 @@ test("ensures a controlled final response after a successful tool-only run", asy }); +test("a completed run records the finish reason and the authoritative step count", async () => { + const state = createConsultationRuntimeState(); + state.jyotishSkillLoaded = true; + state.consultationToolCallCount = 1; + state.consultationToolSuccessCount = 1; + state.consultationToolCompleted = true; + state.workflowReceipt = { route: "career", status: "ready", preciseTiming: "blocked", missingLayers: [] }; + async function* chunks() { + yield { type: "step-finish", payload: { stepResult: { reason: "tool-calls" } } }; + yield { type: "text-delta", payload: { text: "事业方向的判断如下。" } }; + yield { type: "step-finish", payload: { stepResult: { reason: "stop" } } }; + // The terminal chunk carries the runtime's own step list, which wins over + // the chunks we counted. + yield { type: "finish", payload: { stepResult: { reason: "stop" }, output: { usage: {}, steps: [{}, {}, {}] } } }; + } + const response = streamAgentResponse({ + runId: "run", requestId: "req", state, stream: chunks(), requireTool: true, + toolStatus: () => "ready", receipt: () => receipt(state), + }); + await response.text(); + + assert.equal(state.modelFinishReason, "stop"); + assert.equal(state.modelStepCount, 3); +}); + +test("a run that stops while still wanting tools records the exhausted step budget", async () => { + const state = createConsultationRuntimeState({ plannedSteps: 8 }); + state.jyotishSkillLoaded = true; + state.consultationToolCallCount = 1; + state.consultationToolSuccessCount = 1; + state.consultationToolCompleted = true; + state.workflowReceipt = { route: "career", status: "ready", preciseTiming: "blocked", missingLayers: [] }; + // The production shape: the calculation succeeded, the model never wrote an + // answer, and only the fallback text reached the user. Nothing in the public + // event stream said the step budget ran out. + async function* chunks() { + yield { type: "tool-result", payload: { toolCallId: "tool-1", toolName: "run-jyotish-consultation", result: {} } }; + yield { type: "step-finish", payload: { stepResult: { reason: "tool-calls" } } }; + yield { type: "step-finish", payload: { stepResult: { reason: "tool-calls" } } }; + yield { type: "finish", payload: { stepResult: { reason: "tool-calls" }, output: { usage: {} } } }; + } + let completedOutput = ""; + const response = streamAgentResponse({ + runId: "run", requestId: "req", state, stream: chunks(), requireTool: true, + toolStatus: () => "ready", receipt: () => receipt(state), + onComplete: (output) => { completedOutput = output; }, + }); + await response.text(); + + assert.equal(completedOutput, ensureFinalResponseText("", true)); + assert.equal(state.modelFinishReason, "tool-calls"); + assert.equal(state.modelStepCount, 2); +}); + +test("a retry accumulates model steps and reports the latest finish reason", async () => { + const state = createConsultationRuntimeState({ plannedSteps: 8 }); + async function* firstAttempt() { + yield { type: "step-finish", payload: { stepResult: { reason: "tool-calls" } } }; + yield { type: "finish", payload: { stepResult: { reason: "tool-calls" }, output: { usage: {} } } }; + } + async function* retriedAttempt() { + state.jyotishSkillLoaded = true; + state.consultationToolCallCount = 1; + state.consultationToolSuccessCount = 1; + state.consultationToolCompleted = true; + state.workflowReceipt = { route: "career", status: "ready", preciseTiming: "blocked", missingLayers: [] }; + yield { type: "text-delta", payload: { text: "补齐后的回答。" } }; + yield { type: "step-finish", payload: { stepResult: { reason: "stop" } } }; + yield { type: "finish", payload: { stepResult: { reason: "unrecognized-provider-reason" }, output: { usage: {} } } }; + } + const response = streamAgentResponse({ + runId: "run", requestId: "req", state, stream: firstAttempt(), requireTool: true, + retry: async () => retriedAttempt(), + toolStatus: () => "ready", receipt: () => receipt(state), + }); + await response.text(); + + assert.equal(state.modelStepCount, 2); + assert.equal(state.modelFinishReason, "unknown"); +}); + test("persistence failure emits run.failed instead of run.completed", async () => { const state = createConsultationRuntimeState(); state.jyotishSkillLoaded = true; diff --git a/frontend/tests/consultation-stream-recovery.test.ts b/frontend/tests/consultation-stream-recovery.test.ts index 84a93dfa..97929dc9 100644 --- a/frontend/tests/consultation-stream-recovery.test.ts +++ b/frontend/tests/consultation-stream-recovery.test.ts @@ -53,7 +53,9 @@ test("Agentic failures always refund and detached execution uses a server-owned consultRoute.indexOf("async function runAgenticConsultation("), consultRoute.indexOf(" try {\n const { history } = parsed.data;"), ); - assert.match(agentic, /const agentAbortSignal = AbortSignal\.timeout\(110_000\)/); + // The value lives beside the model step budget, which bounds the same run. + assert.match(consultRoute, /const AGENT_TIMEOUT_MS = 110_000;/); + assert.match(agentic, /const agentAbortSignal = AbortSignal\.timeout\(AGENT_TIMEOUT_MS\)/); assert.equal(agentic.match(/abortSignal: agentAbortSignal/g)?.length, 2); assert.doesNotMatch(agentic, /abortSignal: request\.signal/); assert.equal( diff --git a/frontend/tests/consultation-workflow-contract.test.ts b/frontend/tests/consultation-workflow-contract.test.ts index 3de32772..4c550a41 100644 --- a/frontend/tests/consultation-workflow-contract.test.ts +++ b/frontend/tests/consultation-workflow-contract.test.ts @@ -52,6 +52,15 @@ test("consultation plans are server-owned and bounded", () => { assert.match(pythonPlanContract, /can_answer_precise_timing": False/); }); +test("the model step budget and the wall-clock budget are declared as one pair", () => { + assert.match(route, /const AGENT_MAX_STEPS = 8;\nconst AGENT_TIMEOUT_MS = 110_000;/); + assert.match(route, /maxSteps: AGENT_MAX_STEPS,/); + assert.match(route, /AbortSignal\.timeout\(AGENT_TIMEOUT_MS\)/); + assert.match(route, /createConsultationRuntimeState\(\{ plannedSteps: AGENT_MAX_STEPS \}\)/); + assert.doesNotMatch(route, /maxSteps: \d/); + assert.doesNotMatch(route, /AbortSignal\.timeout\(\d/); +}); + test("uses one runtime step append entry and no scattered hard-coded step cap", () => { assert.match(tools, /export function appendConsultationRuntimeStep/); assert.doesNotMatch(tools, /steps\.length\s*>=\s*32/);