refactor(voice): the contract and the eval come home, and a stale copy stops lying - #11
Merged
Merged
Conversation
…ops lying
cloud-api held a second implementation of the strings this app speaks, in
TypeScript, canonical on paper and never executed anywhere. It existed to
generate the JSON fixtures that held the two ports equal — across a hand-run
`cp` between two repositories that nothing checked. chenchenSimular's review of
simular-pro-unified-ui#1635 called that crossing manual. It was worse than
manual: it was broken.
`parity/prompt-and-tools.json` still declared an `approveAlways` tool and still
told the model to offer "want me to just allow these from now on?" — months
after ADR 0014 retired the feature and sai-fi#9 removed it. `VoiceProfileTest`
graded that file, so the test written to catch a dropped prompt block was
reading something nothing ships. `LiveBrain` read it too, which means every
judged loop run since sai-fi#9 put the real model through a prompt for a tool
that no longer exists. Nothing failed. Nothing could.
So there is one implementation now, and one copy of everything it reads.
**The profile.** `prompt-and-tools.json` is deleted; `assets/voice-profile.json`
is on the unit-test classpath and `shippedProfile()` is the only way to it. The
file that ships is the file that gets graded. 17 tools, not 18.
**The goldens.** `GoldenFixtures.kt` ports cloud-api's `contract/fixtures.ts`,
calling the real `describeAgentEvent` / `describeCompleteAskFirst` /
`renderAgentActivity` / `ActivityLog` over the same canonical inputs on the same
pinned clock. Regenerating from Kotlin reproduced all 61 committed fixtures
byte for byte, which is the proof the port is exact rather than merely
self-consistent — `GoldenJson.kt` matches `JSON.stringify(x, null, 2)` for that
reason, since org.json can neither hold key order nor indent the same way.
`RegenerateGoldensTest` writes them, gated on SAI_REGEN_GOLDENS. CI never sets
it, and that guarantee is load-bearing: in cloud-api this generator was once an
ordinary test that wrote its own expected output, so every CI run silently
rewrote the fixtures. A golden that regenerates itself is not a golden.
The two `*ParityTest`s become `*GoldenTest`s — with no second port, what they
pin is the wording, which is the honest description and the valuable one, since
nearly every string in there was found by hearing it fail on a call. They also
absorb the behavioural assertions from `contract/fixtures.test.ts`: what the
strings SAY, which a byte diff cannot tell you. One of those was weaker than it
looked. The fencing invariant asserted `indexOf("[agent]") < indexOf(fence)`,
which passes trivially when the marker is absent — -1 is less than everything —
so it said nothing at all about the failed-step nudge, the one that says
`[context — …]` instead. It now asserts the ordering it meant to.
**The eval.** `eval/rubric.json` stops being a generated import and becomes the
source, and the harness that read it from over there comes with it:
`Transcripts.kt` (all 32, names and targets verified identical to the
TypeScript) and `TranscriptEvalTest` (`SAI_TRANSCRIPT_EVAL=1`), which reuses the
`GeminiText`, `Judge` and `EvalData` already here. Its subject was always this
repo's — the prompt, the tools and the nudge wording all ship from here, so over
there each had to be vendored, and one of them went stale.
Following `LoopEvalTest`, only the deterministic effect-choice half fails the
build; judged verdicts print as a score. cloud-api's runner gated its exit code
on them and the measured result was seven permanent reds on the default model,
which is a check everyone learns to skip.
`EvalDataTest` gains the wiring assertions from `rubric.test.ts` — every target
resolves, none targets a rule the judge cannot grade, every transcript checks
something, names are unique. They were unassertable while the transcripts lived
in the other repo: this side could see the rubric but not what read it.
321 JVM tests, 0 failures. Smoke-run two transcripts against a real model to
confirm the harness drives it: silent tool capture, `getSaiStatus` answered from
the real ActivityLog, the `toolResults` failure override, and both graders.
Companion to simular-pro-unified-ui#1635, which deletes the TypeScript side.
Merge this first.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
The problem discovery
chenchenSimular's review of simular-pro-unified-ui#1635 asked for ~4.4k lines of
voice/contract/+voice/eval/to move here. The argument: cloud-api holds a second implementation, in TypeScript, of wording whose only running copy is Kotlin — maintained forever solely to generate fixtures — and the cross-repo crossing defending it is manual anyway.Checking that claim before acting on it turned up something worse. The crossing was not merely manual. It was already broken, in both directions, and had been for months.
app/src/main/assets/voice-profile.jsonapp/src/test/resources/parity/prompt-and-tools.jsonapproveAlwaysPrompt blocks 21, 22, 38 and 39 differed. Block 38 still told the model to "offer to always-allow it" and to call
approveAlways— months after ADR 0014 retired the feature and #9 removed it.Two things read that file:
fsm/VoiceProfileTest— the test written to catch a dropped prompt block was grading a file nothing ships. It should have caught theapproveAlwaysremoval. Instead it pinned the old one.eval/LiveBrain— so everyLoopEvalTestrun since fix(voice): an "always allow" that never allowed anything #9 put the real model through a prompt for a tool that does not exist. (cloud-api's eval read the same copy, with the same consequence.)Nothing failed. Nothing could: each side only ever checked itself against its own copy.
Discussion insight: "A fixture crossing that silently writes nowhere is worse than no crossing: it reports success, the assertion that would have caught the drift skips itself, and both sides look green while the device answers differently from every other surface." — written in #1635's own description about a different instance of this bug. It happened again, one layer up, to the artefact that description was about.
So: one implementation, and one copy of everything it reads.
The profile
prompt-and-tools.jsonis deleted.assets/voice-profile.jsonis on the unit-test classpath (asourceSetsline inbuild.gradle.kts) andshippedProfile()is the only way to it — one function, because it used to be two paths and they disagreed. The file that ships is the file that gets graded. 17 tools.The goldens
GoldenFixtures.ktports cloud-api'scontract/fixtures.ts: the same canonical inputs, calling the realdescribeAgentEvent/describeCompleteAskFirst/renderAgentActivity/ActivityLogon the same pinned clockT0.Regenerating from Kotlin reproduced all 61 committed fixtures byte for byte. That is the proof the port is exact rather than merely self-consistent, and it is why
GoldenJson.ktexists:org.jsoncan neither preserve the order keys were written in (HashMap-backed) nor indent likeJSON.stringify(x, null, 2), and without both the diff would be noise.Generation is a switched-off test.
RegenerateGoldensTest, gated onSAI_REGEN_GOLDENS, which CI never sets. That guarantee is load-bearing rather than tidy: in cloud-api this generator was once an ordinary test that calledwriteFileSync, so every CI run silently rewrote its own expected output and drift became undetectable. A golden that regenerates itself is not a golden.*ParityTest→*GoldenTest. With no second port, what these pin is the wording — and that is the more valuable framing, since nearly every string in there was found by hearing it fail on a real call. They also absorb the behavioural assertions fromcontract/fixtures.test.ts: what the strings SAY, which a byte diff cannot tell you.One of those was weaker than it looked. The fencing invariant asserted
indexOf("[agent]") < indexOf(fence), which passes trivially when the marker is absent —-1is less than everything — so it said nothing at all about the one nudge that says[context — …]instead. It now asserts the ordering it meant to, and caught that case immediately.The eval
eval/rubric.jsonstops being a generated import and becomes the source. The harness that read it from over there comes with it:eval/Transcripts.kt— all 32 transcripts. Names and target lists verified identical to the TypeScript.eval/TranscriptEvalTest(SAI_TRANSCRIPT_EVAL=1) — reuses theGeminiText,JudgeandEvalDataalready here.GeminiTextalready carried "the retry policy the TS runner used", andJudgewas already a port of its judge prompt, so this was mostly data.Its subject was always this repo's: the prompt, the tools and the nudge wording all ship from here, so over there each had to be vendored — and one of them went stale. Here it reads the asset and calls
describeAgentEventdirectly. Nothing left to vendor.One deliberate divergence from the TypeScript runner. Following
LoopEvalTest, only the deterministic effect-choice half fails the build; judged verdicts print as a score. cloud-api'srun.tsgated its exit code on them, and the measured result was 7 permanent reds on its default model — a check that is always red is a check everyone learns to skip. The effect-choice half has no such excuse: either the model called the tool or it did not.EvalDataTestgains the wiring assertions fromrubric.test.ts— every target resolves, none targets a rule the judge cannot grade, every transcript checks something, names are unique. Those were unassertable while the transcripts lived in the other repo: this side could see the rubric but not what read it.Verification
321 JVM tests, 0 failures (
./gradlew :app:assembleDebug :app:testDebugUnitTest --rerun-tasks).Goldens are stable — regenerating produces no diff:
Two transcripts smoke-run against a real model to confirm the harness actually drives it — silent tool capture,
getSaiStatusanswered from the realActivityLog, thetoolResultsfailure override, and both graders:Merge order
This merges first. Its companion is simular-pro-unified-ui#1635, which deletes the TypeScript side; nothing is removed there until it exists here.
🤖 Generated with Claude Code