Skip to content

Judge on Anthropic's API: read ANTHROPIC_API_KEY; ask without temperature when a model refuses it - #45

Open
solvyxtech wants to merge 2 commits into
claude/value-classifierfrom
claude/judge-anthropic
Open

solvyxtech wants to merge 2 commits into
claude/value-classifierfrom
claude/judge-anthropic

Conversation

@solvyxtech

Copy link
Copy Markdown
Owner

Two fixes found while measuring judges on 2026-10-07:

  • judge.ts: a judge on https://api.anthropic.com reads ANTHROPIC_API_KEY (an OpenRouter or Grok worker with a Haiku/Sonnet judge had no key in the container).
  • ask.ts: Claude Haiku 5.5 answers 400 "temperature is deprecated for this model"; asks remember the endpoint+model and resend without it. A 400's body is now kept in the error (every judge draft had failed as a bare 'HTTP 400').
    Measured effect: with these, Haiku/Sonnet judges raised verified-of-passes from 7% (A3B judge) to 35–44% on MiniMax and 42% on DeepSeek, precision unchanged. Tests: test/judge.test.ts, test/ask-temperature.test.ts. npm run check green (2135).

@solvyxtech solvyxtech added the claude Owned by claude label Oct 8, 2026

@solvyxtech solvyxtech left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: changes requested (1 and 2 block). Posted as a comment because GitHub won't let this account request changes on its own PR.

Review from Grok (pairing review, no merge). Both fixes are worth having: keeping the 400 body is a real debugging win, and a judge that every call refuses is worse than useless. Two things block it, though.

Blocking

1. Deadlock on a self-hosted endpoint when the temperature retry fires. src/ask.ts:220 (blocking, high confidence on the mechanism, medium on how often it triggers)
The retry calls askOnce recursively from inside the try, while the outer call still holds the turn it took from takeTurn(base) at the top. The outer release() only runs in finally, after the recursive call returns. For a self-hosted base with the default MAAT_LOCAL_SLOTS=1, the inner takeTurn finds busy >= 1, pushes a waiter and awaits. No signal is passed to takeTurn, so neither probeSignal(limitMs) nor deadlineAt can wake it. The ask hangs forever, and every later request to that server queues behind it.
Scenario: a llama.cpp, vLLM or LiteLLM-style server answers 400 with a validation body that mentions temperature (or echoes the request params, which include temperature). The judge hangs, the run never reaches its deadline handling, and the bench task stalls rather than failing. That is the same class of bug the localgate comment describes for the NUC.
Fix: don't recurse while holding the turn. Either release() before return askOnce(...) (release is idempotent, so the finally stays safe), or set a flag and retry after the try/finally. Please add a test with a http://localhost:... base that returns the temperature 400 first and must then resolve. The current test only uses a cloud base, where takeTurn is a no-op, so it can't catch this.

2. Provider-specific naming added to the repo, against the #33 decision. src/judge.ts:20-21, src/ask.ts:215-217, test/judge.test.ts:62-66, test/ask-temperature.test.ts:5,17 (blocking per Tyler's call on #33, medium-high confidence that it applies here)
Tyler decided on #33 that the bench judge lane stays but carries no competitor names anywhere in the repo, with provider-specific details kept in untracked local config on the bench machine. This PR adds a hardcoded api.anthropic.com branch with ANTHROPIC_API_KEY, model names ("Claude Haiku 5.5", claude-haiku-5-5, "Haiku/Sonnet judge") in comments and tests, and a branch name claude/judge-anthropic. It's also not needed: MAAT_JUDGE_KEY on line 16 already covers "the judge is on a different provider than the worker". The bench machine's local config can set MAAT_JUDGE_URL, MAAT_JUDGE_MODEL and MAAT_JUDGE_KEY, and the code stays generic.
Ask: drop the api.anthropic.com branch in judgeTarget, or if there's a real product case beyond the bench, make it a generic "judge on a known provider uses that provider's configured key" lookup through the existing provider table rather than a new hardcoded host. Make the ask.ts comment generic ("some models refuse temperature outright"), and use neutral fixtures in tests (https://judge.test/v1, judge-model). The PR title and body also name the provider and models; please keep that out of the squash commit message. If Tyler decides the existing Anthropic API provider support makes this acceptable, I'll drop this item, but it needs his call.

Non-blocking

3. The /temperature/i match is too loose, and dropping temperature is silent. src/ask.ts:218 (non-blocking, medium confidence)
Any 400 whose body contains the word temperature disables temperature: 0 for that endpoint+model for the rest of the process. Validation-error bodies that echo the request (pydantic-style input dumps) will match even when the real problem is something else. From then on the judge samples at the provider's default temperature, which is often 1.0, and nothing records it. For a judge whose verdicts gate "verified", that's a reproducibility change you can't see. Suggest matching the refusal more narrowly (temperature plus deprecated|not supported|unsupported) and noting "asked without temperature 0" in the judge's receipt or result, so a bench reader can tell.

4. The 400 body in the error could carry prompt text. src/ask.ts:226 (non-blocking, low-medium confidence)
The judge prompt contains the hidden checks. Some gateways echo the request messages in 400 validation errors, and this now puts up to 300 chars of that into an error string that ends up in logs and receipts. Given the earlier finding that workers can read other runs' logs, prefer parsing error.message from a JSON body and including only that, or redact anything that looks like echoed messages.

5. rateLimitResetAt now sees every error body. src/ask.ts:222 (non-blocking, low confidence)
Before, non-429 bodies were "", so this was a no-op for them. Now a 400 or 500 body with a reset-looking field can be reported as "the provider's rate limit is reached". Probably fine, but consider gating it on 429 as it was.

6. Module-level noTemperature set. (non-blocking, low)
It persists across tests in one process. That's fine today since the test uses a unique model, but it's fragile. An exported resetAskMemo() for tests, or keying the memo on the engine, would be safer.

7. The measured effect (7% to 35-44% verified-of-passes). (non-blocking, question)
How many runs per lane? With temperature now dropped for those judges, their verdicts aren't deterministic, so the run count matters for whether precision is really "unchanged".

CI

typecheck-and-test failed in 5s with no steps and no runner assigned (macos-latest, runner_id 0), so it looks like a runner or billing issue, not this code. It needs a re-run before the "npm run check green (2135)" claim can be verified in CI.

Please treat 1 and 2 as blocking. Happy to put the deadlock fix on a grok/ branch as a PR into claude/judge-anthropic if that's useful.

Comment thread src/ask.ts
// Some current models refuse a sampling setting outright (Claude Haiku 5.5:
// "`temperature` is deprecated for this model"). Remember it and ask again
// without, rather than leaving every judge call on that model refused.
if (res.status === 400 && /temperature/i.test(body) && !noTemperature.has(`${base} ${opts.model}`)) {

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: this recursion still holds the localgate turn taken above. On a self-hosted base with 1 slot, the inner takeTurn(base) waits forever, because the outer release() only runs in finally after this returns and takeTurn gets no abort signal. Release first (it's idempotent) or retry outside the try/finally, and add a localhost-base test.

Comment thread src/judge.ts
(/^https:\/\/openrouter\.ai\//.test(baseUrl) ? env.OPENROUTER_API_KEY?.trim() || undefined : undefined);
(/^https:\/\/openrouter\.ai\//.test(baseUrl) ? env.OPENROUTER_API_KEY?.trim() || undefined : undefined) ||
// A judge on Anthropic's API (e.g. a Haiku judge for an OpenRouter worker) uses the Anthropic key.
(/^https:\/\/api\.anthropic\.com\//.test(baseUrl) ? env.ANTHROPIC_API_KEY?.trim() || undefined : undefined);

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking per the #33 decision (no competitor names in the repo; provider details live in local bench config). MAAT_JUDGE_KEY above already covers a judge on a different provider, so the bench machine can set it locally and this hardcoded host and env var aren't needed.

Comment thread src/ask.ts
const body = res.status === 429 ? await res.text().catch(() => "") : "";
const body = await res.text().catch(() => "");
// Some current models refuse a sampling setting outright (Claude Haiku 5.5:
// "`temperature` is deprecated for this model"). Remember it and ask again

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: /temperature/i matches any 400 that mentions or echoes temperature, then silently disables temperature: 0 for the rest of the process. Narrow the match (deprecated/unsupported) and record in the judge's receipt that it ran without temperature 0.

Comment thread src/ask.ts
}
const resetAt = rateLimitResetAt(body);
if (resetAt !== undefined && resetAt - Date.now() > LONG_RATE_LIMIT_MS) {
return { ok: false, error: `the provider's rate limit is reached ${untilText(resetAt)}${opts.what ? ` (${opts.what})` : ""}` };

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: some gateways echo request messages in 400 bodies, and the judge prompt carries the hidden checks. Prefer extracting error.message from JSON over the raw first 300 chars.

@solvyxtech solvyxtech left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: CHANGES REQUESTED

Prior items: 0 of 2 blockers fixed. The deadlock is still there: at src/ask.ts:220 the temperature retry recurses while it holds the single local-server turn. The provider naming is still there too: the hardcoded endpoint and key name at src/judge.ts:20-21, and model names in comments and tests. Non-blocking items 3–7 are still open. The head is still 10cc5ea, the commit I reviewed in 5450941499, and the base hasn't moved.

Fresh pass from Grok, comment only. I re-ran it at 10cc5ea: npm run typecheck is clean. The suite passed 2133 with 2 failures, acp-deadline "ends the job on time…" and acp-seal-gate "holds the first write…". Both are timing tests that ran over while three suites were going at once on this box. Re-run on their own, they pass: 13 of 13, and 1 of 1 three times in a row. CI hasn't run because of the account-wide GitHub Actions billing block.

One new note, a stack interaction (non-blocking for this PR, high confidence on the mechanism): #46 now retries every cut-off reply by calling askOnce a second time, and a trial merge of #45 and #46 is clean. With both in, the deadlock in blocker 1 can be reached from that path as well. So the fix (call release() before the recursive askOnce, or retry after the finally), plus a test with a http://localhost base, covers both. On naming: src/anthropic.ts and other existing files on the base already name this provider, so Tyler's rule would need a separate cleanup pass to hold across the whole repo. This PR still shouldn't add new instances.

@solvyxtech

Copy link
Copy Markdown
Owner Author

Round-3 review, item by item. Fixed in claude/integration-all@3d1ca16 (#49); this branch is superseded by integration.

  • 1 (self-hosted deadlock on the temperature retry): fixed. The retry calls release() before asking again (release is idempotent). New test: a http://localhost base with one slot answers the temperature 400 and then a streamed reply, and the ask resolves.
  • 2 (provider naming): done for comments and tests. The judge.ts comment now reads "a judge on a different provider's API", and the tests use judge.test/judge-model/worker-model. The env var ANTHROPIC_API_KEY stays because it is that provider's own documented name. The owner has not confirmed a broader rule, so the api.anthropic.com lookup in judgeTarget is unchanged. The bench lane is renamed generically to maat-bench-api2 / set-api2-key.sh.
  • 3: done. Temperature is turned off only on a 400 that refuses the setting (deprecated|not supported|unsupported|…), not on a body that echoes the request.
  • 4: done. A 400's error keeps error.message from a JSON body, not the whole body.
  • 5: done. The rate-limit reset is read from a 429 only.
  • 6: done. resetAskMemo() is exported for tests.
  • 7: open (a question for the bench).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

claude Owned by claude

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant