refactor(driver)!: delete the AgentDriver class and publish the served-model guard - #626
Conversation
…d-model guard AgentDriver was never on the public export surface: it is absent from the barrel and from every published subpath, so no consumer could import it. A sweep of nine sibling default branches and ten published npm dependents finds zero callers. Delete the class, its config interface, and the limbs that existed only to feed it — MetricsCollector, TurnMetrics, DriverResult. decideNextUserTurn stays: gtm-agent still calls it. Its guard against a substitute model answering a persona turn lives in assertServedModel, which the barrel did not export, so a consumer owning its own conversation loop could not keep that check. Export the served-model family so the migration off the packaged driver does not have to drop it. Repoint the remaining deprecation strings at #618.
…served-model guard
|
@tangletools review now |
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — b78d3738
This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.
This approval is provisional. It rests on the audit running. If the audit cannot run — for example the CLI bridge rejects it — this approval is dismissed rather than left standing, so an unrun check never reads as a passing one.
tangletools · auto-approval · reason: drewstone_author · 2026-08-16T18:57:43Z
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — b78d3738
This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.
This approval is provisional. It rests on the audit running. If the audit cannot run — for example the CLI bridge rejects it — this approval is dismissed rather than left standing, so an unrun check never reads as a passing one.
tangletools · auto-approval · reason: drewstone_author · 2026-08-16T18:57:48Z
tangletools
left a comment
There was a problem hiding this comment.
🟢 Value Audit — sound
| Verdict | sound |
| Coverage | 1 of 2 lenses (usefulness) |
| Concerns | 1 (1 weak-concern) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 214.1s (2 bridge agents) |
| Total | 214.1s |
⚠️ Partial audit — the verdict covers only usefulness. value: agent returned no usable verdict. Treat the missing lens as unexamined, not as clear.
💰 Value — error
value agent produced no parseable value-audit JSON.
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 2
- Bridge error: opencode/kimi-for-coding/k2p7: opencode: opencode error
🎯 Usefulness — sound
Deletes surface that provably never reached consumers and publishes the served-model guard an imminent migration needs — coherent, verified, in the grain of the repo.
- Integration: Verified end-to-end in the checkout. Deleted symbols (AgentDriver, AgentDriverConfig, MetricsCollector, TurnMetrics, DriverResult) leave zero dangling references: rg over the whole tree matches only CHANGELOG.md:15-16, none appear in src/index.ts, and package.json's 25-entry exports map has no ./driver or ./metrics subpath — so no released version could ever import them; deletion is safe. The kept
- Fit with existing patterns: Fits the established pattern exactly: public surface = barrel '.' plus curated subpaths; publishing assertServedModel as a primitive matches the substrate doctrine (honesty layer, fail-loud, no fallbacks) and the repo rule that substrate primitives live in agent-eval. The export set is coherent — error class (ModelSubstitutionError), policy function (servedModelAcceptable), classifier (checkServed
- Real-world viability: The published code is pure, branch-complete (exact/alias/within-family/cross-family/unreported), and already exercised by 5318-test suite internals; the touched test files pass here (46 tests), pnpm typecheck is clean, and the rotated analyst benchmark digests verify via pnpm check:analyst-benchmark (implementation e5af51a8…, lock 2ad623bf…). The guard defaults fail-closed (unreported ≠ pass) and
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
🎯 Usefulness Audit
🟡 Served-model family published only partially — assertCrossFamilyServed stays unreachable [integration] ``
src/index.ts:761-779 exports assertServedModel/assertServedModels/checkServedModel/servedModelAcceptable/ModelSubstitutionError + 3 types, but assertCrossFamilyServed, ServedCrossFamilyError, normalizeModelId, and PROBE_MAX_TOKENS have 0 occurrences in the barrel and no ./integrity subpath exists (verified against package.json exports). docs/building-doctrine.md:23 and src/model-seats.ts's docstring (asserted by src/model-seats.test.ts:156) name assertCrossFamilyServed as an enforcement mechanis
What this audit checks
It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.
| Pass | What it asks |
|---|---|
| Heuristic | Vague title? Whitespace-only or cruft-bearing diff? (content signals only) |
| Duplication | Do added function/class names already exist elsewhere in the repo? |
| Value Audit | What does it do? What goal does it achieve? Is it good? Better architecture or already-exists? |
| Usefulness Audit | Does it integrate and fit? Will it hold up in real use and actually get used? |
Findings are concerns, not blocks — the human reviewer decides what to do with them.
docs/building-doctrine.md names assertCrossFamilyServed as an enforcement mechanism while the barrel reached only half the module. A caller that can assert one served model but not a cross-family set has an unusable half of one primitive.
|
@tangletools review now |
tangletools
left a comment
There was a problem hiding this comment.
🟡 Value Audit — sound-with-nits
| Verdict | sound-with-nits |
| Coverage | 2 of 2 lenses (value, usefulness) |
| Concerns | 2 (2 weak-concern) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 269.9s (2 bridge agents) |
| Total | 269.9s |
💰 Value — sound-with-nits
Deletes the never-exported AgentDriver class and its private limbs (verified unexported pre-PR) while publishing the served-model guard family so the surviving deprecated driver function's future owners keep model-identity enforcement — clean, evidence-based substrate slimming with one leftover dead
- What it does: Removes the deprecated AgentDriver class plus the four types/modules that only served it (AgentDriverConfig, MetricsCollector, TurnMetrics, DriverResult), keeps the two still-referenced driver functions (decideNextUserTurn, buildDriverSystemPrompt) with deprecation pointers moved to agent-eval#618, and newly exports the served-model identity family (assertServedModel, checkServedModel, assertServe
- Goals it achieves: Shrinks the substrate's product-coupled surface per the repo's own layering doctrine (a one-product driver welded to ProductClient does not belong in agent-eval), and unblocks the eventual migration of decideNextUserTurn's one external caller: that function holds its transport to the requested model id via assertServedModel (src/driver.ts:178), which was previously unexported, so any harness takin
- Assessment: Good on its merits and unusually well-evidenced. I independently verified the load-bearing claims: pre-PR barrel (git show 8471168~1:src/index.ts) contains zero references to AgentDriver/AgentDriverConfig/MetricsCollector/TurnMetrics/DriverResult, package.json exposes only directory entry points (no driver/metrics subpath), and no subpath index re-exports driver.ts — so the deletion could not brea
- Better / existing approach: none — this is the right approach. I searched for an existing public equivalent of the guard (barrel + all 25 subpath entry points + LlmClientOptions) and found only the unexported module and its client-level opt-in wrapper; exporting the family is the minimal change. One finishing gap, filed below as a nit rather than a redirect.
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 2
- Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error
🎯 Usefulness — sound-with-nits
Deletes provably unreachable driver surface and publishes an already-internally-pervasive integrity primitive the migrating caller needs — verified clean and in the codebase's grain, with one orphaned class left behind.
- Integration: Deletion side verified safe: at base f99af3a the barrel exported only decideNextUserTurn/DecideNextUserTurnOpts from './driver' (src/index.ts:292-293) and only cost functions from './metrics' (src/index.ts:488-494); AgentDriver, AgentDriverConfig, MetricsCollector, TurnMetrics, DriverResult appear nowhere in the barrel, and package.json exposes only fixed dist entry subpaths — so no released consu
- Fit with existing patterns: Matches the repo's charter exactly: agent-eval is the substrate that holds verification primitives (CLAUDE.md: 'If a type that feels like it belongs in a consumer is actually a substrate primitive... move it INTO agent-eval'), and assertServedModel is already consumed by 8 internal modules (src/llm-client.ts:832, src/judge-panel.ts, src/judge-families.ts, src/completion-verifier.ts, src/model-seat
- Real-world viability: Verified working, not just claimed: pnpm typecheck and pnpm typecheck:examples clean; targeted vitest run (deprecation, driver, served-model, benchmark-implementation) 50/50 pass. The guard is exercised on the real driver path today (driver.ts:178 with allowUnreported for transports that echo no id) and errors extend AgentEvalError (src/integrity/served-model.ts:158,264), so it fails loud per repo
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
💰 Value Audit
🟡 ConvergenceTracker is left behind as unreachable dead code [maintenance] ``
The PR deletes the limbs that 'existed only to serve' AgentDriver, but src/convergence.ts (ConvergenceTracker) was in exactly that position — pre-PR its only importer was the deleted src/driver.ts:3 (verified via git grep at 8471168~1), and post-PR nothing imports it except its own test (tests/convergence.test.ts:2). It is not on the barrel (src/index.ts:550-551 exports only series-convergence). Delete it (and its test) in the #618 follow-up, or this sweep, to finish the limb removal the PR's ow
🎯 Usefulness Audit
🟡 ConvergenceTracker is orphaned by this deletion [integration] ``
At base, ConvergenceTracker's only consumer was the deleted AgentDriver (f99af3a:src/driver.ts:3,84 — verified via git grep). Post-change src/convergence.ts:9 has zero callers and was never on the barrel, so it is now dead internal surface — a fifth 'limb that existed only to serve' the class that the PR body's own logic names but the diff misses. Fold its deletion into the #618 follow-up that removes decideNextUserTurn/buildDriverSystemPrompt.
What this audit checks
It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.
| Pass | What it asks |
|---|---|
| Heuristic | Vague title? Whitespace-only or cruft-bearing diff? (content signals only) |
| Duplication | Do added function/class names already exist elsewhere in the repo? |
| Value Audit | What does it do? What goal does it achieve? Is it good? Better architecture or already-exists? |
| Usefulness Audit | Does it integrate and fit? Will it hold up in real use and actually get used? |
Findings are concerns, not blocks — the human reviewer decides what to do with them.
Part one of #618. It removes the part of the deprecated driver surface that provably has no callers, and it publishes the one primitive the remaining caller needs before its own removal can proceed.
Measured caller count
Method:
git grepon the default branch of nine sibling repositories, plus a tarball grep of the ten published npm packages that declare a dependency on this one at their current latest. Matches on unrelated identifiers (HostAgentDriverin agent-dev-container,RedTeamAgentDriverin starter-foundry) and on theMultishotShape.buildDriverSystemPromptcallback field — a caller-supplied property that shares the name but is not this export — are excluded.AgentDriverbuildDriverSystemPromptdecideNextUserTurneval/kernel.ts)AgentDrivercould not be imported by any consumer at any released version. It is deleted here with the limbs that existed only to serve it:AgentDriverConfig,MetricsCollector,TurnMetrics,DriverResult. None of the four reached the export surface either.decideNextUserTurnandbuildDriverSystemPromptstay until gtm-agent moves, which is the follow-up.Why the served-model guard ships with this
decideNextUserTurnholds its transport to the model id it was asked for: a persona turn written by a substitute model is not the requested model's behaviour, so it must not be scored as such. That check isassertServedModel, and the barrel did not export it. A consumer that takes ownership of its conversation loop therefore had to drop the check to migrate. The served-model family is now public, so the migration keeps it.Verification
pnpm typecheck,pnpm typecheck:examples,pnpm lint— clean.pnpm test— 5318 passed, 3 skipped, 381 files. A later rerun under concurrent load timed outreference-equivalence-judge.test.tsandrollout/reward-invariant.test.ts; both pass on rerun in 6.26s (70 tests), and neither touches this change.pnpm buildandpnpm run verify:package— exit 0.package.json,clients/python/pyproject.toml,clients/python/src/agent_eval_rpc/__init__.py,uv.lock. Analyst implementation and dependency-lock digests repinned.Refs #618.