fix(scorer-gated-skill-edits): close two unclear-agent-instruction gaps from retro #343

[tvna]

Summary

Closes two unclear-agent-instruction gaps issue #343's own retrospective found (a stale cross-reference/ordinal after a bounded edit, and a "restraint check" log entry substituting unrelated dispatches for the named fixture) by extending skills/scorer-gated-skill-edits/SKILL.md's own Procedure and Stop boundaries -- the shared gate every skill's iterative edit goes through, per issue #1444's own scoping decision.

Facts

• Issue chore(retrospective): merge retrospective for PR #335 #343's Repair 1: "Nothing prompted checking whether a new section's own cross-references ... stayed consistent with the edit itself before committing" -- a stale ordinal ("the fourth Mechanism-fit check") and a stale corpus-size math note both slipped past review.
• Issue chore(retrospective): merge retrospective for PR #335 #343's Repair 3: "Nothing enforced that a claimed 'restraint check' in a Kept-edit log entry actually corresponds to a real, isolated dispatch of the named fixture rather than inferred/substituted evidence."
• Both were classified unclear-agent-instruction by issue chore(retrospective): merge retrospective for PR #335 #343's own text, meaning no deterministic gate could have caught them -- confirmed by direct re-read of issue chore(retrospective): merge retrospective for PR #335 #343's own body before implementing.
• skills/scorer-gated-skill-edits/SKILL.md governs exactly the workflow both defects occurred in (step 3 "Propose bounded edits", step 5 "Log rejected edits", step 7 "Record the run") -- confirmed by direct read.
• git diff bc77e90..origin/main (fetched mid-implementation) showed scorer-gated-skill-edits/SKILL.md had just gained a new step 8 (issue chore(retrospective): merge retrospective for PR #216 #218's retrospective) -- re-synced onto that before editing so this PR's diff lands cleanly against it, not a stale base.
• python3 skills/evaluating-skill-quality/scripts/gitapex_check_skill_shape.py skills/scorer-gated-skill-edits: 47/47 checks pass on the candidate, including body-length (456/500 lines), anchor-targets-resolve, and no-step-location-contradiction.
• uv run python3 -m pytest -q --no-cov: 6700 passed, 1 failed (test_gitapex_scan_harden_checkout_pin_drift.py::test_repository_workflows_are_drift_free) -- reproduced identically on unmodified bc77e90 (stashed my changes and re-ran), so this is a pre-existing shallow-clone artifact in this sandbox, not a regression from this diff. Full history was then fetched (git fetch --unshallow) and the local 40-gate preflight's own harden-checkout-pin-drift entry (the same check) passed afterward.
• .github/scripts/gitapex_gate_skill_branch_fixture_coverage.py, run directly against the real before/after content and the real evals/scorer-gated-skill-edits/tasks/*.yaml count (15 files, via git ls-tree): the two new Stop-boundary bullets bring this skill's own decision-branch count to 13; PASS (15 >= 13) -- no new fixture required by that gate's own absolute count-comparison rule.
• .github/scripts/gitapex_gate_transfer_check_disclosure.py (default whole-repo glob): PASS -- the new evals/scorer-gated-skill-edits/split.md entry (this file's first ever ## Iteration: entry) discloses a non-empty ### Transfer check subsection in the standardized post-feat(evals): move split.md machine-read facts to split.json and unify both markdown conventions #928 heading form.
• python3 .github/scripts/gitapex_gate_local_preflight.py: all 40 wired gates PASS on the final candidate.
• evals/scorer-gated-skill-edits/'s own selection split has 7 fixtures (split.json); read all 7 directly -- none probes ordinal/count cross-reference staleness or named-fixture corroboration integrity, so no live measured gate run could register a score change from this edit either way.

Assumptions

• Speculation: no other in-flight PR is concurrently relying on scorer-gated-skill-edits/SKILL.md's current step numbering or Stop-boundary bullet text -- checked .github/workflows/scorer-gated-keep-gate.yml's own step references directly; unaffected, since this diff is purely additive (no step renumbered, no existing bullet reworded).
• Speculation: the .github/scripts/gitapex_gate_skill_branch_fixture_coverage.py and gitapex_gate_transfer_check_disclosure.py CI gates are wired to fire on this PR's diff (both are .github/scripts/*.py gates whose applicability is diff-computed by their own calling workflow) -- not independently confirmed against this specific PR's live CI run at the time of opening, only against local invocation with the equivalent inputs.

Risk / blast radius

• Two files touched: skills/scorer-gated-skill-edits/SKILL.md (prose-only: one sub-step under step 3, one sentence in step 5, one paragraph in step 7, two new Stop-boundary bullets) and references/worked-example.md (two new illustrative sections). No script, schema, or CI workflow file changed.
• evals/scorer-gated-skill-edits/split.md gains its first-ever ## Iteration: entry, disclosed as NOT MEASURED rather than a false KEEP -- does not claim a measured result the corpus cannot support.
• No description: frontmatter change (no new trigger/routing surface), no new script, no new dispatch branch (the two new Stop-boundary bullets are plain Never ... prose rules, not a "token" -> dispatch shape) -- confirmed directly via dispatch_branch_counter in the fixture-coverage gate script, which found 0 dispatch-branch matches in either bullet.
• Worst case if the new prose is itself ambiguous: a future skill edit under scorer-gated-skill-edits reads the new step 3/step 5/step 7/Stop-boundary text and still gets it wrong -- the same residual risk this repository's own issue fix(evaluating-skill-quality): Contract discipline's precondition enumeration has drifted stale from SKILL.md steps 2/4 #406 precedent already disclosed for the identical remediation shape (a prose Stop-boundary addition, not a deterministic gate).

Rollback

git revert the two commits on this branch; no schema, generated-file, or state migration is involved. The two commits are independently revertible (the second is a pure restructuring of the first's own split.md addition, touching no other file).

Verification

Acceptance Criteria Map, restated verbatim from issue #1444 (independently re-verified against the live repository before implementation -- see the Re-verified: marker on that issue):

Criterion	Interpretation	Planned ops	Proof method	Residual risk
"Nothing prompted checking whether a new section's own cross-references... stayed consistent with the edit itself before committing" (#343 Repair 1, verbatim)	When a bounded edit (step 3) changes an enumerated/ordinal item that the target skill's own references/ directory or evals/<skill>/ docs cite, the edit is not complete until every such cross-reference has been checked and updated in the same patch	Add a new sub-step to step 3 requiring a grep sweep of the target skill's own references/ directory and evals/<skill>/ docs for ordinal/count language referencing the changed item; add a matching Stop-boundary bullet	Manual read-through confirming the new text names both the trigger and the required action; a worked-example addition reproducing #343's own incident as a regression narrative	Prose Stop-boundary addition, not a deterministic gate -- depends on a future editor actually following it, the same enforcement-gap class #406 already disclosed for the same remediation shape
"Nothing enforced that a claimed 'restraint check' in a Kept-edit log entry actually corresponds to a real, isolated dispatch of the named fixture rather than inferred/substituted evidence" (#343 Repair 3, verbatim)	A "restraint check" or named-fixture corroboration claim recorded in step 5's rejected-edit log or step 7's run record must name an actual, isolated dispatch of that specific fixture; if not dispatched, the entry discloses the gap explicitly	Extend step 5/step 7 and the Stop boundaries with an explicit anti-substitution rule, distinct from and complementary to the existing isolation-verification Stop boundary (#1410, merged)	Manual read-through confirming the new rule names both the required behavior and the disclosed-gap fallback; a worked-example addition reproducing #343's own incident	Same as row 1 -- a process-discipline instruction, not a mechanically checkable gate; #343's own text already names this explicitly

Results, row by row

1. PASS. skills/scorer-gated-skill-edits/SKILL.md step 3 now reads: "When the edit changes an enumerated or ordinal item that the target skill's own references/ directory (or its evals/<skill>/ docs) cites elsewhere ... grep that skill's own references/ directory and evals/<skill>/ docs for every such reference to the changed item before scoring, and fix each one found in the same patch, not a follow-up," now attributed to "a real incident" rather than a bare issue-number citation (see the Independent review verdict's follow-up-commit note below), and cross-referencing a new references/worked-example.md "Cross-reference sweep before scoring" section that narrates the real incident. A matching Stop-boundary bullet added.
2. PASS. Step 5 gained a pointer sentence, step 7 gained a paragraph, and a new Stop-boundary bullet states the anti-substitution rule in full: "Never record a 'restraint check' or any other named-fixture corroboration claim in a rejected-edit log entry (step 5) or a run record (step 7) unless that specific fixture was actually, independently dispatched. ... If the named fixture was not dispatched, say so explicitly (a disclosed gap in known_gaps, step 7) instead of implying indirect coverage." A matching references/worked-example.md "Restraint-check corroboration must be a real dispatch" section narrates the real incident. Distinct from feat(evaluating-skill-quality,battle-testing-a-skill,scorer-gated-skill-edits): forbid subagent fallback and require a fix-or-handoff code block when isolation cannot be verified #1410's isolation-mechanism Stop boundary (confirmed by direct re-read of both): feat(evaluating-skill-quality,battle-testing-a-skill,scorer-gated-skill-edits): forbid subagent fallback and require a fix-or-handoff code block when isolation cannot be verified #1410 governs whether the dispatch mechanism itself is contamination-free; this rule governs whether the claimed fixture was the one actually dispatched -- orthogonal concerns, not a duplicate.

Commands run (all against the final candidate):

• python3 skills/evaluating-skill-quality/scripts/gitapex_check_skill_shape.py skills/scorer-gated-skill-edits: 47/47 PASS.
• python3 skills/evaluating-skill-quality/scripts/gitapex_scan_execution_requirements_drift.py skills/scorer-gated-skill-edits: no drift.
• uv run python3 -m pytest -q --no-cov: 6700 passed, 1 pre-existing/environmental failure (see Facts).
• LC_ALL=C grep -nP '[^ -~\t]' <every changed file>: clean (ASCII-only).
• python3 skills/outward-artifact-preflight/scripts/gitapex_scan_provenance.py --file <every changed file>: PASS, no candidate provenance markers.
• .github/scripts/gitapex_gate_skill_branch_fixture_coverage.py (real before/after content, real fixture count): PASS.
• .github/scripts/gitapex_gate_transfer_check_disclosure.py (both single-file and default whole-repo glob): PASS.
• python3 .github/scripts/gitapex_gate_local_preflight.py (all 41 wired local gates): all PASS.

Skill audit evidence

• battle-testing-a-skill: WAIVED: this diff touches only scorer-gated-skill-edits/SKILL.md (prose across steps 3/5/7 plus two new Stop-boundary bullets) and its references/worked-example.md. No description: frontmatter change (no new trigger/routing surface), no new script, no new dispatch branch (confirmed: 0 matches for the dispatch-branch pattern in either new bullet, per gitapex_gate_skill_branch_fixture_coverage.py's own dispatch_branch_counter). The two new Stop boundaries are narrow, mechanically-scoped process rules, each paired with a concrete worked-example section grounding it in a real incident so its meaning is unambiguous by construction -- the same mitigation this file's other Stop boundaries already use (e.g. the git-stash-race boundary cites its own worked-example section). Unlike feat(evaluating-skill-quality): implement #218's remaining 3 gate proposals #1426's WAIVED precedent for this same file, this diff does add Stop boundaries, so verified instead via the deterministic shape checker's full pass (47/47, including no-step-location-contradiction, which specifically checks for a "step N" reference asserted to run in two contradicting locations -- directly relevant to the step 3/5/7 cross-references this diff adds) and a manual read-through cross-checking every new sentence against every existing step/Stop-boundary for contradiction or duplication.
• evaluating-skill-quality: WAIVED: same reasoning as above -- proportionate to a small, prose-only, non-code addition to an already-mature governance skill, verified via this repository's own deterministic tooling rather than a fresh isolated dispatch. Residual risk disclosed: an isolated adversarial review of the new prose's own clarity was not run; if a future review surfaces ambiguity in the new Stop-boundary wording, fix it then.
• adversarial-coverage-mapping: WAIVED: scorer-gated-skill-edits is a gate/verification skill by nature (security-relevant by the workflow's own keyword heuristic), but this diff adds no new attack surface -- two prose Stop-boundary rules with no automated action and no code-path change, matching PR feat(evaluating-skill-quality): implement #218's remaining 3 gate proposals #1426's own reasoning for the same file.
• checker-script-adversarial-review: N/A -- no skills/*/scripts/*.py, evals/scripts/*.py, or .github/scripts/*.py file touched by this PR.
• deterministic-gate-quality: N/A -- no deterministic gate/checker script added or modified; the two existing gates this PR's own diff was checked against (gitapex_gate_skill_branch_fixture_coverage.py, gitapex_gate_transfer_check_disclosure.py) were read and run against this diff's real inputs, not modified.
• defeat-test-disclosure: N/A -- no new test-backed detection logic added; this PR is a skill-instruction (prose) change with no code to defeat-test.

Checklist

• Tests pass locally
• Docs updated if behavior changed (the two new Stop-boundary bullets and the two new references/worked-example.md sections are the documentation)
• Issue number cited in every commit
• Skill audit evidence section discloses the required verdicts/waivers (see above)
• This PR adds a new Kept-edit-log entry to evals/scorer-gated-skill-edits/split.md -- that entry discloses a ### Transfer check subsection (see Facts; verified via gitapex_gate_transfer_check_disclosure.py)
• This PR adds Stop-boundary bullets to scorer-gated-skill-edits/SKILL.md -- evals/scorer-gated-skill-edits/tasks/*.yaml's existing 15 fixtures already meet or exceed the resulting 13-branch decision-branch count (see Facts; verified via gitapex_gate_skill_branch_fixture_coverage.py against the real before/after content), so no new fixture was required by that gate's own rule

Merge gate: independent review

This PR is also subject to the independent-review-pending required status check. It stays pending/failing until a ## Independent review verdict section naming this PR's current head commit is recorded in this body -- drafting-a-pr-to-merge's own Step 8 records it once its independent review completes. Not pre-filled here.

Independent review verdict

• Verdict: CLEAN
• Verified commit: 52efa18

Outer layer: did not run. Neither Anthropic's "Claude Code Review" GitHub App nor GitHub Copilot's automated PR review is confirmed configured/reachable for this repository in this environment (matching PR #1426's own same-day precedent for this same repository). CodeRabbit and Devin both left automated comments on this PR; both explicitly disclosed they skipped review (CodeRabbit: "This repository does not receive automatic reviews because it has fewer than 10 stars"; Devin: "Full review skipped: trial expired and no credits remaining") -- neither is a substitute for this step's own outer layer, and neither produced a findings-bearing verdict to fold in here.

Inner layer: mandatory, ran against this PR's full diff (reviewing-an-artifact, low effort) at commit 079dc46d. The target is mixed (two skills/scorer-gated-skill-edits/ files plus one ordinary evals/scorer-gated-skill-edits/split.md doc), so per that skill's own Step 0 the SKILL.md/references/worked-example.md portion was deferred to a dedicated pass applying evaluating-skill-quality's own review spirit (a fresh, isolated Agent-tool subagent with no memory of this session's own authoring), and evals/scorer-gated-skill-edits/split.md was independently classified safe under Step 1 (a narrative log entry -- prose only, no code, no security-tier signal) and correctly skipped Steps 2-5 per that skill's own rule.

The deferred pass returned two candidate findings, both independently verified (Step 3's own FABRICATED pre-check plus counterfactual check) rather than accepted at face value:

• Confirmed and fixed (commit 9ef849ee): the new step 3 cross-reference-sweep sub-step sat immediately after the pruning-only eligibility definition without stating whether fixing a stale ordinal/count citation elsewhere counts as the "reword" that disqualifies an otherwise deletion-only candidate from pruning-only status. Fixed by stating explicitly that this sweep-and-fix is mechanical citation upkeep, not new behavior, and does not by itself change that classification.
• Investigated and rejected (no code change): the same pass flagged "Kept-edit log entry"/"Kept-edit entries" wording this branch's own commits introduce as stale post-feat(evals): move split.md machine-read facts to split.json and unify both markdown conventions #928 terminology, citing gitapex_gate_transfer_check_disclosure.py's docstring (the literal ## Kept-edit log heading was retired in favor of ## Iteration:). Independently re-verified against evals/evaluating-skill-quality/split.md -- this repository's own largest, most actively maintained split.md -- which still uses "Kept-edit log"/"Kept-edit log entry" as live descriptive prose at its own most recent entries, well after the feat(evals): move split.md machine-read facts to split.json and unify both markdown conventions #928 migration. Only the literal heading was retired; the descriptive phrase this diff's own wording matches remains current usage, so this finding did not hold and no change was made for it.

No finding survives against commit 079dc46d. Re-verified after the fix: gitapex_check_skill_shape.py (47/47), ASCII-only, and gitapex_gate_skill_branch_fixture_coverage.py (unchanged PASS -- the clarifying sentence added no new Stop-boundary bullet or dispatch branch).

Follow-up commit 729d1da8 (this section's own current head, 52efa180 after a base-branch merge): per explicit operator instruction, removed the three repository-local issue-number citations ("Issue #343: ...") this branch's own commits had introduced into skills/scorer-gated-skill-edits/SKILL.md and references/worked-example.md -- this skill declares portability: Mixed, and a bare issue citation in its own portable content would dangle once vendored into another repository's tracker. A purely mechanical text substitution (the lead-in phrase only; no change to the narrative, the rule, or any Stop boundary), verified by direct diff inspection (confirmed via grep that no issue-number citation this branch introduced remains, and that evals/scorer-gated-skill-edits/split.md -- an internal audit log, not portable skill content -- was correctly left untouched) plus gitapex_check_skill_shape.py (47/47) and ASCII-only, rather than a fresh isolated subagent dispatch -- disclosed as a deliberate proportionality judgment for a wording-only change, not an oversight.

Limitation, disclosed rather than glossed: the deferred pass's single Agent-tool subagent dispatch is prompt-level isolation (no memory of this session's own authoring), not the formal positive/negative sentinel-control isolation verification evaluating-skill-quality's own references/adversarial-self-audit.md describes for confirming no project instruction file leaked in -- not run here, disclosed rather than assumed equivalent, matching PR #1426's own identical disclosure for the same class of dispatch.

Related Issue

Closes #1444

Refs #343

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 1da068e3-a1cd-4f8d-9d1e-26f6e18fa76a

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.57%. Comparing base (0f6b92a) to head (52efa18).
⚠️ Report is 248 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1453   +/-   ##
=======================================
  Coverage   99.57%   99.57%           
=======================================
  Files         127      127           
  Lines       22539    22539           
  Branches     2766     2766           
=======================================
  Hits        22443    22443           
  Misses         96       96           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tvna]

eval-gate is failing on this PR's head commit (625649d2), not a required status check per .github/rulesets/main.json (required_status_checks lists actionlint, ruff, pytest, the four pytest-bash-oracle-* jobs, coverage-combine, mypy, exception-handler-gaps, hidden-characters, plugin-root-brace-notation, provenance-disclosure, pr-title-convention, betterleaks, independent-review-pending -- eval-gate is absent from that list).

Root cause, established by direct log inspection: the workflow correctly detects this PR touches scorer-gated-skill-edits's eval corpus and invokes evals/scripts/gitapex_run_eval_suite.py --eval-yaml evals/scorer-gated-skill-edits/eval.yaml --skill-md skills/scorer-gated-skill-edits/SKILL.md, which shells out to the real claude CLI via gitapex_run_ablation.py's subprocess_executor. That subprocess exits 1 with an empty stderr almost immediately (under 2 seconds both times -- before any fixture dispatch could plausibly complete), surfacing as error: model CLI exited 1: with nothing after the colon. Reproduced identically on a re-run (job re-queued via rerun_failed_jobs, second attempt failed the same way in ~2s) -- not a one-off flake.

This is not this PR's own defect: this PR adds no code, touches no evals/scripts/*.py file, and does not modify .github/workflows/eval-gate.yml or gitapex_run_ablation.py's subprocess_executor/_hermetic_env. list_workflow_runs for eval-gate.yml shows exactly one prior run (a rename-only PR, #1372, "no job logic changes") -- this appears to be the first time the workflow's live per-skill dispatch loop has actually executed end-to-end since the file was renamed, so there is no working baseline to compare against on main either. No existing fix for this CLI-startup failure was found (searched for related open issues; none cover a model CLI exited 1 failure in this workflow).

Per this repository's own CI-triage convention, standing down here rather than widening this PR to debug gitapex_run_ablation.py's own subprocess invocation, which is unrelated code outside this issue's (#1444) stated scope. The one re-run budget for this failure is now spent; leaving this for the repository owner to investigate as a separate, eval-gate-scoped infrastructure issue.

---

Generated by Claude Code