Skip to content

fix(discipline): widen eval graders that reject correct runs - #8103

Merged
kyle-sexton merged 3 commits into
mainfrom
work-resolve-issue/issue-7440-39105818532-1
Oct 19, 2026
Merged

kyle-sexton merged 3 commits into
mainfrom
work-resolve-issue/issue-7440-39105818532-1

Conversation

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

Refs: #7440

Hold: a human merges this

Summary

Opened by the work-resolve-issue lane. The run's own summary:

Widened the discipline outcome-suite graders named in the issue so they accept the correct phrasings they rejected, each with must-pass and must-fail samples. Tightened hmh performer-labeled to require a label on every step and added a swa grader against Skill-tool calls to other correctors.

Not verified

  • The eval validator (validate-cases.py) could not be run here (Bash allows only the named wrappers), so sample verdicts are hand-traced, not machine-checked. Run it on plugins/discipline/evals before merge.
  • uys reads-the-brief now checks the reply mentions the brief rather than a Read/Bash call, so it is weaker evidence of reading.
  • Issue comment findings from #7441 and the paid rescore/rerun are out of scope per the brief.
  • Issue stays open (Refs) because the rescore/rerun follows merge.

Fix

  • uys-unnamed-skills-brief/case.yaml: reads-the-brief is now a tool-agnostic regex on the reply (tool_used matches one tool name only, so Read-or-cat cannot be expressed)
  • rau-tsconfig-upstream/case.yaml: undocumented-divergence-surfaced accepts 'no written reason' and 'adopt the default or record why'
  • ror-second-error-way/case.yaml: second-way-named accepts 'a second one' and 'a different approach' in error context
  • swa-session-start-digest/case.yaml: no-audit-stated accepts 'didn't audit'; new no-other-corrector-skills grader (Skill tool, max 0)
  • mym-buried-answer-draft/case.yaml: 'came/was last'; cut word before the tool names
  • hmh-runner-setup-phase-one/case.yaml: advancing-reply-literal gap widened to 120 chars with backticks allowed; performer-labeled requires every numbered step or table row to be labeled
  • samples/*.json for each grader above (new no-other-corrector-skills.json)
  • .changes/discipline: patch fragment

Verification

The run reports:

Left out: the items under Not verified.

Related

#7440

Refs #7440

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

github-actions Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

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

No security issues found in this pull request.

The PR changes only discipline eval graders: regex patterns and must-pass and must-fail samples in case.yaml and samples/*.json under plugins/discipline/evals/, plus a changelog fragment. It touches no workflows, hooks, permissions, secrets, tokens, command execution, or standing instructions (CLAUDE.md, AGENTS.md, rules, skill or agent bodies). The Actions hardening and instruction-surface-deletion lenses don't apply.

The widened regexes run only inside the eval grader on model output. The longest one (the hmh performer-labeled pattern) uses lookaheads over [\s\S]*?, which could backtrack heavily on large inputs. That would at worst slow an eval run, so it is not a security finding.

I did not consult the instruction exception register, because no standing instruction was removed or softened.

@github-actions

github-actions Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

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

Code review: no blocking findings

I hand-traced each changed grader pattern against its pass and fail samples, and the existing samples against the new patterns. I found no defect that a careful reviewer would block on. I could not run validate-cases.py. It needs approval in this lane, so none of the sample verdicts are machine-checked. The PR body asks for that run before merge, and I agree it should happen.

What I checked

  • hmh performer-labeled: the start-of-input anchor, the "any unlabeled numbered step or table row rejects" lookaheads and the "at least one label" lookahead are consistent with each other. All existing and new pass and fail samples come out as labeled. The existing first pass sample (1. (you) … steps with prose lines between them) is unaffected.
  • hmh advancing-reply-literal: the widened gap (120 characters, backticks allowed) matches the new "like this:" samples. The command-fence exclusion still rejects the new long-gap fail sample.
  • rau, ror, mym, swa no-audit-stated: the negation lookbehinds sit directly before the new phrases. Each new fail sample is blocked as claimed: "not a second one", "not a different approach", "did not cut …", and the rau reason that belongs to strict, where the tempered gap stops at strict.
  • swa no-other-corrector-skills:
    • It uses the same tool_used fields as the existing ror and sdc cases (tool, input_match, min, max, arm).
    • The prompt's allowed_tools includes Skill, so the grader is not vacuous.
    • The corrector list matches the directories under plugins/discipline/skills, minus setup and sweep-all.
    • The sample file's tool_used answers use the call-list shape the validator documents.
  • Changelog fragment: a patch fragment under .changes/discipline. No path-scoped rule under .claude/rules is broken. The new samples are synthetic and hold no transcripts or machine paths.

Observations (not findings)

  • uys reads-the-brief: it now only checks that the reply says "the brief" or delegation-brief. The prompt itself says "check the brief in notes/delegation-brief.md", so a reply that echoes it passes without reading the file. The PR body discloses this. brief-names-the-skill still needs an edited file, and an edit requires a prior read, so the outcome is still covered indirectly.
  • mym buried-answer-finding: the new (?:…|was|…) last branch ends in \b. A phrase like "the answer was last updated in March" would count as a finding. This is the same weakness is last already had, and I judged it too minor to flag.
  • Embedded text: I found no instructions to the reviewer in the PR body. "Hold: a human merges this" is a merge hold, not a directive to this lane.
  • Closing issues: the PR uses Refs: #7440, so there are no closing-issue acceptance criteria to check. Issue discipline eval suite: widen graders that reject correct runs #7440's text was not in the PR context file, so I did not compare the change against it.

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

pr-unblock / address-feedback finished round 1 of 3 on 78af256.

@melodic-automation-lanes melodic-automation-lanes Bot added the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 18, 2026
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

The pr-unblock lane stopped on this pull request and needs a person: the lane did not record a disposition for every review thread it kept, so none was posted.

  • Reason: feedback-incomplete
  • Resume key: 78af256c8118ddd6be652215aeb7e8fa0a8517d8 (the head the lane stopped on)

To resume, deal with the reason above, remove the needs-human label (and do-not-merge, if set), then dispatch the pr-unblock workflow with pr-number=8103 and activity=address-feedback:

gh workflow run pr-unblock.yml -R melodic-software/claude-code-plugins -f pr-number=8103 -f activity=address-feedback

kyle-sexton added a commit that referenced this pull request Oct 18, 2026
…ectory

A kept review thread's scope is now its path, any file in the
plugins/<p>/ directory that path sits in, and a test file beside the
path. This replaces the per-eval-case rule, which still refused real
fixes: #8178 (plugins/planning/README.md, run 39117146913) on top of
#8043, #8103 and #8142. .github/ stays refused.

thread-scope.mjs and the feedback-thread.sh jq rule change together;
tests, the contract, comments-ci.md, the convention CHANGELOG (2.11.4)
and the source-control fragment are updated.

Refs: #8109
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Oct 18, 2026
…the plugin-wide scope

The plugin-wide part of a kept thread's scope now leaves out anything
under hooks/, .claude-plugin/ or a .claude/ directory, any CLAUDE.md or
AGENTS.md, and any .sh, .py, .mjs, .js or .ts file. A thread's own path
and a sibling test file stay in scope. thread-scope.mjs and
feedback-thread.sh apply the same rule; tests cover the four recorded
trips (#8043, #8103, #8142, #8178) and refusals for each excluded kind.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
kyle-sexton and others added 2 commits October 18, 2026 23:17
…ves against negation

The widened second-way-named alternatives for "a second one" and
"a different approach/way" had no (?<!\bnot )(?<!\bno ) guard, so a
negated reply in error context still passed. Adds the guard to all four
alternatives and a negated, error-context must-fail sample for each.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 19, 2026
Merged via the queue into main with commit 2fe2a5a Oct 19, 2026
18 checks passed
@kyle-sexton
kyle-sexton deleted the work-resolve-issue/issue-7440-39105818532-1 branch October 19, 2026 04:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant