fix: the shipped recipe kept a bare default, so a pasted copy lost the judge - #49
Conversation
…e judge The judge rule went into the control plane's provisioning constant and its own pasteable copy, and not into this one — the file a BYO consumer actually pastes. So setup produced a correct recipe while the documented one sent the judge to `default:`, i.e. the reviewer's own model. A judge scoring work its own model produced agrees with it and still reports the pass as successful, so a workspace following the docs got a silently inert L2. Caught by a reader, which is the third time a recipe has drifted that way: the fact contract still said none|cheap|strong after one value was ever sent, the per-angle rules outlived the path that could reach them, and now this. The cause is structural and stays: the authority for what setup writes is a Go constant in another repository, and nothing here can see it. Cross-repo agreement is not assertable. So the new suite pins the properties that make a recipe self-consistent instead, which is what each of those three failures actually violated: - every rule names a model, and a default exists - every `when:` keys on a fact the action actually sends, so a rule cannot describe a policy that never applies - the Action's recipe carries a judge rule - that rule does not name the default's model - it carries no per-angle rule, which no Action call can reach - it names the router by the alias action.yml defaults to, so a rename cannot leave the paste target pointing at a name that no longer resolves Verified the way it will be relied on: removing the judge rule turns two of them red. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 067644218e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (const c of conditions) { | ||
| assert.match( | ||
| c, | ||
| /headers\["x-cr-(lens|prev-tier|prev-p0p1)"\]/, |
There was a problem hiding this comment.
Validate reachable fact values, not only header names
A rule such as headers["x-cr-prev-tier"] == "cheap" or headers["x-cr-lens"] == "ripple" passes this assertion because the regex checks only the header name, even though the Action sends only standard and judge, respectively. This leaves the suite unable to catch the exact stale-value drift it was introduced to prevent; validate the supported value for each fact as well.
Useful? React with 👍 / 👎.
| const useMatch = line.match(/^\s*use:\s*\{\s*model:\s*"([^"]+)"/); | ||
| if (useMatch && id) { | ||
| out.push({ id, model: useMatch[1] }); |
There was a problem hiding this comment.
Retain model-less rules so the assertion can reject them
When a non-judge rule loses its use line or has an empty model, rules() silently omits that rule because it only appends after matching a nonempty quoted model. Consequently, the later assert.ok(r.model) loop sees nothing and the advertised “every rule names a model” contract passes despite the broken route; record each ID immediately and attach any later model so missing models remain observable.
Useful? React with 👍 / 👎.
| # TWO CALLS, TWO LINES TO EDIT. The Action asks this router for two things, both | ||
| # with the alias as the `model` rather than a model name: |
There was a problem hiding this comment.
Document the judge-model override before promising two recipe edits
For workflows that set the supported judge-model input, action.yml selects L2_MODEL="${JUDGE_MODEL:-$ROUTER}", so the judge request names that concrete override and never traverses this router rule. The new claim that both calls use the alias—and therefore that operators edit two lines here—will make judge-model changes appear ineffective; qualify these instructions to explain that the recipe controls the judge only when the input is empty.
Useful? React with 👍 / 👎.
The judge rule went into the control plane's provisioning constant and its own pasteable copy, and not into this one — the file a BYO consumer actually pastes. Setup produced a correct recipe while the documented one sent the judge to
default:, i.e. the reviewer's own model. A judge scoring work its own model produced agrees with it and still reports the pass as successful, so anyone following the docs got a silently inert L2.Caught by a reader. That is the third recipe drift of the same kind: the fact contract still said
none | cheap | strongafter one value was ever sent; the per-angle rules outlived the path that could reach them; now this.Why it will happen again, and what to do about it
The authority for what setup writes is a Go constant in another repository. Nothing here can see it, so cross-repo agreement is not assertable and I am not pretending otherwise.
What the new
scripts/recipe.test.mjspins instead is self-consistency — and each property is one the three drifts actually violated:when:keys on a fact the action actually sends, so a rule cannot describe a policy that never appliesaction.ymldefaults to, so a rename cannot leave the paste target pointing at a name that no longer resolvesVerified the way it will be relied on: removing the judge rule turns two of them red.
Suite counts otherwise match
main(installer 2 / platforms 1 / report 1, one fewer insettings); those are pre-existing and unrelated.