Structured output: constrain an answer to a JSON schema - #10
Conversation
A consumer building anything on top of this, a code review being the case in hand, wants findings back as data. Without a schema it has to parse prose, which is a guess about formatting the model never promised. `Request::schema` constrains the answer and `Outcome::structured` carries the parsed value. The template that uses it, review prompts, a findings shape, diff handling, belongs a layer up in the consumer: this is the capability, not the policy, and only this crate can express it because it is a CLI flag. The three agents differ and the difference is hidden rather than leaked: - Claude takes the schema inline and reports the conforming value in its own `structured_output` field. - Codex reads it from a file, so the runner writes one for the run and removes it afterwards through a guard, meaning cancel and timeout clean up too. Its answer is the JSON itself, parsed only when a schema was asked for, so prose is never reinterpreted as data. - Copilot 1.0.75 has no schema support at all, so asking is Error::Unsupported rather than prose presented as data. Both shapes verified live against the real CLIs. Two things found while building it, both worth keeping: Codex sends the schema to a provider requiring `additionalProperties: false` on every object, and rejects the request with a 400 before the model runs otherwise. Claude accepts the looser form, so a schema that works there can still fail on Codex. Documented on the builder and in the README, since the error arrives from the provider rather than from anything this crate could check. That rejection also exposed a reporting bug. Error::Failed took the first line of stderr as the cause, but CLIs open with progress chatter: the report read "Reading additional input from stdin...", which explains nothing, while the real cause sat in Codex's JSON error event on stdout. A line that looks like an error now wins over one that narrates, and stdout is consulted when stderr only narrates. That misdirection cost me three rounds of diagnosis here, which is exactly what it would cost a user.
|
This PR now has two commits. Please wait for both before merging — the second landed after the first push, which is the pattern that stranded work on #5, #8 and #9. Second commit: a real bug, now observable
The auth classifier's Copilot branch was inference. I said so at the time, because a bogus Pointing Copilot at an empty Claude says "Not logged in". Codex says "not logged in". Copilot shares no vocabulary with either. So a missing Copilot login was reported as a generic What that says about the rest: the phrases that were observed are right, and the one that was inferred was wrong in a way no amount of reasoning would have caught. The remaining unobserved phrases are still guesses. They stay because a false negative there costs a worse error message rather than a wrong result, but they are not evidence. Two other things worth recording from the same experiment:
113 unit tests, clippy clean, |
|
Third commit: the streaming fix ( Streaming existed but was effectively off with the settings a caller gets for free. Two causes.
Claude also needed Claude sends both the deltas and the finished message they build up to, so emitting both would show every answer twice. The finished copy is dropped once deltas have been seen, detected rather than configured: deltas always precede it, so seeing one proves the copy is redundant. Suppression covers text and thinking only, never tool calls, which the deltas do not duplicate. Proven live, not by reading flags: with no Full live suite: 14/14. 117 unit tests, clippy clean, Not in scope, noted for later: |
The capability behind a code-review interface, kept as a primitive. The template (review prompts, a findings shape, diff handling) belongs a layer up in the consumer; this is the part only this crate can express, because it is a CLI flag.
Missed 0.2.0: it was pushed to #9 after that PR had already been merged, so it lands as 0.2.1.
The three differ, and the difference is hidden
--json-schema <inline>structured_outputfield--output-schema <FILE>Codex needs a file, so the runner writes one and removes it through a guard, meaning cancel and timeout clean up too. Its answer is parsed only when a schema was asked for, so prose is never reinterpreted as data. Copilot 1.0.75 has no schema support, so asking is
Error::Unsupportedrather than prose presented as data.Both shapes verified against the real CLIs, including a test that no schema file is left in the temp directory.
Two things found while building it
Codex requires strict schemas. It sends yours to a provider that demands
"additionalProperties": falseon every object and rejects the request with a 400 before the model runs:Claude accepts the looser form, so a schema that works there can still fail on Codex. Documented on the builder and in the README, since the error comes from the provider rather than anything this crate could check.
That rejection exposed a reporting bug.
Error::Failedtook the first line of stderr as the cause, but CLIs open with progress chatter. The report read"Reading additional input from stdin...", which explains nothing, while the real cause sat in Codex's JSON error event on stdout. Now a line that looks like an error wins over one that narrates, and stdout is consulted when stderr only narrates.That misdirection cost me three rounds of diagnosis on this PR, which is exactly what it would cost a user reading the error.
112 unit tests, clippy clean,
cargo packageverifies, live schema tests pass on Claude and Codex.