mirror of
https://github.com/THU-MAIC/OpenMAIC.git
synced 2026-10-02 09:24:43 +08:00
refactor(pbl-v2): route runtime agents through the shared LLM entry point (#1006)
* refactor(pbl-v2): route runtime agents through the shared LLM entry point The five PBL v2 runtime call sites (instructor x2, evaluator, simulator x2) invoked the AI SDK directly while the other 21 call sites in lib/ and app/ go through callLLM / streamLLM, so they silently opted out of everything the shared entry point applies: - Usage accounting. recordUsage is reached only from the wrappers, so no instructor turn, evaluator pass, simulator line or narrator pass was ever accounted for - the highest-frequency, longest-context LLM traffic in the product. The comment at llm.ts:275, that every server-side call funnels through the wrappers, did not hold. - The LLM_THINKING_DISABLED kill switch, read inside the wrappers only. - One thinking resolution path. runtime-thinking.ts existed precisely because the agents bypassed the wrapper, and it was wired into instructor (x2) and evaluator but neither simulator site. Also fix callLLM recording result.usage, which on a multi-step tool run (stopWhen) is the final step alone; totalUsage is the aggregate, and the instructor turn is exactly such a run. streamLLM already preferred the aggregate. Thinking semantics are preserved per call site: the teaching turns keep force-disabling thinking, now through the wrapper's own `thinking` argument, and keep their existing providerOptions spread, which injectProviderOptions yields to; the simulator keeps honouring a per-request config, as before. The one behavioural difference is on native adapters when no thinking config arrives: the policy now resolves to an explicit lowest/disabled provider option instead of leaving the model's default in place - which is what the policy always claimed to do, and what OpenAI-compatible providers already got. Unifying the wider divergence is a product decision and stays out of scope; runtime-thinking.ts now documents it, along with the fact that the original tool_choice justification no longer matches the tree. A lint guard restricts importing generateText / streamText to lib/ai/llm.ts so the split cannot grow back. It uses the @typescript-eslint variant of no-restricted-imports deliberately: flat config replaces a rule's options per key rather than merging them, and the base rule is already configured for the lib/choreography and lib/video-export module boundaries. The runtime-thinking test moves from asserting the retired helper to asserting what the call site is responsible for, at the provider boundary: an evaluator turn reaches the provider with a disabled config in the thinking store, does not leak it, and is accounted for under its own usage source. Closes #1003 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(pbl-v2): address cross-review findings on the shared-entry migration Consensus findings from an independent cross-vendor review ofcc48ada. callLLM recorded usage only after validation passed, so with `retries > 0` every attempt that failed validation went unaccounted, and a result handed back after the retries were exhausted recorded nothing at all. Each attempt that reaches that point was billed, so record before validating. Latent today (no caller passes retryOptions) but it sits in the function this PR already touches. The simulator's character line now hands its incoming thinking config to streamLLM instead of resolving providerOptions by hand. The hand-rolled call covered native adapters only, so a stage-route thinking config was silently dropped on OpenAI-compatible providers - the wrapper also seeds the thinking context that their fetch wrapper reads. Native adapters resolve identically: both paths end in the same normalizeProviderId + buildThinkingProviderOptions. runtime-thinking.ts documents two things the previous docstring implied away. On native adapters the constant is a no-op whenever a thinking config does arrive, because the teaching turns pass their own providerOptions and injectProviderOptions yields to a caller-set value. And "disabled" is resolved against each model's catalogued capability, which for some models is not off: measured against the current catalog, gpt-5.4-pro resolves to effort medium, gemini-2.5-pro to thinkingBudget -1 (dynamic thinking) and claude-fable-5 to adaptive/low. Those match the model's own default today, so behaviour is unchanged, but the policy is weaker than its name suggests. Tests cover both accounting paths (a retried attempt and an all-attempts-failed run each record every attempt), and the defensive totalUsage fallback is now labelled as defensive - ai@6 always reports the aggregate. The lint guard's scope comment also notes namespace imports, which importNames cannot see. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(pbl-v2): correct the thinking-policy rationale; stop tests writing the real usage log Two things, both prompted by probing the live DeepSeek API with the instructing turn's exact shape rather than reasoning from the tree. The docstring in runtime-thinking.ts claimed the policy's original justification had expired, on the grounds that `begin_turn` and the forced `tool_choice` it used are gone. That reads as an invitation to delete the force-disable, and it is wrong. Measured against DeepSeek V4 Pro: thinking on + tools + stopWhen -> streams fine, 66 reasoning tokens thinking on + forced toolChoice -> still 400, "Thinking mode does not support this tool_choice" The incompatibility is bound to a FORCED tool choice, not to tools in general. Today's turns do not reach it, but anyone who reintroduces a forced tool choice will, so the policy is a live guard rather than dead weight. It also buys latency: the same probe reached its first token in 1.4s with thinking off against 3.0s with DeepSeek's default thinking on, and the teaching turns gain nothing from thinking while the instructor is the chattiest surface in the product. The docstring now records the measurements instead of the guess. Separately: a test run could append to the app's real usage log. Any test that exercises callLLM / streamLLM reaches `recordUsage` through `recordUsageSafe` without asking for it, so rows sourced `minimax-auth-test`, `minimax-thinking-test`, `minimax-fixed-thinking-test` and `serialization-test` were sitting in the live `data/usage/` file next to production traffic (tests/ai/minimax-provider.test.ts:58,101,146 and tests/ai/anthropic-serialization.test.ts:38 — neither predates this PR). `recordUsage` now returns early under vitest unless the caller asked for a specific `baseDir`, which is the fix at the boundary rather than in the two tests that happened to trip it. Storage tests pass an explicit `baseDir` and tests that assert accounting mock the module, so both keep working; verified by running all three files and confirming the live file's line count is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(pbl-v2): drop the hardcoded thinking policy, finishing the unification The point of this PR is that the PBL v2 runtime reaches the model the same way everything else does. Keeping a hardcoded `PBL_V2_TEACHING_THINKING` override inside the agents worked against that: the teaching turns still resolved thinking through a second, private path, and it produced the awkward edges this PR had to disclose — an override that silently loses to a caller-set `providerOptions` on native adapters, and a "disabled" that resolves to `effort: medium` or `thinkingBudget: -1` (dynamic thinking) on models whose catalogued capability has no off value. So the policy is gone. All five sites now pass the incoming request / stage-route config straight to `streamLLM` / `callLLM`, which is the one place that resolves provider options and seeds the thinking context. `runtime-thinking.ts` is deleted, and the hand-rolled `resolveThinkingProviderOptions` calls with it. What justified keeping it, and why that was not enough: - The DeepSeek 400 is real — probed live, thinking on + a FORCED `toolChoice` still fails with "Thinking mode does not support this tool_choice" — but no call site in the tree forces a tool choice, so the policy was guarding a scenario that does not exist, at the cost of the entry-point unification. The hazard is now a comment where a forced tool choice would be introduced, which is where a future author will actually look. - Thinking costs latency on the teaching turns (~1.4s to ~3.0s first token on a thinking-by-default model, measured on DeepSeek V4 Pro). That is a real cost but a deployment preference, and it belongs in the `pbl-v2-runtime` stage route, not hardcoded in the agent where it overrides whatever that route says. Noted at the instructor call site with the numbers. Behaviour change to be aware of: on OpenAI-compatible providers the teaching turns no longer force thinking off, so a thinking-by-default model now thinks unless the route says otherwise — slower first token and reasoning tokens on the bill. Pin `thinking` off on the stage route to keep the old behaviour. The runtime-thinking test becomes runtime-llm-entry and asserts the new invariant at the provider boundary: an incoming config arrives intact, NO config means no config (a re-added constant would fail this), the global kill switch still applies, nothing leaks, and the turn is accounted for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(pbl-v2): forward the resolved thinking config to the simulator narrator Review finding from @cosarah on #1006, and a correct one: the director-narrator pass was the last site still dropping the resolved config. Both callers passed `thinkingConfig` into `runDirectorNarratorPass`, but the helper never destructured it and never handed it to `callLLM`, so the provider silently fell back to the model default instead of honouring the `pbl-v2-runtime:simulator` route. Earlier revisions of this PR documented that as pre-existing and deliberately untouched. That defence stopped holding once the hardcoded thinking policy was deleted: the PR's invariant became "every PBL v2 site resolves thinking through the shared entry point, and the deployment decides", and the narrator was the one site contradicting it. Coverage per the review: the simulator turn is now driven end to end against a scenario fixture, asserting that BOTH provider calls — the streamed character line and the non-streamed narrator pass — see the incoming config, that neither substitutes one when the request carries none, and that both are accounted for under their own usage sources. The `['generate','stream']` assertion keeps the test from silently covering only one of the two calls. Fault-injected to confirm it bites: dropping the new fourth argument again turns "forwards an incoming config to the character line AND the narrator" red. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(lint): close the dynamic-import bypass on the LLM entry-point guard Second review pass from @cosarah on #1006. He is right that the guard was not airtight, though the picture turned out narrower than described — I checked each form against eslint instead of reasoning about it: import { streamText } from 'ai' already blocked import * as ai from 'ai' already blocked (ESLint reports a namespace import when importNames is set) require('ai') already blocked, by the repo-wide @typescript-eslint/no-require-imports await import('ai') NOT blocked <- the real hole await import(`ai`) NOT blocked So the dynamic form is now covered by a no-restricted-syntax block, in both the literal and template-literal shapes. That rule key is configured by five earlier blocks and flat config replaces options per key rather than merging them, so the new block explicitly ignores those directories rather than silently dropping their module boundaries. It costs nothing: lib/choreography and lib/video-export already allowlist their import sources (importing 'ai' there is an error today, and video-export bans ImportExpression and require outright), and the two @openmaic packages are built in isolation against @openmaic/dsl with the SDK nowhere in their dependency graph. The comment above the guard claimed namespace imports were out of scope because importNames cannot see them. That was wrong; it now lists what each form is caught by, and notes that an eslint-disable still defeats all of it — which is the point, since the bypass then has to be written down where a reviewer sees it. Verified with a probe file using all five shapes: five errors, one per line. Injecting `import 'react'` plus an `'ai'` import into lib/choreography still fails on both pre-existing boundary rules, confirming no clobbering. Also updates the PR description, which still described the narrator as ignoring its thinking config — stale since9bbb720. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(lint): cover the @openmaic packages with the dynamic-import ban Third review pass from @cosarah on #1006, and he is right again: the guard ignored packages/@openmaic/renderer and packages/@openmaic/storage, and `void import('ai')` under the renderer source path passed lint. Reproduced before fixing. The previous revision justified those two exclusions by arguing the packages are built in isolation against @openmaic/dsl with the SDK nowhere in their dependency graph. That is an argument about why a bypass would not matter, not a rule that stops one — exactly the kind of reasoning this PR exists to replace with enforcement. The two selectors are now a shared AI_SDK_DYNAMIC_IMPORT_BAN spread into both package blocks, rather than dropping those directories from the repo-wide block's ignores: they configure no-restricted-syntax for their own `@/` boundary, and flat config replaces rule options per key, so matching their files would have silently deleted that boundary. lib/choreography and lib/video-export stay ignored and stay covered — their blocks ban EVERY ImportExpression outright, which subsumes this one. That claim is now checked rather than asserted. Verified with a `void import('ai')` probe in six production paths — both @openmaic packages, choreography, video-export, lib/pbl, app/api — one error each; lib/ai/llm.ts and tests/ stay clean; and an `@/lib/foo` import under the renderer still fails its own host-path boundary, confirming no clobbering. Also rewrites the PR description's e2e section, which still described the deleted thinking policy. The browser run predates that deletion, so its streaming and accounting evidence stands (the wrapper path is unchanged) but its thinking observations do not. Replaced with a fresh live probe of the final behaviour: with no route thinking the instructing turn reports 60 reasoning tokens, with `thinking` pinned off on the stage route it reports 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(lint): guard every source extension, and pin the coverage matrix in a test Fourth review pass from @cosarah on #1006. Confirmed before fixing: an `app/api/**/route.js` containing both a named and a dynamic `'ai'` import passed lint with exit code 0, because both guard blocks matched only `**/*.{ts,tsx}` while every module-boundary block in the same file matches `**/*.{ts,tsx,js,jsx,mjs,cjs}`. Both blocks now match the same set. The repo has real `.mjs` under scripts/, so this was not hypothetical. That is the third scope hole review has found in this guard — two @openmaic package directories, then the extension list — and each one was closed and then verified by hand, which is the process that produced the next one. So the matrix is now an executable contract: tests/lint-llm-entry-guard.test.ts runs the real eslint against the real config over every import form (named, namespace, dynamic, dynamic-template) × every source extension × nine production paths, asserts the three deliberate exemptions still pass, and asserts the module boundaries that share the `no-restricted-syntax` key still fire. A future edit that narrows `files`, adds an `ignores` entry or swaps the rule key fails CI instead of quietly reopening the door. Fault-injected against both earlier findings to prove it bites: narrowing the scope back to ts/tsx turns the js/jsx/mjs/cjs rows red, and re-adding the two @openmaic packages to `ignores` turns the dynamic-import rows red. It also treats an eslint-ignored path as "not covered", so a path that silently stops being linted cannot masquerade as a pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -2,6 +2,27 @@ import { defineConfig, globalIgnores } from 'eslint/config';
|
||||
import nextVitals from 'eslint-config-next/core-web-vitals';
|
||||
import nextTs from 'eslint-config-next/typescript';
|
||||
|
||||
// The AI SDK must be reached only through callLLM / streamLLM in lib/ai/llm.ts
|
||||
// (#1003). Static and namespace imports are handled by
|
||||
// @typescript-eslint/no-restricted-imports further down; a DYNAMIC import is an
|
||||
// ImportExpression, which no-restricted-imports cannot see, so it needs
|
||||
// no-restricted-syntax. That key is configured per-block and flat config replaces
|
||||
// rule options rather than merging them, so the ban lives here and is spread into
|
||||
// every block that sets the key, instead of one repo-wide block that would
|
||||
// silently drop those blocks' own module boundaries.
|
||||
const AI_SDK_DYNAMIC_IMPORT_BAN = [
|
||||
{
|
||||
selector: "ImportExpression > Literal[value='ai']",
|
||||
message:
|
||||
"Call the model through callLLM / streamLLM in @/lib/ai/llm instead of importing the AI SDK dynamically. A dynamic import('ai') reaches the same generateText / streamText and skips usage accounting, the LLM_THINKING_DISABLED kill switch and per-provider thinking resolution.",
|
||||
},
|
||||
{
|
||||
selector: "ImportExpression > TemplateLiteral > TemplateElement[value.cooked='ai']",
|
||||
message:
|
||||
'Call the model through callLLM / streamLLM in @/lib/ai/llm instead of importing the AI SDK dynamically (template-literal form of the same bypass).',
|
||||
},
|
||||
];
|
||||
|
||||
const eslintConfig = defineConfig([
|
||||
...nextVitals,
|
||||
...nextTs,
|
||||
@@ -78,6 +99,7 @@ const eslintConfig = defineConfig([
|
||||
rules: {
|
||||
'no-restricted-syntax': [
|
||||
'error',
|
||||
...AI_SDK_DYNAMIC_IMPORT_BAN,
|
||||
{
|
||||
selector: 'Literal[value=/^@\\//]',
|
||||
message:
|
||||
@@ -102,6 +124,7 @@ const eslintConfig = defineConfig([
|
||||
rules: {
|
||||
'no-restricted-syntax': [
|
||||
'error',
|
||||
...AI_SDK_DYNAMIC_IMPORT_BAN,
|
||||
{
|
||||
selector: 'Literal[value=/^@\\//]',
|
||||
message:
|
||||
@@ -362,6 +385,95 @@ const eslintConfig = defineConfig([
|
||||
],
|
||||
},
|
||||
},
|
||||
// Single LLM entry point (machine-enforced): server-side model calls go through
|
||||
// `callLLM` / `streamLLM` in lib/ai/llm.ts. That wrapper is where usage
|
||||
// accounting (`recordUsage`), the `LLM_THINKING_DISABLED` kill switch, and
|
||||
// per-provider thinking resolution live — a direct `generateText` / `streamText`
|
||||
// silently opts out of all three, and the opt-out is invisible at the call site.
|
||||
// The PBL v2 runtime drifted exactly this way (#1003): five direct calls meant
|
||||
// zero usage records for the busiest traffic in the product, plus three
|
||||
// different meanings for one thinking config.
|
||||
//
|
||||
// The rule uses the @typescript-eslint variant deliberately: the base
|
||||
// `no-restricted-imports` is already configured for lib/choreography and
|
||||
// lib/video-export, and flat config REPLACES a rule's options per key rather
|
||||
// than merging them, so reusing that key here would silently drop those
|
||||
// module-boundary bans. Different key, no interference.
|
||||
//
|
||||
// Every reachable import form is covered, verified by feeding each one to
|
||||
// eslint rather than assumed:
|
||||
// - `import { streamText } from 'ai'` → this rule
|
||||
// - `import * as ai from 'ai'` → this rule too; ESLint reports a
|
||||
// namespace import when `importNames` is set, since the namespace would
|
||||
// carry the restricted name
|
||||
// - `require('ai')` → the repo-wide
|
||||
// `@typescript-eslint/no-require-imports` (inherited from
|
||||
// eslint-config-next/typescript) already forbids require() anywhere
|
||||
// - `await import('ai')` → the no-restricted-syntax block below
|
||||
// An `eslint-disable` comment defeats any of them, which is the point: the
|
||||
// bypass has to be written down where a reviewer sees it.
|
||||
//
|
||||
// Scope is every linted source extension, matching the module-boundary blocks
|
||||
// above — NOT just ts/tsx. An earlier revision guarded only ts/tsx, which left
|
||||
// `app/api/route.js` and `scripts/*.mjs` free to import the SDK; review caught
|
||||
// it. tests/lint-llm-entry-guard.test.ts pins the whole matrix so the scope
|
||||
// cannot quietly narrow again.
|
||||
{
|
||||
files: ['**/*.{ts,tsx,js,jsx,mjs,cjs}'],
|
||||
ignores: [
|
||||
// The entry point itself.
|
||||
'lib/ai/llm.ts',
|
||||
// Offline harnesses and a test whose subject IS the SDK: not server request
|
||||
// paths, nothing to account for, and the integration test must be able to
|
||||
// call the raw SDK to assert what the wrapper is built on.
|
||||
'eval/**',
|
||||
'tests/**',
|
||||
],
|
||||
rules: {
|
||||
'@typescript-eslint/no-restricted-imports': [
|
||||
'error',
|
||||
{
|
||||
paths: [
|
||||
{
|
||||
name: 'ai',
|
||||
importNames: ['generateText', 'streamText'],
|
||||
message:
|
||||
'Call the model through callLLM / streamLLM in @/lib/ai/llm instead of the AI SDK directly — that is where usage accounting, the LLM_THINKING_DISABLED kill switch, and per-provider thinking resolution are applied. Both wrappers pass every SDK option straight through, and take an optional per-call thinking config.',
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
// Same boundary, dynamic form. This block cannot match the files of the five
|
||||
// blocks above that also set no-restricted-syntax (flat config replaces rule
|
||||
// options per key, so it would drop their module boundaries), hence the ignores.
|
||||
// Every ignored directory is nonetheless covered, and covered by a rule rather
|
||||
// than by an argument:
|
||||
// - packages/@openmaic/renderer, packages/@openmaic/storage — the same
|
||||
// AI_SDK_DYNAMIC_IMPORT_BAN is spread into their own blocks above. An earlier
|
||||
// revision left them out on the reasoning that they are built in isolation
|
||||
// against @openmaic/dsl; review showed `void import('ai')` under the renderer
|
||||
// source path passing lint, which is exactly why that reasoning was not good
|
||||
// enough.
|
||||
// - lib/choreography, lib/video-export — their blocks ban EVERY
|
||||
// ImportExpression outright, which subsumes this one.
|
||||
{
|
||||
files: ['**/*.{ts,tsx,js,jsx,mjs,cjs}'],
|
||||
ignores: [
|
||||
'lib/ai/llm.ts',
|
||||
'eval/**',
|
||||
'tests/**',
|
||||
// Blocks above that configure no-restricted-syntax for their own boundary.
|
||||
'lib/choreography/**',
|
||||
'lib/video-export/**',
|
||||
'packages/@openmaic/renderer/**',
|
||||
'packages/@openmaic/storage/**',
|
||||
],
|
||||
rules: {
|
||||
'no-restricted-syntax': ['error', ...AI_SDK_DYNAMIC_IMPORT_BAN],
|
||||
},
|
||||
},
|
||||
]);
|
||||
|
||||
export default eslintConfig;
|
||||
|
||||
+17
-2
@@ -212,7 +212,12 @@ function buildThinkingProviderOptions(
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve providerOptions for direct AI SDK calls that bypass callLLM/streamLLM.
|
||||
* Resolve providerOptions the way callLLM / streamLLM resolve them internally.
|
||||
*
|
||||
* There are no production callers left: every server-side call goes through the
|
||||
* wrappers (a lint rule enforces it), and they inject provider options
|
||||
* themselves. Kept exported because it is the only way to inspect that mapping
|
||||
* from the outside, which the SDK-integration tests do.
|
||||
*/
|
||||
export function resolveThinkingProviderOptions(
|
||||
model: LanguageModel,
|
||||
@@ -343,6 +348,17 @@ export async function callLLM<T extends GenerateTextParams>(
|
||||
generateText(injectedParams),
|
||||
);
|
||||
|
||||
// Record before validating: every attempt that got this far was billed,
|
||||
// including one that fails validation below and one that is handed back
|
||||
// after the retries are exhausted. Recording on the success path only
|
||||
// would drop both.
|
||||
//
|
||||
// `usage` is the LAST step only; on a multi-step tool run (`stopWhen`)
|
||||
// every earlier step would go unaccounted. `totalUsage` aggregates across
|
||||
// steps and equals `usage` for a single-step call. Mirrors streamLLM,
|
||||
// which already prefers the aggregate.
|
||||
recordUsageSafe(result.totalUsage ?? result.usage, buildUsageMeta(params, source));
|
||||
|
||||
// Validate result (only when retries are configured)
|
||||
if (validate && !validate(result.text)) {
|
||||
log.warn(
|
||||
@@ -352,7 +368,6 @@ export async function callLLM<T extends GenerateTextParams>(
|
||||
continue;
|
||||
}
|
||||
|
||||
recordUsageSafe(result.usage, buildUsageMeta(params, source));
|
||||
return result;
|
||||
} catch (error) {
|
||||
lastError = error;
|
||||
|
||||
@@ -41,12 +41,10 @@
|
||||
* persist).
|
||||
*/
|
||||
|
||||
import { streamText } from 'ai';
|
||||
import type { LanguageModel } from 'ai';
|
||||
|
||||
import { createLogger } from '@/lib/logger';
|
||||
import { resolveThinkingProviderOptions } from '@/lib/ai/llm';
|
||||
import { withThinkingDisabled } from './runtime-thinking';
|
||||
import { streamLLM } from '@/lib/ai/llm';
|
||||
import { buildVisionUserContent } from '@/lib/generation/prompt-formatters';
|
||||
import type { ThinkingConfig } from '@/lib/types/provider';
|
||||
|
||||
@@ -154,8 +152,8 @@ async function* runShared(args: RunSharedArgs): AsyncGenerator<PBLSSEEvent, void
|
||||
const isMilestone = kind === 'milestone';
|
||||
let lastSanitizedLength = 0;
|
||||
try {
|
||||
const result = withThinkingDisabled(() =>
|
||||
streamText({
|
||||
const result = streamLLM(
|
||||
{
|
||||
model: languageModel,
|
||||
system: systemPrompt,
|
||||
// Image submission on a vision-capable model → send the picture as a
|
||||
@@ -173,11 +171,10 @@ async function* runShared(args: RunSharedArgs): AsyncGenerator<PBLSSEEvent, void
|
||||
],
|
||||
}
|
||||
: { prompt: userPrompt }),
|
||||
...(thinkingConfig
|
||||
? { providerOptions: resolveThinkingProviderOptions(languageModel, thinkingConfig) }
|
||||
: {}),
|
||||
...(signal ? { abortSignal: signal } : {}),
|
||||
}),
|
||||
},
|
||||
`pbl-v2-evaluator-${kind}`,
|
||||
thinkingConfig,
|
||||
);
|
||||
for await (const part of result.fullStream) {
|
||||
switch (part.type) {
|
||||
|
||||
@@ -11,16 +11,15 @@
|
||||
* owned by the right-side submission/evaluation flow, not by chat.
|
||||
*/
|
||||
|
||||
import { streamText, tool, stepCountIs } from 'ai';
|
||||
import { tool, stepCountIs } from 'ai';
|
||||
import type { LanguageModel } from 'ai';
|
||||
|
||||
import { createLogger } from '@/lib/logger';
|
||||
import { resolveThinkingProviderOptions } from '@/lib/ai/llm';
|
||||
import { streamLLM } from '@/lib/ai/llm';
|
||||
import { loadPBLV2Prompt } from '../prompts/loader';
|
||||
import type { ThinkingConfig } from '@/lib/types/provider';
|
||||
import { tierGuidanceBlock } from './tier-guidance';
|
||||
import { compressIfNeeded } from './instructor-memory';
|
||||
import { withThinkingDisabled } from './runtime-thinking';
|
||||
|
||||
import type {
|
||||
PBLProjectV2,
|
||||
@@ -1497,22 +1496,31 @@ export async function* runInstructorTurn(
|
||||
// speaking, leaving the learner with an empty chat. The instructing path
|
||||
// exposes only the two non-advance tools (record_observation /
|
||||
// adjust_difficulty).
|
||||
const result = withThinkingDisabled(() =>
|
||||
streamText({
|
||||
const result = streamLLM(
|
||||
{
|
||||
model: languageModel,
|
||||
system: systemPrompt,
|
||||
messages: finalMessages,
|
||||
// SCENARIO ONLY: prep-stage turns are pure Q&A — expose NO tools so the
|
||||
// model cannot record/advance; advancing the prep stage is the sidebar
|
||||
// "enter scenario" button's job.
|
||||
// Never force a tool choice here. Measured against the live DeepSeek V4
|
||||
// Pro API: thinking on + these `tools` + `stopWhen` streams fine, but
|
||||
// thinking on + a FORCED `toolChoice` fails with 400 "Thinking mode does
|
||||
// not support this tool_choice". A forced tool would also take the
|
||||
// learner's turn away from the model, which the comment above is about.
|
||||
...(phase === 'instructing' && !scenarioPrepStage
|
||||
? { tools, stopWhen: stepCountIs(MAX_INSTRUCTOR_STEPS) }
|
||||
: {}),
|
||||
...(thinkingConfig
|
||||
? { providerOptions: resolveThinkingProviderOptions(languageModel, thinkingConfig) }
|
||||
: {}),
|
||||
...(signal ? { abortSignal: signal } : {}),
|
||||
}),
|
||||
},
|
||||
'pbl-v2-instructor',
|
||||
// Thinking is whatever the request / stage route asked for — the runtime
|
||||
// holds no opinion of its own, same as every other call in the tree. If a
|
||||
// deployment wants these turns cheap and snappy (on the same probe a
|
||||
// thinking-by-default model pushed the first token from ~1.4 s to ~3.0 s),
|
||||
// pin `thinking` off on the `pbl-v2-runtime` stage route.
|
||||
thinkingConfig,
|
||||
);
|
||||
|
||||
for await (const part of result.fullStream) {
|
||||
@@ -1770,8 +1778,8 @@ async function* runSetupFollowup(args: SetupFollowupArgs): AsyncGenerator<PBLSSE
|
||||
const historyMessages = buildSetupHistoryMessages(instructorThread);
|
||||
|
||||
try {
|
||||
const result = withThinkingDisabled(() =>
|
||||
streamText({
|
||||
const result = streamLLM(
|
||||
{
|
||||
model: languageModel,
|
||||
system: systemPrompt,
|
||||
messages: [
|
||||
@@ -1781,11 +1789,10 @@ async function* runSetupFollowup(args: SetupFollowupArgs): AsyncGenerator<PBLSSE
|
||||
content: syntheticPlatformOpener('setup', project.language),
|
||||
},
|
||||
],
|
||||
...(thinkingConfig
|
||||
? { providerOptions: resolveThinkingProviderOptions(languageModel, thinkingConfig) }
|
||||
: {}),
|
||||
...(signal ? { abortSignal: signal } : {}),
|
||||
}),
|
||||
},
|
||||
'pbl-v2-instructor-setup-followup',
|
||||
thinkingConfig,
|
||||
);
|
||||
|
||||
let rawAssistantText = '';
|
||||
|
||||
@@ -1,28 +0,0 @@
|
||||
import { thinkingContext } from '@/lib/ai/thinking-context';
|
||||
import type { ThinkingConfig } from '@/lib/types/provider';
|
||||
|
||||
/**
|
||||
* PBL v2 runtime LLM calls force-disable thinking.
|
||||
*
|
||||
* The instructing turn forces `begin_turn` via `tool_choice`, which several
|
||||
* providers reject when thinking is on — DeepSeek returns 400 "Thinking mode
|
||||
* does not support this tool_choice". We never intentionally enabled thinking
|
||||
* on these turns (the PBL v2 client sends no `thinkingConfig`); some pinned
|
||||
* models just default it on. Disabling it removes the incompatibility without
|
||||
* losing any behavior we relied on.
|
||||
*
|
||||
* For OpenAI-compatible providers (e.g. DeepSeek) thinking is injected by the
|
||||
* fetch wrapper in `providers.ts`, which reads the per-request config from the
|
||||
* `thinkingContext` AsyncLocalStorage. The agents call the AI SDK directly
|
||||
* (not via `callLLM`/`streamLLM`), so nothing seeds that store — we do it here.
|
||||
* The SDK call must be started INSIDE `run` (its consumption can be outside) so
|
||||
* the lazily-issued fetch inherits the context, matching `streamLLM`.
|
||||
*
|
||||
* Scope: PBL v2 runtime only (instructor / evaluator). Generation (planner) is
|
||||
* intentionally untouched.
|
||||
*/
|
||||
const PBL_V2_THINKING_DISABLED: ThinkingConfig = { mode: 'disabled', enabled: false };
|
||||
|
||||
export function withThinkingDisabled<T>(startCall: () => T): T {
|
||||
return thinkingContext.run(PBL_V2_THINKING_DISABLED, startCall);
|
||||
}
|
||||
@@ -23,11 +23,10 @@
|
||||
* intentionally NOT here — they land in increment 4.
|
||||
*/
|
||||
|
||||
import { streamText, generateText } from 'ai';
|
||||
import type { LanguageModel } from 'ai';
|
||||
|
||||
import { createLogger } from '@/lib/logger';
|
||||
import { resolveThinkingProviderOptions } from '@/lib/ai/llm';
|
||||
import { callLLM, streamLLM } from '@/lib/ai/llm';
|
||||
import type { ThinkingConfig } from '@/lib/types/provider';
|
||||
import { loadPBLV2Prompt } from '../prompts/loader';
|
||||
|
||||
@@ -263,7 +262,17 @@ async function runDirectorNarratorPass(args: {
|
||||
* warns against re-describing what the prior act already covered. */
|
||||
firstEntry?: boolean;
|
||||
}): Promise<string[]> {
|
||||
const { project, milestone, microtask, phase, thread, languageModel, signal, firstEntry } = args;
|
||||
const {
|
||||
project,
|
||||
milestone,
|
||||
microtask,
|
||||
phase,
|
||||
thread,
|
||||
languageModel,
|
||||
thinkingConfig,
|
||||
signal,
|
||||
firstEntry,
|
||||
} = args;
|
||||
const system = buildNarratorSystemPrompt(project, milestone, microtask);
|
||||
const history = buildSimulatorHistory(thread, 'director');
|
||||
const greetingNudge = firstEntry
|
||||
@@ -278,12 +287,21 @@ async function runDirectorNarratorPass(args: {
|
||||
const messages = [...history, { role: 'user' as const, content: nudge }];
|
||||
|
||||
try {
|
||||
const result = await generateText({
|
||||
model: languageModel,
|
||||
system,
|
||||
messages,
|
||||
...(signal ? { abortSignal: signal } : {}),
|
||||
});
|
||||
const result = await callLLM(
|
||||
{
|
||||
model: languageModel,
|
||||
system,
|
||||
messages,
|
||||
...(signal ? { abortSignal: signal } : {}),
|
||||
},
|
||||
'pbl-v2-simulator-narrator',
|
||||
undefined,
|
||||
// Same contract as every other site: the request / stage route decides.
|
||||
// This pass used to drop the config on the floor — its callers passed one
|
||||
// and the helper never forwarded it, so the provider silently fell back to
|
||||
// the model default.
|
||||
thinkingConfig,
|
||||
);
|
||||
const text = (result.text ?? '').trim();
|
||||
if (!text) return [];
|
||||
// Sentinel: model says nothing happened → no narration this turn.
|
||||
@@ -500,15 +518,22 @@ export async function* runSimulatorTurn(
|
||||
// so the caller can decide retry vs surface.
|
||||
async function* streamCharacterLine(): AsyncGenerator<PBLSSEEvent, string, void> {
|
||||
let acc = '';
|
||||
const stream = streamText({
|
||||
model: languageModel,
|
||||
system,
|
||||
messages,
|
||||
...(thinkingConfig
|
||||
? { providerOptions: resolveThinkingProviderOptions(languageModel, thinkingConfig) }
|
||||
: {}),
|
||||
...(signal ? { abortSignal: signal } : {}),
|
||||
});
|
||||
// The incoming per-request / stage-route config is what applies, handed to
|
||||
// the wrapper: it resolves provider options for native adapters AND seeds the
|
||||
// thinking context that the OpenAI-compatible fetch wrapper reads. The
|
||||
// hand-rolled `resolveThinkingProviderOptions` call this replaced only ever
|
||||
// covered the former, so a stage-route config was silently dropped on
|
||||
// OpenAI-compatible providers.
|
||||
const stream = streamLLM(
|
||||
{
|
||||
model: languageModel,
|
||||
system,
|
||||
messages,
|
||||
...(signal ? { abortSignal: signal } : {}),
|
||||
},
|
||||
'pbl-v2-simulator',
|
||||
thinkingConfig,
|
||||
);
|
||||
for await (const part of stream.fullStream) {
|
||||
if (part.type === 'text-delta') {
|
||||
const delta =
|
||||
|
||||
@@ -90,6 +90,15 @@ export async function recordUsage(
|
||||
input: UsageRecordInput,
|
||||
opts: RecordOptions = {},
|
||||
): Promise<void> {
|
||||
// A test run must never append to the app's real usage log. Any test that
|
||||
// exercises callLLM / streamLLM reaches this through `recordUsageSafe` without
|
||||
// asking for it, and used to write rows into the live `data/usage/` file —
|
||||
// `minimax-auth-test`, `serialization-test` and friends were sitting in there
|
||||
// next to production traffic, corrupting any real usage analysis. Tests that
|
||||
// mean to exercise storage pass an explicit `baseDir` (or mock this module),
|
||||
// so both of those keep working.
|
||||
if (!opts.baseDir && (process.env.VITEST || process.env.NODE_ENV === 'test')) return;
|
||||
|
||||
try {
|
||||
const kind: UsageKind = input.kind ?? 'llm';
|
||||
const usage = input.usage ?? ZERO_USAGE;
|
||||
|
||||
@@ -1,11 +1,16 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
const aiMock = vi.hoisted(() => ({
|
||||
generateText: vi.fn(async (params: unknown) => ({
|
||||
text: 'ok',
|
||||
params,
|
||||
usage: undefined as unknown,
|
||||
})),
|
||||
// `totalUsage` is optional here so a single-step case can omit it: multi-step
|
||||
// runs report the aggregate there and callLLM prefers it over `usage`.
|
||||
generateText: vi.fn(
|
||||
async (
|
||||
params: unknown,
|
||||
): Promise<{ text: string; params: unknown; usage?: unknown; totalUsage?: unknown }> => ({
|
||||
text: 'ok',
|
||||
params,
|
||||
}),
|
||||
),
|
||||
streamText: vi.fn(),
|
||||
}));
|
||||
|
||||
@@ -116,6 +121,134 @@ describe('LLM thinking provider options', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('records the aggregate usage of a multi-step tool run, not the last step', async () => {
|
||||
// `usage` on a multi-step run (`stopWhen`) is the final step alone; every
|
||||
// earlier step's tokens live only in `totalUsage`.
|
||||
aiMock.generateText.mockResolvedValueOnce({
|
||||
text: 'ok',
|
||||
params: undefined,
|
||||
usage: { inputTokens: 3, outputTokens: 4 },
|
||||
totalUsage: { inputTokens: 30, outputTokens: 40 },
|
||||
});
|
||||
|
||||
await callLLM(
|
||||
{
|
||||
model: {
|
||||
provider: 'openai.responses',
|
||||
modelId: 'gpt-5.6',
|
||||
},
|
||||
prompt: 'hi',
|
||||
} as Parameters<typeof callLLM>[0],
|
||||
'test',
|
||||
);
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(usageMock.recordUsage).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ usage: { inputTokens: 30, outputTokens: 40 } }),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
it('records every attempt when a retry is configured, not just the accepted one', async () => {
|
||||
// Both attempts hit the provider and were billed; the first one just failed
|
||||
// validation. Recording on the success path only would drop it.
|
||||
aiMock.generateText
|
||||
.mockResolvedValueOnce({
|
||||
text: '',
|
||||
params: undefined,
|
||||
totalUsage: { inputTokens: 7, outputTokens: 0 },
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
text: 'ok',
|
||||
params: undefined,
|
||||
totalUsage: { inputTokens: 9, outputTokens: 2 },
|
||||
});
|
||||
|
||||
await callLLM(
|
||||
{
|
||||
model: {
|
||||
provider: 'openai.responses',
|
||||
modelId: 'gpt-5.6',
|
||||
},
|
||||
prompt: 'hi',
|
||||
} as Parameters<typeof callLLM>[0],
|
||||
'test',
|
||||
{ retries: 1 },
|
||||
);
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(usageMock.recordUsage).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
// Nested objectContaining: the recorded usage may carry normalised
|
||||
// zero-valued fields alongside the two this test set.
|
||||
expect(usageMock.recordUsage).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
usage: expect.objectContaining({ inputTokens: 7, outputTokens: 0 }),
|
||||
}),
|
||||
);
|
||||
expect(usageMock.recordUsage).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
usage: expect.objectContaining({ inputTokens: 9, outputTokens: 2 }),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('records the last attempt even when every attempt fails validation', async () => {
|
||||
// `Once` twice rather than a persistent implementation: beforeEach only
|
||||
// clears calls, so a lingering mockResolvedValue would leak into later tests.
|
||||
const failed = {
|
||||
text: '',
|
||||
params: undefined,
|
||||
totalUsage: { inputTokens: 5, outputTokens: 0 },
|
||||
};
|
||||
aiMock.generateText.mockResolvedValueOnce(failed).mockResolvedValueOnce(failed);
|
||||
|
||||
await callLLM(
|
||||
{
|
||||
model: {
|
||||
provider: 'openai.responses',
|
||||
modelId: 'gpt-5.6',
|
||||
},
|
||||
prompt: 'hi',
|
||||
} as Parameters<typeof callLLM>[0],
|
||||
'test',
|
||||
{ retries: 1 },
|
||||
);
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(usageMock.recordUsage).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
});
|
||||
|
||||
// Defensive: ai@6's GenerateTextResult always carries `totalUsage`, so this
|
||||
// shape is not one the current SDK produces — the fallback exists so a future
|
||||
// result shape without the aggregate degrades to the final step instead of
|
||||
// recording nothing.
|
||||
it('falls back to the single-step usage when no aggregate is reported', async () => {
|
||||
aiMock.generateText.mockResolvedValueOnce({
|
||||
text: 'ok',
|
||||
params: undefined,
|
||||
usage: { inputTokens: 3, outputTokens: 4 },
|
||||
});
|
||||
|
||||
await callLLM(
|
||||
{
|
||||
model: {
|
||||
provider: 'openai.responses',
|
||||
modelId: 'gpt-5.6',
|
||||
},
|
||||
prompt: 'hi',
|
||||
} as Parameters<typeof callLLM>[0],
|
||||
'test',
|
||||
);
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(usageMock.recordUsage).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ usage: { inputTokens: 3, outputTokens: 4 } }),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
it('sends Claude Haiku 4.5 thinking budget without effort', async () => {
|
||||
await callLLM(
|
||||
{
|
||||
|
||||
@@ -0,0 +1,105 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { ESLint } from 'eslint';
|
||||
|
||||
/**
|
||||
* Executable contract for the single-LLM-entry-point lint guard (#1003).
|
||||
*
|
||||
* The guard is the thing that keeps `generateText` / `streamText` reachable only
|
||||
* through `callLLM` / `streamLLM` in `lib/ai/llm.ts`. Its weak spot is not the
|
||||
* rules — it is their SCOPE, and review found three separate holes in it: two
|
||||
* `@openmaic` package directories excluded from the dynamic-import ban, and both
|
||||
* blocks matching only `ts,tsx` so a `route.js` or a `scripts/*.mjs` could import
|
||||
* the SDK freely.
|
||||
*
|
||||
* Each hole was closed and then verified by hand, which is exactly the process
|
||||
* that produced the next hole. So the matrix lives here instead: every import
|
||||
* form × every source extension × the production paths that must be covered and
|
||||
* the paths that are deliberately exempt. A future edit that narrows `files`,
|
||||
* adds an `ignores` entry, or swaps the rule key fails this test rather than
|
||||
* quietly reopening the door.
|
||||
*
|
||||
* Runs the real eslint against the real `eslint.config.mjs` on in-memory text, so
|
||||
* there is no fixture drift and nothing is written to the tree.
|
||||
*/
|
||||
|
||||
const BYPASS_FORMS = {
|
||||
named: "import { streamText } from 'ai';\nexport const a = streamText;\n",
|
||||
namespace: "import * as ai from 'ai';\nexport const a = ai.generateText;\n",
|
||||
dynamic: "export async function a() {\n return import('ai');\n}\n",
|
||||
dynamicTemplate: 'export async function a() {\n return import(`ai`);\n}\n',
|
||||
} as const;
|
||||
|
||||
/** Production paths: an SDK import here must be an error. */
|
||||
const GUARDED_PATHS = [
|
||||
'lib/pbl/v2/agents/probe',
|
||||
'lib/server/probe',
|
||||
'app/api/probe/route',
|
||||
'components/probe',
|
||||
'packages/@openmaic/renderer/src/probe',
|
||||
'packages/@openmaic/storage/src/probe',
|
||||
'lib/choreography/probe',
|
||||
'lib/video-export/probe',
|
||||
'scripts/probe',
|
||||
] as const;
|
||||
|
||||
/** Deliberate exemptions: the entry point itself, offline harnesses, and tests. */
|
||||
const EXEMPT_PATHS = ['lib/ai/llm', 'eval/probe', 'tests/probe'] as const;
|
||||
|
||||
const EXTENSIONS = ['ts', 'tsx', 'js', 'jsx', 'mjs', 'cjs'] as const;
|
||||
|
||||
const eslint = new ESLint({ cwd: process.cwd() });
|
||||
|
||||
async function errorsFor(filePath: string, code: string): Promise<string[]> {
|
||||
// A path ESLint would ignore (build output, vendored trees) returns no result;
|
||||
// treat that as "not covered" so an ignored path can never look like a pass.
|
||||
if (await eslint.isPathIgnored(filePath)) return ['<path is eslint-ignored>'];
|
||||
const [result] = await eslint.lintText(code, { filePath, warnIgnored: false });
|
||||
return (result?.messages ?? []).filter((m) => m.severity === 2).map((m) => m.ruleId ?? 'unknown');
|
||||
}
|
||||
|
||||
describe('LLM entry-point lint guard — coverage matrix', () => {
|
||||
for (const [form, code] of Object.entries(BYPASS_FORMS)) {
|
||||
for (const ext of EXTENSIONS) {
|
||||
// A namespace or named import of a value is not valid in a .cjs/.mjs mix
|
||||
// only in principle; eslint parses all of these fine, so no exclusions.
|
||||
it(`blocks the ${form} import in every guarded path (.${ext})`, async () => {
|
||||
for (const base of GUARDED_PATHS) {
|
||||
const filePath = `${base}.${ext}`;
|
||||
const errors = await errorsFor(filePath, code);
|
||||
expect(errors, `${filePath} should reject the ${form} form`).not.toHaveLength(0);
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
it('leaves the entry point, eval harnesses and tests exempt', async () => {
|
||||
for (const base of EXEMPT_PATHS) {
|
||||
for (const [form, code] of Object.entries(BYPASS_FORMS)) {
|
||||
const filePath = `${base}.ts`;
|
||||
const errors = await errorsFor(filePath, code);
|
||||
expect(errors, `${filePath} should allow the ${form} form`).toEqual([]);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it('still enforces the pre-existing module boundaries it shares a rule key with', async () => {
|
||||
// The dynamic-import ban uses `no-restricted-syntax`, a key several boundary
|
||||
// blocks configure. Flat config REPLACES rule options per key, so a careless
|
||||
// edit to the guard silently deletes those boundaries — assert two of them.
|
||||
const hostPathImport = "import x from '@/lib/foo';\nexport default x;\n";
|
||||
for (const filePath of [
|
||||
'packages/@openmaic/renderer/src/probe.ts',
|
||||
'packages/@openmaic/storage/src/probe.ts',
|
||||
'lib/choreography/probe.ts',
|
||||
]) {
|
||||
const errors = await errorsFor(filePath, hostPathImport);
|
||||
expect(errors, `${filePath} must still reject a host-app @/ import`).toContain(
|
||||
'no-restricted-syntax',
|
||||
);
|
||||
}
|
||||
|
||||
const reactImport = "import 'react';\nexport const a = 1;\n";
|
||||
const choreographyErrors = await errorsFor('lib/choreography/probe.ts', reactImport);
|
||||
expect(choreographyErrors).toContain('no-restricted-imports');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,309 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { MockLanguageModelV3, convertArrayToReadableStream } from 'ai/test';
|
||||
|
||||
/**
|
||||
* #1003: the PBL v2 runtime reaches the model only through the shared entry
|
||||
* point, and holds no thinking opinion of its own.
|
||||
*
|
||||
* The runtime used to force thinking off on the teaching turns through a
|
||||
* hand-rolled helper that seeded the thinking AsyncLocalStorage directly — which
|
||||
* is also why those turns called the AI SDK directly. Both are gone: the agents
|
||||
* call `streamLLM` / `callLLM`, and what the request / stage route asked for is
|
||||
* what the provider sees.
|
||||
*
|
||||
* Asserted at the provider boundary, where a regression would actually bite:
|
||||
*
|
||||
* 1. an incoming thinking config arrives intact,
|
||||
* 2. NO config means NO config — nothing in the runtime substitutes one, which
|
||||
* is the property a re-added policy constant would silently break,
|
||||
* 3. the global kill switch still applies,
|
||||
* 4. the turn is accounted for; going through the wrapper is what makes the
|
||||
* call visible to usage recording at all.
|
||||
*/
|
||||
const usageMock = vi.hoisted(() => ({
|
||||
normalizeUsage: vi.fn((usage: unknown) => usage),
|
||||
recordUsage: vi.fn(async () => undefined),
|
||||
}));
|
||||
|
||||
vi.mock('@/lib/usage/normalize', () => ({
|
||||
normalizeUsage: usageMock.normalizeUsage,
|
||||
}));
|
||||
|
||||
vi.mock('@/lib/server/usage-storage', () => ({
|
||||
recordUsage: usageMock.recordUsage,
|
||||
}));
|
||||
|
||||
import { thinkingContext } from '@/lib/ai/thinking-context';
|
||||
import { runTaskEvaluation } from '@/lib/pbl/v2/agents/evaluator';
|
||||
import { runSimulatorTurn } from '@/lib/pbl/v2/agents/simulator';
|
||||
import { addSubmission } from '@/lib/pbl/v2/operations/submission';
|
||||
import type { PBLProjectV2 } from '@/lib/pbl/v2/types';
|
||||
import type { PBLSSEEvent } from '@/lib/pbl/v2/api/sse';
|
||||
import type { ThinkingConfig } from '@/lib/types/provider';
|
||||
|
||||
type DoStreamConfig = NonNullable<
|
||||
NonNullable<ConstructorParameters<typeof MockLanguageModelV3>[0]>['doStream']
|
||||
>;
|
||||
type StreamResult = Extract<DoStreamConfig, { stream: unknown }>;
|
||||
type StreamPart = StreamResult['stream'] extends ReadableStream<infer P> ? P : never;
|
||||
|
||||
const USAGE = {
|
||||
inputTokens: { total: 12, noCache: 12, cacheRead: 0, cacheWrite: 0 },
|
||||
outputTokens: { total: 8, text: 8, reasoning: 0 },
|
||||
};
|
||||
|
||||
function textStep(text: string): StreamPart[] {
|
||||
return [
|
||||
{ type: 'stream-start', warnings: [] },
|
||||
{ type: 'text-start', id: 'p1' },
|
||||
{ type: 'text-delta', id: 'p1', delta: text },
|
||||
{ type: 'text-end', id: 'p1' },
|
||||
{ type: 'finish', finishReason: { unified: 'stop', raw: 'stop' }, usage: USAGE },
|
||||
];
|
||||
}
|
||||
|
||||
function mkProject(): PBLProjectV2 {
|
||||
return {
|
||||
uiPhase: 'workspace',
|
||||
title: 't',
|
||||
description: 'd',
|
||||
proficiency: 'intermediate',
|
||||
language: 'zh-CN',
|
||||
tags: [],
|
||||
status: 'active',
|
||||
roles: [],
|
||||
milestones: [
|
||||
{
|
||||
id: 'ms1',
|
||||
title: 'M1',
|
||||
status: 'completed',
|
||||
order: 0,
|
||||
microtasks: [
|
||||
{
|
||||
id: 't1',
|
||||
title: 'T1',
|
||||
status: 'completed',
|
||||
assignee: 'user',
|
||||
hints: [],
|
||||
order: 0,
|
||||
},
|
||||
],
|
||||
documents: [],
|
||||
},
|
||||
],
|
||||
submissions: [],
|
||||
evaluations: [],
|
||||
threads: [],
|
||||
engagementEvents: [],
|
||||
createdAt: 'ts',
|
||||
updatedAt: 'ts',
|
||||
};
|
||||
}
|
||||
|
||||
/** SCENARIO fixture, trimmed from simulator.test.ts — enough for one roleplay
|
||||
* turn, which fires BOTH simulator provider calls (character line + narrator). */
|
||||
function scenarioProject(): PBLProjectV2 {
|
||||
return {
|
||||
language: 'zh-CN',
|
||||
roles: [{ id: 'role-i', type: 'instructor', name: '教练' }],
|
||||
threads: [{ agentId: 'role-i', messages: [] }],
|
||||
updatedAt: 'ts',
|
||||
scenario: {
|
||||
setting: '校园咖啡馆的午后',
|
||||
goal: '练习倾听与共情',
|
||||
learnerRole: '你是林夏的好朋友',
|
||||
characters: [{ id: 'c1', name: '林夏', persona: '内向', situation: '情绪低落' }],
|
||||
},
|
||||
milestones: [
|
||||
{
|
||||
id: 'ms-rp',
|
||||
title: '和林夏聊一聊',
|
||||
status: 'active',
|
||||
order: 0,
|
||||
documents: [],
|
||||
scenarioStage: 'roleplay',
|
||||
briefing: '你坐在咖啡馆里。',
|
||||
microtasks: [
|
||||
{
|
||||
id: 'beat-1',
|
||||
title: 'beat',
|
||||
status: 'in_progress',
|
||||
assignee: 'user',
|
||||
hints: [],
|
||||
order: 0,
|
||||
description: '林夏坐在你对面。',
|
||||
narration: '你们走进了一家安静的咖啡馆。',
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
evaluations: [],
|
||||
engagementEvents: [],
|
||||
} as unknown as PBLProjectV2;
|
||||
}
|
||||
|
||||
const NOT_CAPTURED = Symbol('provider was never invoked');
|
||||
|
||||
/** Run one task evaluation against a scripted model, capturing what the
|
||||
* provider saw in the thinking store at the moment it was invoked. */
|
||||
async function runEvaluation(thinkingConfig?: ThinkingConfig): Promise<{ seenThinking: unknown }> {
|
||||
const project = mkProject();
|
||||
addSubmission(project, {
|
||||
microtaskId: 't1',
|
||||
milestoneId: 'ms1',
|
||||
kind: 'text',
|
||||
content: 'submission',
|
||||
});
|
||||
|
||||
let seenThinking: unknown = NOT_CAPTURED;
|
||||
|
||||
const model = new MockLanguageModelV3({
|
||||
doStream: async () => {
|
||||
seenThinking = thinkingContext.getStore();
|
||||
return {
|
||||
stream: convertArrayToReadableStream(textStep('{"feedback":"ok","score":80}')),
|
||||
};
|
||||
},
|
||||
});
|
||||
|
||||
const events: PBLSSEEvent[] = [];
|
||||
for await (const ev of runTaskEvaluation({
|
||||
project,
|
||||
milestoneId: 'ms1',
|
||||
microtaskId: 't1',
|
||||
languageModel: model,
|
||||
...(thinkingConfig ? { thinkingConfig } : {}),
|
||||
})) {
|
||||
events.push(ev);
|
||||
}
|
||||
|
||||
// Guard the guard: an evaluation that errored out before calling the model
|
||||
// would make every assertion below vacuous.
|
||||
expect(seenThinking).not.toBe(NOT_CAPTURED);
|
||||
expect(events.at(-1)?.type).toBe('done');
|
||||
return { seenThinking };
|
||||
}
|
||||
|
||||
describe('PBL v2 runtime goes through the shared LLM entry point (#1003)', () => {
|
||||
beforeEach(() => {
|
||||
usageMock.normalizeUsage.mockClear();
|
||||
usageMock.recordUsage.mockClear();
|
||||
delete process.env.LLM_THINKING_DISABLED;
|
||||
});
|
||||
|
||||
it('passes an incoming thinking config through to the provider unchanged', async () => {
|
||||
const thinkingConfig: ThinkingConfig = { mode: 'enabled', effort: 'low' };
|
||||
const { seenThinking } = await runEvaluation(thinkingConfig);
|
||||
expect(seenThinking).toEqual(thinkingConfig);
|
||||
});
|
||||
|
||||
it('substitutes nothing when the request carries no thinking config', async () => {
|
||||
// The runtime holds no policy of its own. A re-added hardcoded constant
|
||||
// would show up right here as a config the caller never asked for.
|
||||
const { seenThinking } = await runEvaluation();
|
||||
expect(seenThinking).toBeUndefined();
|
||||
});
|
||||
|
||||
it('still honours the global kill switch when no config is supplied', async () => {
|
||||
process.env.LLM_THINKING_DISABLED = 'true';
|
||||
try {
|
||||
const { seenThinking } = await runEvaluation();
|
||||
expect(seenThinking).toEqual({ mode: 'disabled', enabled: false });
|
||||
} finally {
|
||||
delete process.env.LLM_THINKING_DISABLED;
|
||||
}
|
||||
});
|
||||
|
||||
it('does not leak the config past the turn', async () => {
|
||||
await runEvaluation({ mode: 'enabled', effort: 'low' });
|
||||
expect(thinkingContext.getStore()).toBeUndefined();
|
||||
});
|
||||
|
||||
it('accounts for the turn under its own usage source', async () => {
|
||||
await runEvaluation();
|
||||
await vi.waitFor(() => {
|
||||
expect(usageMock.recordUsage).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ kind: 'llm', source: 'pbl-v2-evaluator-task' }),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The simulator turn makes TWO provider calls — the streamed character line and
|
||||
* the non-streamed director-narrator pass. The narrator was the last site still
|
||||
* dropping the resolved config: both callers passed one in and the helper never
|
||||
* forwarded it, so the provider silently fell back to the model default. Caught
|
||||
* in review on #1006.
|
||||
*/
|
||||
describe('PBL v2 simulator turn — both provider calls see the resolved config', () => {
|
||||
beforeEach(() => {
|
||||
usageMock.normalizeUsage.mockClear();
|
||||
usageMock.recordUsage.mockClear();
|
||||
delete process.env.LLM_THINKING_DISABLED;
|
||||
});
|
||||
|
||||
async function runSimulator(thinkingConfig?: ThinkingConfig) {
|
||||
const seen: Array<{ kind: 'stream' | 'generate'; thinking: unknown }> = [];
|
||||
|
||||
const model = new MockLanguageModelV3({
|
||||
doGenerate: async () => {
|
||||
seen.push({ kind: 'generate', thinking: thinkingContext.getStore() });
|
||||
return {
|
||||
content: [{ type: 'text' as const, text: '她放下杯子,抬头看你。' }],
|
||||
finishReason: { unified: 'stop' as const, raw: 'stop' },
|
||||
usage: USAGE,
|
||||
warnings: [],
|
||||
};
|
||||
},
|
||||
doStream: async () => {
|
||||
seen.push({ kind: 'stream', thinking: thinkingContext.getStore() });
|
||||
return { stream: convertArrayToReadableStream(textStep('嗯……你来了。')) };
|
||||
},
|
||||
});
|
||||
|
||||
const events: PBLSSEEvent[] = [];
|
||||
for await (const ev of runSimulatorTurn({
|
||||
project: scenarioProject(),
|
||||
userMessage: '',
|
||||
phase: 'greeting',
|
||||
languageModel: model,
|
||||
...(thinkingConfig ? { thinkingConfig } : {}),
|
||||
})) {
|
||||
events.push(ev);
|
||||
}
|
||||
expect(events.at(-1)?.type).toBe('done');
|
||||
return seen;
|
||||
}
|
||||
|
||||
it('forwards an incoming config to the character line AND the narrator', async () => {
|
||||
const thinkingConfig: ThinkingConfig = { mode: 'enabled', effort: 'low' };
|
||||
const seen = await runSimulator(thinkingConfig);
|
||||
|
||||
// Both passes must have run, or this test would silently cover only one.
|
||||
expect(seen.map((s) => s.kind).sort()).toEqual(['generate', 'stream']);
|
||||
for (const call of seen) {
|
||||
expect(call.thinking).toEqual(thinkingConfig);
|
||||
}
|
||||
});
|
||||
|
||||
it('substitutes nothing on either call when the request carries no config', async () => {
|
||||
const seen = await runSimulator();
|
||||
expect(seen.map((s) => s.kind).sort()).toEqual(['generate', 'stream']);
|
||||
for (const call of seen) {
|
||||
expect(call.thinking).toBeUndefined();
|
||||
}
|
||||
});
|
||||
|
||||
it('accounts for both passes under their own usage sources', async () => {
|
||||
await runSimulator();
|
||||
await vi.waitFor(() => {
|
||||
expect(usageMock.recordUsage).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ source: 'pbl-v2-simulator' }),
|
||||
);
|
||||
expect(usageMock.recordUsage).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ source: 'pbl-v2-simulator-narrator' }),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -1,28 +0,0 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
|
||||
import { thinkingContext } from '@/lib/ai/thinking-context';
|
||||
import { withThinkingDisabled } from '@/lib/pbl/v2/agents/runtime-thinking';
|
||||
|
||||
/**
|
||||
* #669: PBL v2 runtime LLM calls must run with thinking disabled so a pinned
|
||||
* model whose thinking defaults on (e.g. DeepSeek) doesn't reject the forced
|
||||
* `begin_turn` tool_choice. The provider fetch wrapper reads the disabled
|
||||
* config from this AsyncLocalStorage, so the contract is "the store reads
|
||||
* disabled inside the wrapped call".
|
||||
*/
|
||||
describe('withThinkingDisabled (#669)', () => {
|
||||
it('seeds the thinking AsyncLocalStorage with a disabled config', () => {
|
||||
const seen = withThinkingDisabled(() => thinkingContext.getStore());
|
||||
expect(seen).toEqual({ mode: 'disabled', enabled: false });
|
||||
});
|
||||
|
||||
it('returns the wrapped callable result (so streamText/generateText pass through)', () => {
|
||||
const result = withThinkingDisabled(() => 'sentinel');
|
||||
expect(result).toBe('sentinel');
|
||||
});
|
||||
|
||||
it('does not leak the disabled config outside the wrapped call', () => {
|
||||
withThinkingDisabled(() => undefined);
|
||||
expect(thinkingContext.getStore()).toBeUndefined();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user