Repository navigation
[agentic] - Fail the harness optimizer's score gate closed on NaN and map malformed prior artifacts to AgenticError - #1647
Conversation
… map malformed prior artifacts to AgenticError decide_candidate's score_not_improved gate is `candidate.score <= baseline.score`, which is False whenever either side is NaN, so a NaN RunReport was accepted. RunReport now rejects non-finite scores and CaseResult's range check rejects NaN/inf. apply_candidate_artifact now maps a wrong-shaped prior artifact ([], null, a non-integer/negative/bool version, non-UTF-8 bytes) to the typed AgenticError instead of a raw AttributeError/ValueError or a silently bogus version. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BJXTdWRbDpkynPYbpL2pxd
|
PR is a complete fill of |
|
@codex review (for bugs to fix and performance/security/other regressions potentially introduced in this pr) |
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f775cd0d4
ℹ️ 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".
…cores keep the typed-error contract math.isfinite(10**1000) raises OverflowError converting the int to float, so the new check turned a previously valid RunReport into an untyped exception and replaced CaseResult's AgenticError range rejection with one (Codex review). Ints are always finite; the check now runs on floats only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BJXTdWRbDpkynPYbpL2pxd
Proposed changes
This PR closes two gaps in
agentic/harness_optimizer/: one gate fails open, and one error escapes the typed-error contract. A/CyClaw-Optimizescan found both, and I reproduced each onmainbefore fixing it.decide_candidaterejects withscore_not_improvedwhencandidate.score <= baseline.score.False, so a NaN score on either side skips the gate.RunReport.__post_init__only checked that the score was numeric.CaseResult's0 <= score <= 1range check also passes NaN.main: a NaN candidate against a 0.7 baseline returnsaccepted: True.RunReportnow rejects non-finite scores (math.isfinite), andCaseResult's range check now rejects NaN and inf.apply_candidate_artifactmapped onlyJSONDecodeErrortoAgenticError. Reproduced onmain:[]andnullraisedAttributeError.{"version": "x"}raisedValueError.{"version": -1}and{"version": true}were silently accepted, producing versions 0 and 2.Fix: the function now raises
AgenticError("existing harness artifact is malformed")when the prior artifact is any of:The artifact is left untouched and the lock is released.
Honest scope: the shipped
GitHubCodingRunnerbuilds scores withfloat(bool), so shipped code cannot produce NaN today.RunReportandCaseResultare public:agentic.harness_optimizerexports them. This change hardens that API to fail closed when a custom runner or direct caller passes NaN; it is not a fix for a live exploit.Invariant / Governance Impact: none of the six invariants is touched.
agentic/.check_invariants.pyreports 46 passed, 0 failed.Types of changes
Scope: Out-of-band agentic layer (
agentic/harness_optimizer/core.py,scoring.py,patching.py, plus two test files).Benefits / why
AgenticErrorthat the agentic CLI maps to its exit codes. Before, these cases produced raw tracebacks or silently wrong version counters.Risks to monitor
versionis a numeric string ("3") or a float (3.0) was previously coerced byint(). It is now refused as malformed. The only writer,apply_candidate_artifact, always writes anint, so this affects only hand-edited files.infnow raises at construction instead of silently winning the gate.tests/test_agentic_harness_optimizer.pycover non-finiteRunReportandCaseResultscores. New tests intests/test_agentic_harness_phase679.pycover five malformed prior-artifact shapes.Checklist
docs/THREAT_MODEL.md,INVARIANTS.mdandSECURITY.md(and any relevant Phase docs)GROK_API_KEY=dummy pytest tests/ -q --tb=short, andbash .claude/skills/CyClaw-Sandbox/verify.shfor core RAG/agentic paths) and passes with no regressions. Not run locally, per the CLAUDE.md §5 verification policy; CI runs the suites.Further comments
Verification I ran directly, following CLAUDE.md §5:
mainvs this branch: onmain, a NaN candidate returnedaccepted: True. On this branch, NaN and inf are rejected at construction, and a normal 0.7 → 0.8 candidate is still accepted.pytest tests/test_agentic_harness_phase679.py -k malformed_priorgives 5 passed. Withpatching.pyreverted tomain, the same selection gives 5 failed: oneValueErrorand four "DID NOT RAISE".ruff check --select E,F,I,B,C4,UP,S --ignore E501is clean.ruff formatdrift before this PR. I did not reformat them, and this diff adds no new drift (the drift count matchesmain).check_invariants.pyreports 46 passed, 0 failed.Suggested merge order of open PRs
Trial merges verified (
git merge --no-ff, all three branches in sequence onto a throwaway checkout) against main @ 6396152 on 2026-10-13. All three touch disjoint files.#1647 (this PR) · Blocked ⛔ (draft, CI pending)
Impact:
agentic/harness_optimizer/only; independent.Action: Merge first (lowest number). Trial merge clean vs main.
#1648 · Blocked ⛔ (draft, CI pending)
Impact:
agentic/fsconnect/indexer.py+ async/filters.pycomment; independent.Action: Merge second; no shared files.
#1649 · Blocked ⛔ (draft, CI pending)
Impact:
memory/store.py,memory/policy.py+ tests; independent.Action: Merge third; no shared files.
ELI5
Think of the harness optimizer as a judge who lets a new version through only if it scores higher than the old one. A NaN ("not a number") score is like a blank scorecard. The judge's rule, "reject if new ≤ old," can't evaluate a blank card, so the candidate got waved through.
RunReportandCaseResultnow refuse blank and infinite scorecards before they reach the judge (decide_candidate).Separately, recording an accepted candidate means reading the previous record to bump its version number. When that file held valid JSON of the wrong shape, the code either crashed with a raw Python error or quietly computed a nonsense version (
-1became 0,truebecame 2).apply_candidate_artifactnow reports "this artifact is malformed" with the project's standard error type and leaves the file alone.What it does not do: it changes no gate other than these input checks, and it touches no CyClaw core path. I did not run the full test suite locally; CI runs it.
🤖 Generated with Claude Code
https://claude.ai/code/session_01BJXTdWRbDpkynPYbpL2pxd
Generated by Claude Code
Last updated: 2026-10-13 09:43 ET