worker: separate requested, resolved, observed and actually-used model identity - #73
Merged
Merged
Conversation
An adoption claim must be impossible when the model that actually served a run is unknown. Three separate facts were previously fused into one field: requested what the envelope asked for (route.initial_* -- was null on every path) resolved what the in-process resolver chose (message.model) observed what the provider reported on the wire (responseModel) `usage-accumulator.ts:157` and `task-outcome.ts:309` both collapse these to `responseModel ?? model`, so a consumer reading `usage[].model` cannot tell an observed agreement from a provider that reported nothing. `buildFallbackModel` makes that indistinguishability load-bearing: it returns another model's entire configuration relabelled with the requested id, so identity keyed on `model.id` reports the request back as if it were an observation. Carry observed and resolved as their own channels end to end, and emit `generation_identity` on the economics summary with a three-valued observation (observed / unavailable / conflict). `observed_model` is only ever a value the provider put on the wire; when nothing was observed it is null and `matches_requested` is null -- never false, and never back-filled. `usage[].model` keeps its existing collapsed meaning; other consumers read it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bNEs7T8pAdT7eJQTDgtkA
Two defects in the previous commit, both found by the existing suite: 1. `addVinciAccumulatedUsage` did `target.observedModelCalls += ...`. The adder is called from `combinedTaskUsage` with an object assembled in task-outcome that predates these fields, so `undefined += n` produced NaN -- which then travels as a plausible-looking number rather than failing. Sum through `finite()` on both sides instead. 2. `receipt-integration.mjs:302` asserts the accumulator rollup and the task-outcome rollup are deep-equal. Adding the fields to one side only made the two disagree. That assertion is a real consistency invariant, not a stale expectation, so task-outcome carries the same three fields -- which also closes the second collapse site (`task-outcome.ts:309`), where `responseModel ?? message.model` was fusing the same two facts for a different consumer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bNEs7T8pAdT7eJQTDgtkA
…guard Both controls exist because mutation testing found them missing, not because the code looked untested: - A mutation back-filling observed<-resolved inside task-outcome's rollup survived every existing case. That rollup is a second consumer (it feeds the receipt) and had no coverage at all. CASE 6 drives the real TypeScript module through jiti, the way the other extension tests do. - A mutation restoring the `+=` form of the adder also survived: giving VinciTaskUsage the field had already closed the only reachable NaN path, leaving the guard itself unfalsifiable. CASE 7 exercises the shape the guard actually defends -- a legacy target with no observed fields -- and fails with "observedModelCalls is NaN" when the guard is removed. Also drops a .vinci-worker-outbox/ test artifact that a test run left behind and that was committed by accident. That directory is not in .gitignore. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bNEs7T8pAdT7eJQTDgtkA
F3 (real defect, found against unmutated code). `session-read.mjs` took `observedModels[0]`, so ONE persisted entry carrying two distinct served ids -- which crew/helper rollups legitimately produce through the same recordVinciTaskUsage path -- reported a confident `observation: "observed"` with the second id dropped and both calls credited to the first. Carry the full sets through and let the aggregation see the disagreement. F2 (no coverage, proven by a surviving mutation). Every existing case started from the already-split persisted shape, so none reached `usageFromResponse` -- the function that actually performs the split. Reverting it to `responseModel || model` broke nothing. Export it and test the boundary directly. F4. The outer crash-fallback summary in `worker.mjs` omitted `generation_identity` entirely, contradicting the design intent that the field is always emitted so absence never has to be interpreted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bNEs7T8pAdT7eJQTDgtkA
The unit controls stopped at buildEconomicsSummary's return value. This drives the real worker daemon (`worker.mjs start --once`) against the fake bus and fixture binaries, and reads all four links back out of the artifact a consumer actually reads -- economics-summary.json on disk, whose sha256 the terminal bus post carries: envelope `model:` header -> route.initial_provider / initial_model (requested) session usage entry -> generation_identity.resolved_model (resolved) session usage entry -> generation_identity.observed_model (observed) terminal post + usage[] -> the digest and what was actually billed (used) The fixture reports a served model DIFFERENT from the requested one, so an implementation that echoes the request cannot pass; the second scenario reports nothing and proves the artifact still says `unavailable` while both wrong answers sit in the same file, copyable. Harness note recorded in the test: worker ids must come from the hardcoded list worker-fixture.mjs:259 seeds an ancient cursor for (w1..w8). Any other id makes the daemon's first run start its cursor at NOW and silently skip a handoff with a historical ts -- no claim, no task file, no economics -- which reads as a broken feature rather than an unseeded id. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bNEs7T8pAdT7eJQTDgtkA
… generation event
Producing the fields was not the deliverable. Nothing read generation_identity;
the only references were producers. Two real downstream surfaces now consume it
and carry the distinction into what they emit:
* the terminal bus post -- observation=, requested_model=, resolved_model=,
used_model=, used_generation_id[_sha256]=, identity_matches_requested=
(fields, never prose, per the existing postFinal convention). `used_model`
is emitted ONLY from a machine observation, so an unobservable identity
reads `unknown` and can never be mistaken for the requested string, and
identity_matches_requested is omitted rather than guessed when nothing was
measured.
* result.json in the evidence bundle, which is what a consumer of the bundle
reads rather than the economics summary.
Lineage is bound to the generation EVENT, not a model string: used_generation_id
/ used_generation_ids / observed_generation_ids come from the response ids
actually consumed. A model string cannot identify what ran -- two generations can
carry the same string, and a relabelled fallback carries one that was never
served.
The terminal body is whitespace-delimited, and a real generation id is
`provider\0responseId`, which would truncate the field and silently corrupt every
token after it. A field-safe id prints raw; anything else prints a digest, which
still binds because result.json carries the id verbatim. Found by the test.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012bNEs7T8pAdT7eJQTDgtkA
…elabelled-fallback case Three gaps closed against the required boundaries: * Observation PROVENANCE now survives downstream. observation_source and observed_provider reach both the bundle and the terminal post, so a reader can tell which channel established identity -- a gateway attestation and a value scraped off a response stream are not the same evidence, and `unavailable` is not the same as "nobody looked". * Stage 4 is asserted as an immutable event identity, not a string: the test proves used_generation_id is none of the three model strings, that it is the id the observation itself came from, and that all four stages are four distinct observable values. * The FAITHFUL relabelled-fallback case. buildFallbackModel writes the requested id onto another model's configuration, so downstream resolved == requested and the agreement is manufactured. A test now proves that agreement is not read as confirmation, and that the wire report still names the model that actually served the call. Also a consumer-level A->A->A positive control: without it the strictness is satisfiable by a downstream that prints `unknown` unconditionally. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bNEs7T8pAdT7eJQTDgtkA
A copied model string is not evidence that a particular generation was consumed. This checks result.json's claim against session.jsonl's immutable response keys, both read out of the ONE tarball the worker handed the uploader: the generations the evidence claims must be exactly the generations present in the session it shipped, and every observed identity must be attributed to one of them. Shown to do real work rather than ride on an earlier assertion: a mutation that renames used and observed generation ids CONSISTENTLY keeps the evidence self-consistent, so the distinctness, non-model-string and four-stage checks all still pass -- and only this control fails. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bNEs7T8pAdT7eJQTDgtkA
…not be closed via record() LOCAL ONLY -- deliberately not pushed. This does NOT close the review finding, and 96ebc35 remains the PR head until a real repair is agreed. An independent exact-head review found that generation identity reaches the terminal bus post only on the COMPLETED path. Reproducing it turned up more than was reported, and also showed my first repair was actively harmful: * The nonzero-exit FAILED path and the UNVERIFIED paths (gh failure after the run, no-commit) DO carry the identity -- they go through the main post-run terminal, which records before transitioning. Three regression tests now pin that, and they caught a regression I introduced while attempting the repair. * Routing the record through emitEconomics -- the obvious one-place fix, since all 14 call sites already receive `lifecycle` -- is WRONG and harmful. `record()` throws on a terminal state (task.mjs: `if (this.isTerminal()) throw`), the 13 call sites transition to their terminal state BEFORE emitting economics, and emitEconomics runs inside a try/catch. The throw is swallowed and the function silently returns its DEGRADED summary instead. Observed as a changed economics_sha256 on the injected-fault path. Reverted; the constraint is now documented at the site so the next attempt does not repeat it. The real gap that remains: the outer-catch FAILED path can fire after a session has run and spent, and cannot carry identity through the task record at all. Closing it needs the identity passed INTO postFinal as a parameter, the way economicsSha already is. That is a signature change across ~15 call sites, which is more than a minimum repair, so it is proposed rather than taken. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bNEs7T8pAdT7eJQTDgtkA
…ing an observed provider Repairs the two defects an independent review raised against 96ebc35. REPAIR 1 -- identity is now an explicit data dependency of the final post. Identity reached the terminal post only on the COMPLETED path, because it travelled through `lifecycle.record()`. Every other post-run terminal lost it, including the outer-catch FAILED path whose own comment notes a session may already have run and spent. It cannot be fixed by centralising that write: `record()` throws once the state is terminal -- correctly, terminal task state is immutable -- the 13 other emitEconomics call sites transition BEFORE emitting economics, and emitEconomics runs inside a try/catch, so such a call is swallowed and silently returns the DEGRADED summary instead. That was measured (a changed economics_sha256) and is documented so the next attempt does not repeat it. Instead `postFinal` now takes the emitEconomics result `{ summary, sha256 }` whole. The digest and the identity are two projections of ONE summary, so the post cannot carry a SHA from one and an identity from another, and no lifecycle mutation is needed to tell postFinal something its caller already holds. The lifecycle transport is removed entirely. Identity is emitted only where a generation could actually have occurred (`model_calls > 0` or a generation id), so a pre-run refusal asserts nothing -- `observation=unavailable` there would be a claim about evidence for a generation that never happened. REPAIR 2 -- `observed_provider` no longer echoes the requested provider. It was set to `requestedProvider` whenever any MODEL was observed, asserting provider provenance the data never had: every adapter sets a response's `provider` from `model.provider` (packages/ai/src/api/*.ts) and NO adapter reads a served provider off the wire. So there is no provider evidence at observation strength anywhere in this system. `observed_provider` is now always null -- the correct answer, kept present so a consumer can see provider is never observed -- and the routing/runtime value is reported under its own name as `runtime_provider` / `runtime_providers`. TESTS: 11 + 14 checks (was 11 + 8). New coverage: FAILED after a generation, UNVERIFIED after gh-failure and after no-commit, a postFinal terminal with no generation, a pre-run refusal, and absent provider provenance. The fixture's runtime provider is deliberately NOT the requested provider -- with both set to "openrouter" a substitution of one for the other was invisible and the provider assertions false-greened, which a mutation exposed. Local suite 1616 groups pass. The reviewer's injected post-run fault now carries the identity, and stops carrying it when the data dependency is reverted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bNEs7T8pAdT7eJQTDgtkA
…ound Tests only. No production file changed. Each of these was a mutant that SURVIVED the full suite, i.e. code whose removal broke nothing. All three are now killed, each for its intended assertion: 1. The multi-generation bus-post branch (`generationIdFields`, ids.length > 1) had zero coverage -- deleting it outright left all 25 checks green. It exists so a multi-generation attempt is never summarised by one id standing for all of them, which is the exact case the lineage was built for. Now driven end to end through the real worker: two generations must reach the post as used_generation_count + used_generation_ids_sha256, and must NOT collapse to a single used_generation_id. 2. Both arms of `generationOccurred` ((model_calls > 0) || (ids.length > 0)) were individually deletable with the suite staying green, because every fixture produced the two together. Two cases now drive exactly one arm each -- a response key whose call count was lost, and a counted call that carried no key -- both real partial-usage shapes. In both, spend happened, so identity must be reported rather than suppressed. 3. The test named "a pre-run refusal does not invent a generation identity" could not fail for the reason it named: deadline refusals post via blockerPostBody, which structurally carries no identity fields, so it held whether or not the gate existed. Relabelled to "the blocker post path carries no identity fields (does NOT test the gate)" and annotated to point at the branch-lease case that genuinely exercises the gate. Kept rather than deleted: it pins a real and otherwise-uncovered property about the blocker path, it just is not the discriminating case its old name implied. NOT CLOSED, and deliberately: the outer-catch FAILED path still has no committed regression test. It is not reachable from the harness -- uploadEvidence catches everything internally, the post-run git diff and readHead both pass allowFailure, publish throws only on internal invariants, the local economics write has its own try/catch, and the one unguarded sync call (sealAttemptDir) is clean-room-only and cannot be forced from a fixture. Writing it needs a production change, so it is reported rather than taken. Suite: 14 + 17 checks (was 11 + 14); 1616 groups pass locally. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…roves S03-R2 flagged the CASE 11 (F2) comment as overstating: it claimed the two cases directly exercise each arm of `generationOccurred`. That gate lives in vinci/worker/worker.mjs and is not reachable from this test file, which loads only session-read.mjs and task-outcome.ts. The comment now says what the fixture actually proves -- the two partial-usage shapes are isolated at the economics level, with downstream consumer cases making the resulting behavior observable -- and explicitly disclaims being a direct unit test of the gate. Comment-only. No executable change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JGUFKeVpTZbdUfLeGiqmrP
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Subject — AUTHORITATIVE
🔴 Review
534fd26eonly. That is the current head and the only review subject.96ebc352was reviewed and returned NO-GO; both of its findings are repaired above it.d6554d34is the previously reviewed predecessor and is production-equivalent to this head; it was superseded by tests-only successors and is no longer the review subject.b4ac707dwas reviewed by S03-R2 (PASS_WITH_LIMITATIONS) and differs from534fd26eby a single comment-only correction closing that review's RC-2, so no production byte differs between them.534fd26eitself has not yet been reviewed. See the UPDATE section at the end of this description.12 commits, 7 files, all inside
vinci/worker/**,vinci/extensions/**,vinci/test/**. Nopackages/**change.vinci/worker/task.mjsuntouched, soTRIPLE_KEYS,TERMINAL_STATES,TRANSITIONSandmaterializeEnvelope's contract are unchanged.Not to be squashed or rebased: individual commits carry the load-bearing controls, including the one proving the consumption binding itself discriminates.
What this closes
An adoption claim must be impossible when the model that actually served a run is unknown. Three facts were previously fused into one field, in two independent places (
usage-accumulator.ts:157andtask-outcome.ts:309, bothresponseModel ?? model), so a consumer readingusage[].modelcould not tell an observed agreement from a provider that reported nothing.buildFallbackModelmakes that indistinguishability load-bearing: it returns another model's entire configuration relabelled with the requested id, so downstreamresolved == requestedand every identity keyed onmodel.idreports the request back as though it were an observation.Interface
generation_identity, on the economics summary and carried into the evidence bundle:observationobserved|unavailable|conflict— three states, never a booleanrequested_provider/requested_modelresolved_model/resolved_modelsobserved_provider/observed_model/observed_modelsobservation_sourceused_generation_id/used_generation_idsobserved_generation_idsmodel_calls/observed_model_calls/unobserved_model_callsmatches_requestedtrue/false/nullwhen nothing was measuredobserved_modelis only ever a value the provider put on the wire. It is never derived from the requested or resolved id. When nothing was observed it isnull,observationisunavailable, andmatches_requestedisnull— neverfalse, because we did not measure a mismatch, we failed to measure.Producer and consumers
Producer:
vinci/worker/economics.mjs:observeGeneration.Two real downstream consumers, both preserving the distinction in emitted evidence:
observation=,observation_source=,observed_provider=,requested_model=,resolved_model=,used_model=,used_generation_id[_sha256]=,identity_matches_requested=);result.jsoninside the evidence bundle.used_modelis emitted only from a machine observation, so an unobservable identity printsunknownand can never be mistaken for the requested string, andidentity_matches_requestedis omitted rather than guessed when nothing was measured.Existing structures reused throughout — the economics summary, the task record's
record()seam, the evidence bundle, the terminal post. No new generation registry: immutable identity already existed asresponseKey(provider\0responseId).Tests
Both files are glob-discovered by
run.shand execute inside the canonical suite.Every control is mutation-proven, each killed for the intended assertion rather than by a crash: observed←resolved (killed at economics and at the wire boundary), used-generation-id←requested identity (at economics and at the post), removed observation yielding UNKNOWN rather than an inferred identity (at session-read and at the post), A→B→C four-stage distinctness, the A→A→A positive control (killed by a report-everything-unknown implementation, so the strictness is not vacuous), and the relabelled-fallback case.
The final binding control was isolated deliberately: a mutation renaming used and observed generation ids consistently keeps the evidence self-consistent, so distinctness, non-model-string and four-stage checks all still pass — and only the binding control fails. It checks
result.json's claim againstsession.jsonl's response keys, both read from the one tarball the worker handed the uploader.Known bound — please read before treating
unavailableas a defectresponseModelis populated only whenchunk.model !== model.id(packages/ai/src/api/openai-completions.ts:323, documented attypes.ts:389). It is a drift-only signal, so today every compliant call reportsobservation=unavailable;observedis reachable only when the provider disagrees with the request — which is the fallback case that matters most, but it means agreement is not currently observable.27 of 28 adapters under
packages/ai/src/api/have no wire observation channel at all (anthropic-messages, bedrock-converse, google-generative-ai, google-vertex, mistral-conversations, openai-responses, openai-codex-responses, azure-openai-responses, cloudflare, openrouter-images).gateway-header(x-vinci-resolved-model) is gated onprovider === "vinci"and its own file records that those headers do not exist yet.The one-condition upstream change that would make agreement observable is in
packages/**, which PR #59 also edits. It is proposed, not taken here, and nothing in this branch depends on it.🤖 Generated with Claude Code
https://claude.ai/code/session_012bNEs7T8pAdT7eJQTDgtkA
UPDATE —
96ebc352was reviewed NO-GO; repaired atd6554d34An independent exact-head review of
96ebc352returned NO-GO with two findings. Both are accepted as real and repaired here. The reviewed head96ebc352should not be used.Finding 1 (HIGH) — identity reached the terminal post only on the COMPLETED path
generation_identitytravelled topostFinalthroughlifecycle.record(), which happened on exactly one code path. Every other post-run terminal lost it — including the outer-catch FAILED path, whose own comment notes a session may already have run and spent.The obvious fix is wrong and harmful. Centralising the write inside
emitEconomics(all 14 call sites already receivelifecycle) fails becauserecord()throws once the state is terminal — correctly; terminal task state is immutable — the 13 other call sites transition before emitting economics, andemitEconomicsruns inside atry/catch. The throw is swallowed and the function silently returns its degraded summary instead. This was measured as a changedeconomics_sha256on an injected-fault run, and is now documented at the site.Repair:
postFinaltakes theemitEconomicsresult{ summary, sha256 }whole. The digest and the identity are two projections of one summary, so the post cannot carry a SHA from one and an identity from another, and no lifecycle mutation is needed to tellpostFinalsomething its caller already holds. The lifecycle transport is removed.Identity is emitted only where a generation could actually have occurred (
model_calls > 0or a generation id): a pre-run refusal asserts nothing, becauseobservation=unavailablethere would be a claim about evidence for a generation that never happened.Finding 2 —
observed_providerclaimed provenance the data never hadIt was set to
requestedProviderwhenever any model was observed. Every adapter sets a response'sproviderfrommodel.provider(packages/ai/src/api/*.ts), and no adapter reads a served provider off the wire — so there is no provider evidence at observation strength anywhere in this system.Repair:
observed_provideris now alwaysnull— the correct answer, kept present so a consumer can see that provider is never observed — and the routing/runtime value is reported under its own name asruntime_provider/runtime_providers.Coverage added
11 + 14 checks (was 11 + 8): FAILED after a generation; UNVERIFIED after gh-failure and after no-commit; a
postFinalterminal with no generation; a pre-run refusal; and absent provider provenance.The fixture's runtime provider is deliberately not the requested provider. With both set to
openrouter, substituting one for the other was invisible and the provider assertions false-greened — a mutation exposed that, and the fixture now discriminates.Every new control is mutation-proven and each fails for its intended assertion. The reviewer's injected post-run fault now carries the identity, and stops carrying it the moment the data dependency is reverted.
Local canonical suite: 1616 groups pass.