Skip to content

fix(evals): let run-validity gate a base side whose unfired runs are the measured change - #11982

Merged
kyle-sexton merged 2 commits into
mainfrom
fix/11979-measured-fire
Nov 1, 2026
Merged

kyle-sexton merged 2 commits into
mainfrom
fix/11979-measured-fire

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

Closes #11979

Summary

run-validity.py marked a whole result file INVALID whenever a should-trigger case did not fire. On the base side of a change that makes a skill fire, not firing is the quantity being measured, so that base file could never gate VALID however clean its runs were (#11895's shape-and-depth-nested-radius: base fired 0/3 with no error or denial and still failed the gate). --measured-fire <case> names such cases; their misses are reported as a measured rate, the way invocation probes already are.

Fix

  • plugins/evals/skills/plugin-eval/scripts/run-validity.py: new repeatable --measured-fire CASE; check_fired treats a named case like an invocation probe (misses become measured: lines and a WARN). Every other check, and every unnamed case, is unchanged. Docstring updated.
  • plugins/evals/skills/plugin-eval/scripts/test_run_validity.py: two tests. A file whose only defect is an unfired run gates INVALID without the flag and VALID with it; a second unfired case not named stays INVALID with the flag.
  • plugins/evals/skills/plugin-eval/reference/reading-results.md: says when to pass the flag.
  • .changes/evals/fix-11979-measured-fire-5c1e8d20.md.

Verification

  • run-validity.test.sh: 101 tests, OK. The new tests fail on main, which rejects the unknown flag.
  • scripts/run-ruff.sh check on both Python files: all checks passed.

Related

🤖 Generated with Claude Code

…the measured change

run-validity.py takes --measured-fire <case> (repeatable): the skill-fired
check reports that case's misses as a measured rate, as for an invocation
probe, so the base side of a change that makes a skill fire can gate VALID.
Every other check, and every unnamed case, is unchanged.

Closes #11979

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

chatgpt-codex-connector Bot commented Nov 1, 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-11-01T16:16:27.419280Z 3302285 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: 3302285336

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

@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed fa090fd6ead3915a4c8f0c7a3c723199a37ca546: read 3 of 3 changed file(s); 0 finding(s), 0 as inline comments.

Security review (incremental, since 3302285)

I found no security issues in this change.

What I read: every hunk of .claude-lane/incremental.diff, which covers 3 files.

  • plugins/evals/skills/plugin-eval/reference/reading-results.md: removes an issue link from the doc. No instruction is deleted, narrowed or softened.
  • plugins/evals/skills/plugin-eval/scripts/run-validity.py:
    • measured_fire now defaults to frozenset() instead of a tuple, and main passes frozenset(args.measured_fire).
    • The new label variable picks the wording of the report text: "measured cases" or "invocation probes".
    • Only display text changes. Gating logic, file handling and subprocess use are unchanged.
  • plugins/evals/skills/plugin-eval/scripts/test_run_validity.py: adds two assertions on the new wording.

Other checks:

  • None of these files match a trusted path, and none is a trust control or a plugin manifest.
  • No settled-threads file was supplied.
  • The PR context file contains no embedded instructions to the lane.
  • The change spends nothing and needs no hold.

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 33022853369ae7986028ace7c7f35d6fbdf5ea24; the summary is in this lane's pull request comment.

claude-lane: claude-review

@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed fa090fd6ead3915a4c8f0c7a3c723199a37ca546: read 3 of 3 changed file(s); 0 finding(s), 0 as inline comments.

Incremental review (3302285 → fa090fd): no findings.

I read every hunk of the three changed files in .claude-lane/incremental.diff, then checked them against the full check_fired body at the head commit.

  • run-validity.py: check_fired now chooses its wording from the case kinds. When any measured tally comes from a case named with --measured-fire, the WARN detail, the "no should-trigger case outside the …" line and the verdict tail say "measured cases". When all tallies come from invocation probes, they still say "invocation probes".
    • The default changed from () to frozenset(), and the call site at main changed from set(...) to frozenset(...). This keeps set(measured) & measured_fire valid: set & tuple would raise TypeError, and set & frozenset does not.
    • check_fired has one caller, and it passes a frozenset, so no caller gets the TypeError.
    • When a run has both probes and named cases, everything is labelled "measured cases". Probes are measured too, so the wording is still accurate.
  • test_run_validity.py: the existing measured-fire test now asserts measured cases fired in 0 of 1 and that invocation probes does not appear. If the old wording comes back, the test fails.
  • reading-results.md: removes only the inline issue link. The guidance on when to pass --measured-fire is still there, so the closing issue's third Done-when item is still met.

Closing issue #11979: the incremental change keeps the earlier-reviewed flag behaviour and tests as they were. I did not run the tests. The author says run-validity.test.sh passes, which I have not verified.

No embedded instructions in the PR context. Nothing in this change spends money or prepaid credit.

…issue link

The skill-fired check labelled every measured tally "invocation probes", so a
--measured-fire case read as a probe in the WARN line and the verdict. The
label is now "measured cases" whenever a named case is among them. The skill
reference drops its issue link (skill criteria: no issue numbers in a skill).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 1, 2026
Merged via the queue into main with commit 602932a Nov 1, 2026
58 of 59 checks passed
@kyle-sexton
kyle-sexton deleted the fix/11979-measured-fire branch November 1, 2026 16:42
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 run-validity gate a base side whose unfired runs are the measured change

1 participant