Files
wyucandClaude Opus 5 97f8585728 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 of cc48ada.

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 since 9bbb720.

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>
2026-07-30 14:11:40 +08:00

212 lines
6.5 KiB
TypeScript

import { promises as fs } from 'fs';
import path from 'path';
import { createLogger } from '@/lib/logger';
import { hasBillableTokens, type NormalizedUsage } from '@/lib/usage/normalize';
const log = createLogger('UsageStorage');
/** Base directory for usage logs; lands in the openmaic-data volume in Docker. */
function usageDir(baseDir?: string): string {
return baseDir ?? path.join(process.cwd(), 'data', 'usage');
}
/** Current month's jsonl file name, e.g. usage/2026-06.jsonl. */
function monthlyFile(dir: string, now: Date): string {
const y = now.getUTCFullYear();
const m = String(now.getUTCMonth() + 1).padStart(2, '0');
return path.join(dir, `${y}-${m}.jsonl`);
}
/** What kind of generation produced this usage. */
export type UsageKind = 'llm' | 'image' | 'video' | 'tts' | 'asr';
/** Unit of the non-token quantity. */
export type UsageUnit = 'token' | 'image' | 'second' | 'character';
/** Input to record one generation's usage. */
export interface UsageRecordInput {
/** Modality. Defaults to 'llm'. */
kind?: UsageKind;
source: string;
providerId: string;
modelId: string;
modelString: string;
/** Token usage (LLM only). */
usage?: NormalizedUsage;
/** Non-token quantity: images count / seconds / characters. */
quantity?: number;
/** Unit for `quantity`. */
unit?: UsageUnit;
}
/** A persisted usage row — pure usage, no cost. */
export interface UsageRecord {
id: string;
createdAt: number;
kind: UsageKind;
source: string;
providerId: string;
modelId: string;
modelString: string;
// LLM token counts (0 for non-LLM rows).
inputTokens: number;
outputTokens: number;
cacheReadTokens: number;
cacheCreationTokens: number;
reasoningTokens: number;
// Non-token usage (e.g. image count, video seconds, TTS characters).
quantity?: number;
unit?: UsageUnit;
}
interface RecordOptions {
baseDir?: string;
/** Injected clock for deterministic tests. */
now?: Date;
}
let counter = 0;
function makeId(now: Date): string {
counter = (counter + 1) % 1_000_000;
return `${now.getTime()}-${counter.toString(36)}`;
}
const ZERO_USAGE: NormalizedUsage = {
inputTokens: 0,
outputTokens: 0,
cacheReadTokens: 0,
cacheCreationTokens: 0,
reasoningTokens: 0,
};
/**
* Records one generation's usage as a jsonl line. Fire-and-forget: never throws —
* a logging failure must not break generation.
*
* - LLM rows: require billable tokens (skips empty usage, e.g. a streamed
* OpenAI-compatible response that omitted usage).
* - Non-LLM rows (image/video/tts/asr): require quantity > 0.
*/
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;
if (kind === 'llm') {
if (!hasBillableTokens(usage)) return;
} else if (!input.quantity || input.quantity <= 0) {
return;
}
const now = opts.now ?? new Date();
const record: UsageRecord = {
id: makeId(now),
createdAt: now.getTime(),
kind,
source: input.source,
providerId: input.providerId,
modelId: input.modelId,
modelString: input.modelString,
inputTokens: usage.inputTokens,
outputTokens: usage.outputTokens,
cacheReadTokens: usage.cacheReadTokens,
cacheCreationTokens: usage.cacheCreationTokens,
reasoningTokens: usage.reasoningTokens,
...(input.quantity != null ? { quantity: input.quantity } : {}),
...(input.unit ? { unit: input.unit } : {}),
};
const dir = usageDir(opts.baseDir);
await fs.mkdir(dir, { recursive: true });
await fs.appendFile(monthlyFile(dir, now), JSON.stringify(record) + '\n', 'utf-8');
} catch (err) {
log.warn('Failed to record usage (ignored):', err);
}
}
/** A non-LLM modality usage event (image / video / tts / asr). */
export interface GenerationUsageInput {
kind: Exclude<UsageKind, 'llm'>;
unit: UsageUnit;
providerId: string;
/** The client-requested model id; falls back to providerId when absent. */
modelId?: string;
quantity: number;
}
/**
* Records a non-LLM generation's usage. Thin wrapper over {@link recordUsage}
* that derives `source` (= kind) and `modelString` (`provider:model`) from the
* modality, so the generate routes don't each repeat that construction.
* Fire-and-forget like `recordUsage`.
*/
export function recordGenerationUsage(input: GenerationUsageInput): Promise<void> {
const modelId = input.modelId || input.providerId;
return recordUsage({
kind: input.kind,
unit: input.unit,
source: input.kind,
providerId: input.providerId,
modelId,
modelString: `${input.providerId}:${modelId}`,
quantity: input.quantity,
});
}
interface ReadOptions {
baseDir?: string;
/** Limit to specific YYYY-MM month files; defaults to all files in the dir. */
months?: string[];
}
/**
* Reads all usage records (across monthly files). Returns [] when the dir is
* absent. Malformed lines are skipped. Legacy rows without `kind` are treated as
* 'llm'; any legacy cost fields are simply ignored.
*/
export async function readUsageRecords(opts: ReadOptions = {}): Promise<UsageRecord[]> {
const dir = usageDir(opts.baseDir);
let files: string[];
try {
files = (await fs.readdir(dir)).filter((f) => f.endsWith('.jsonl'));
} catch {
return [];
}
if (opts.months?.length) {
files = files.filter((f) => opts.months!.some((m) => f.startsWith(m)));
}
const records: UsageRecord[] = [];
for (const file of files.sort()) {
let content: string;
try {
content = await fs.readFile(path.join(dir, file), 'utf-8');
} catch {
continue;
}
for (const line of content.split('\n')) {
const trimmed = line.trim();
if (!trimmed) continue;
try {
const row = JSON.parse(trimmed) as UsageRecord;
if (!row.kind) row.kind = 'llm'; // backward-compat
records.push(row);
} catch {
// skip malformed line
}
}
}
return records;
}