fix(core): harden the eval harness from live A/B and BEAM runs - #1416
fix(core): harden the eval harness from live A/B and BEAM runs#1416phernandez wants to merge 10 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d43c69b6e5
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 628b027058
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
An unterminated slice concatenated the stderr footer onto the final stdout line in terminals and merged captures. Lead with the newline on stderr when content lacks one; stdout stays byte-exact for pipes. Found during live smoke testing on the 1398 eval branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp Signed-off-by: phernandez <paul@basicmachines.co>
The live 100K tier mixes ints with one level of int-list groups in event_ordering source_chat_ids ([116, ..., [136, 138]]) when a single event's evidence spans chats — a shape the fail-fast loader rejected. Flatten one level of int lists into the union; anything deeper or non-int still fails fast. Fixture and tests carry the live shape. Found by the loader's own schema guard during the first real dataset fetch on the 1398 eval branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp Signed-off-by: phernandez <paul@basicmachines.co>
The live 100K tier carries a third marker variant: '->-> 2,22, 24' — an id list with a space after a comma. The pattern now matches an int followed by comma-separated ints or N/A with optional spaces, staying narrow so unknown '->->'-bearing content still trips the fail-fast instead of being silently stripped. Fixture and tests carry all three observed variants. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp Signed-off-by: phernandez <paul@basicmachines.co>
Surveyed every marker in the live 100K tier (2,199 across five shapes) and closed the last gap: six markers carry a trailing ')' that is generator junk (zero opening parens in every affected message). The pattern now covers the complete observed population while unknown '->->' content still fails fast; comment records the survey. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp Signed-off-by: phernandez <paul@basicmachines.co>
Anthropic's OpenAI-compat endpoint rejects identity-linked API keys without an anthropic-workspace-id header, which the first live A/B run hit as an opaque 400 on all 24 tasks. Add a repeatable --model-header Name=value option that rides only in the model-factory closure — never in AgentTasksConfig, so header values can never leak into run artifacts — and include the response body in openai-compat errors so the next rejection names itself. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp Signed-off-by: phernandez <paul@basicmachines.co>
Claude 5 models reject the temperature parameter outright ('temperature
is deprecated for this model'), which the second live A/B hit on all 24
tasks — named precisely by the new error-body reporting. Add
--model-temperature (number, or 'omit' to drop the parameter), default
unchanged at 0 so local openai-compat servers stay pinned; the choice is
recorded in the run config for provenance.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp
Signed-off-by: phernandez <paul@basicmachines.co>
project add registers a project but does not index it, so the DB stayed empty, status reported ready vacuously (zero pending work was ever queued), and every retrieval tool saw an empty project — the first real-model A/B ran 24 tasks against nothing. Run an explicit reindex --full --search after registration, before the settle wait. The scripted smoke masked this because canned answers never consult the index. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp Signed-off-by: phernandez <paul@basicmachines.co>
Three fixes from the first real-model A/B run: - Answer-set graders now strip the task's OWN project prefix from answer-extracted permalinks (agents faithfully quote tool output, which is project-prefixed); a different project's prefix still fails, so cross-project leakage keeps failing. Six of the run's failures were this comparison artifact. - State-graded tasks run reindex --search (whose completion includes forward-reference relation resolution) before settling and grading; the rich curate-connect 'write was verified correct by argument fingerprint but grading saw 0 resolved relations' race is closed. - Task project names no longer double the at- prefix. Also diagnosed, agent-side by design (measured behavior, no harness change): agents sometimes omit the project argument and the warm MCP session routes calls to the previously named project — including writes, with the agent then 'verifying' in the wrong project. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp Signed-off-by: phernandez <paul@basicmachines.co>
Live-run DBs store entity permalinks project-prefixed
('at-<run>-<task>/notes/...'), so RelationResolves' relative-permalink
SQL could never match — curate-connect was ungradeable on every run for
every agent. Match both forms for the source and strip the task's own
project prefix from resolved targets before comparison; a target under
a different project's prefix still fails.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp
Signed-off-by: phernandez <paul@basicmachines.co>
A programmatic caller constructing AgentTasksConfig and calling run_agent_tasks directly got the factory's default temperature while manifest.json recorded the configured value — a silent provenance lie in an artifact whose whole job is provenance. The CLI was unaffected because it pre-binds temperature into its factory. The default is now a None sentinel rather than the function itself, so the branch reads the module attribute at call time; an injected factory still owns its own configuration and is called unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp Signed-off-by: phernandez <paul@basicmachines.co>
628b027 to
65de3a4
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65de3a498e
ℹ️ 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".
| # missing header, quota) — without it the operator sees only | ||
| # a bare status code. | ||
| if isinstance(exc, httpx.HTTPStatusError): | ||
| error_body = exc.response.text[:300] |
There was a problem hiding this comment.
When an OpenAI-compatible gateway includes request headers or other sensitive request details in a 4xx/5xx response, these raw 300 characters are appended to LLMRunnerError; the driver then writes that text to per-task-agent.jsonl and summary.md, and publish copies the entire run into the public results bundle. This can therefore persist the sensitive values accepted through --model-header, despite the new interface promising they never enter run artifacts; retain a safe status/error summary or redact secrets before propagating the body.
AGENTS.md reference: benchmarks/AGENTS.md:L8-L11
Useful? React with 👍 / 👎.
| temperature: float | None = None | ||
| else: | ||
| try: | ||
| temperature = float(model_temperature) |
There was a problem hiding this comment.
When --model-temperature is nan, inf, or -inf, float() accepts it and the config validation also permits it, but httpx rejects the resulting request body with ValueError: Out of range float values are not JSON compliant. That exception is not one of _post's handled transport failures, so the whole benchmark aborts after setup instead of producing an explicit errored task and provenance artifacts; validate that the parsed value is finite before constructing the config.
AGENTS.md reference: benchmarks/AGENTS.md:L93-L98
Useful? React with 👍 / 👎.
1 participant
Why
Live testing of the #1398 eval stack — five real-model A/B runs and a full BEAM 100K pass — against merged
main. Every commit on this branch was bought by a real failure; the runs that followed each fix validated it. Fixes #1400's follow-through and produces the tracker's first quotable numbers.What changed (12 commits, in failure order)
BEAM live-data drift (all caught by the deliberate fail-fasts):
event_orderingchat-id groups mix ints with int-lists ([116, …, [136, 138]]) — flatten one level, deeper still fails->-> 2,22, 24and->-> 1,5)— final pattern covers the complete surveyed population (2,199 markers, five shapes, survey in the comment); the)verified as generator junk; unknown->->content still fails fastAgent-task harness correctness:
project addregisters only,statusreported ready vacuously, and all 24 tasks of run 3 saw empty projects (masked by the scripted smoke, whose canned answers never consult the index) — explicitreindex --full --searchbefore settleRelationResolvesSQL both now match the task's own prefix while foreign prefixes still fail (cross-project leakage keeps failing, verified against live run DBs)at-at-double project prefix removedModel-endpoint integration (Anthropic compat, verified live):
--model-header Name=value(identity-linked keys needanthropic-workspace-id); values ride only in the model-factory closure, structurally excluded from run artifacts--model-temperaturewithomit(Claude 5 rejects the parameter); recorded in the run config for provenanceCLI: plain-slice footer no longer visually concatenates onto unterminated content (stdout stays byte-exact for pipes).
What the validated runs produced
4dedb8c85318): bm-local vs mem0-local at retrieval parity (R@10 0.657 vs 0.648 answerable) with half the latency (379 ms vs 746 ms); QA macro 0.498 vs 0.513; BM's only meaningful per-ability deficits are temporal reasoning (−7.2pp) and contradiction resolution (−6.6pp) — exactly SPEC-82 and SPEC-89, now recorded pre-implementation as measurable deltas.Product issues driven by these runs
#1414 (index-on-add + readiness contract + refuse-on-never-indexed), #1415 (project-qualified paths — round-trip asymmetry evidence), plus tracker findings on #1398.
🤖 Generated with Claude Code
https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp