Skip to content

fix(evals): judge invocation-probe runs by their measured rate in run-validity - #11639

Merged
kyle-sexton merged 4 commits into
mainfrom
fix/11618-probe-run-validity
Oct 31, 2026
Merged

kyle-sexton merged 4 commits into
mainfrom
fix/11618-probe-run-validity

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

Closes #11618

Summary

run-validity.py judged every trigger-measurement run INVALID, because it treated a should-trigger miss, the quantity such a run measures, as a validity failure. It did the same with a turn cap hit after the graded Skill call. This PR teaches it to recognize invocation-probe cases (the invocation-probe tag measure-invocation.sh emit-plugin-eval writes) and judge them by the rate they measure.

Fix

  • skill fired: in an invocation-probe case, a miss is reported as the measured rate per case and overall, as a WARN, never a FAIL. Other cases are unchanged.
  • run errors: in an invocation-probe case, a "Reached maximum number of turns" error in a run whose trace holds a Skill call is a WARN detail. Any other error, a cap with no Skill call, and every non-probe case still FAIL.
  • The module docstring describes both. A small prompt_fields helper is shared with is_control.
  • An evals patch changelog fragment.

Verification

Related

🤖 Generated with Claude Code

kyle-sexton and others added 2 commits October 31, 2026 12:23
…-validity

In a case tagged invocation-probe, a should-trigger miss is reported as
the measured trigger rate (WARN), and a max-turns run error in a run
whose trace holds a Skill call is a WARN. Other errors, a max-turns
error with no Skill call, and every non-probe case still FAIL.

Refs: #11618

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review October 31, 2026 16:50
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-31T16:52:14.992606Z 5ea905e Draft marked ready
ℹ️ 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" or "@codex security review".

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 chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5ea905e43c

ℹ️ 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".

Review on #11639: trace_error_text joined a trace's errors into one
string, so a turn cap beside another error (an API error) passed the
max-turns match and the whole trace was waived. Read each error and
exempt only when all of them are the turn-cap error. Add a test with
both errors, which fails on the previous code.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 50a64e2279a380374b246078b47fba6839004b62: 0 finding(s), 0 as inline comments.

No security findings in the incremental change since ea154fa.

I read .claude-lane/incremental.diff, which covers run-validity.py and test_run_validity.py. I did not read the PR context or the full diff, and I did not open the head files. The review rests on the incremental diff alone.

The change tightens an eval-validity check, so it narrows what the check lets through:

  • Turn-cap exemption now needs the case's own Skill grader. It applies only when that grader shows the skill under test was called, instead of any Skill tool_use in the trace. A probe that capped after calling a different skill is now INVALID.
  • Turn-cap error text must match in full. It uses fullmatch on the stripped text, with an optional exit N: prefix. An error that merely contains the turn-cap phrase no longer qualifies.
  • Tests added for both cases.

Nothing here touches a trust control, token scope, kill switch, merge hold, plugin manifest or instruction surface. It adds no code execution, no injection surface and no secrets handling. The regex is anchored and has no nested quantifiers, so there is no ReDoS risk. No file in this increment matches a trusted path.

Spend: none. The change adds no eval or lane runs.

manifest fields: none touched.
critical-findings: 0

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

claude-review: 1 inline finding(s) on 5ea905e43cdec588106d39fec71ae0ef43316f8c; the summary is in this lane's pull request comment.

claude-lane: claude-review

@github-actions

github-actions Bot commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed 50a64e2279a380374b246078b47fba6839004b62: 0 finding(s), 0 as inline comments.

Incremental review of run-validity.py and test_run_validity.py since ea154fa: no findings. I read the diff and the head code and did not run the tests.

What changed. The turn-cap exemption now requires that the case's own Skill grader show the call was made (graded_call_made: a requiring grader passed, or a max-0 grader failed). It replaces the old any-Skill-tool_use trace check. The error text must also match in full (MAX_TURNS_ERROR.fullmatch(t.strip()), with an optional exit N: prefix). Two tests were added: a probe capped after calling another skill is INVALID, and a turn-cap string embedded in another error is INVALID.

Checked:

  • graded_call_made uses constants already defined in the file, and its rule mirrors skill_graders at plugins/evals/skills/plugin-eval/scripts/run-validity.py:306-326.
  • The probe cases from measure-invocation.sh emit-plugin-eval use a max-0 tool_used: Skill grader with input_match set to the skill under test (plugins/skill-quality/scripts/measure-invocation.sh:581-585). The new logic fits real probe output.
  • The test fixture's grader names, names-conftest and no-skill-fired, match the case definition in fixtures/run-validity/r2-clean-runs.json. The graded parameter drives passed as intended, and the existing WARN-path tests keep their defaults.

Not worth a comment:

  • graded_call_made takes a root argument it never uses.
  • A max-0 grader scoped by input_match to a different skill would count as "called" if it failed. The emitted probes never produce that, and the exemption also needs the invocation-probe tag.

Manifests: the incremental diff touches none, so there are no manifest <field> lines.
Closing issue #11618: the incremental change only tightens the exemption. I saw nothing that regresses its "Done when" criterion.
Spend: none. No hold.

Review on #11639: the waiver accepted any Skill call in the trace and
matched the turn-cap text anywhere in the error. It now reads the run's
own Skill grader results (a requiring grader passed, or a max-0 grader
failed) and matches the whole error text. Two tests, a call to another
skill and a turn cap inside another error, fail on the previous code.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(evals): let trigger-measurement probe runs pass run-validity

1 participant