Skip to content

feat(skill): keep non-ACS fixes out of acs-governed run labels. - #325

Open
alex ngo (ango10) wants to merge 2 commits into
mainfrom
ango10/skill-non-acs-fix-guidance
Open

feat(skill): keep non-ACS fixes out of acs-governed run labels.#325
alex ngo (ango10) wants to merge 2 commits into
mainfrom
ango10/skill-non-acs-fix-guidance

Conversation

@ango10

Copy link
Copy Markdown
Collaborator

Summary

Adds one piece of guidance to the run-assert-eval skill: a non-ACS fix (model upgrade, system-prompt rewrite, agent change) must not be recorded under an acs-governed run id. Doc-only, applied identically to all three front-doors.

Motivation / linked issue

No issue. This came out of running the skill end-to-end on a real internal eval.

Step 8 asks the agent to govern a failure with ACS and re-measure to prove the rate dropped. While troubleshooting, a coding agent can reach for a non-ACS fix instead: such as swapping the target model. Those are frequently the right fix, but they are agent changes, not governance. ACS can only restrict or gate an output, it can never add a capability the model lacks.

Nothing in the skill drew that line. So a governed re-measure could bank a model upgrade's improvement under an acs-governed run id and hand ACS credit for a delta it didn't produce, which corrupts exactly the comparison govern-and-remeasure.md exists to make.

Changes

  • .claude/skills/run-assert-eval/SKILL.md: new paragraph after the "Suggested next step" guidance.
  • .cursor/rules/assert.mdc: same guidance as a bullet in the matching list.
  • .github/prompts/run-assert-eval.prompt.md: same guidance in the matching section.

All three say the same thing: non-ACS fixes are legitimate and are the user's call, but they're outside this skill's loop, and the run should be named for what actually produced the numbers (e.g. model-upgrade) rather than labeled acs-governed.

Testing

  • pytest .claude/skills/run-assert-eval/tests/ -x -q: 68 passed (the command CI runs in regression.yml).
  • Confirmed these are the only three front-doors: Suggested next step appears in exactly these files repo-wide, so no fourth surface is left inconsistent.
  • Verified the guidance is self-contained: it references no example, config, or fixture, so it reads correctly on its own.

Checklist

  • Tests pass locally (pytest and/or viewer checks as applicable).
  • Docs updated if behavior or public API changed.
  • No secrets, credentials, or customer data committed.
  • No breaking change, or a CHANGELOG.md entry is included.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The attribution boundary is useful, but the current guidance still permits the confound it is meant to prevent.

  1. Renaming a mixed intervention does not make its delta attributable. The paragraph says that if a governed run improves after a model, prompt, or agent change, name the run for that non-ACS fix instead of acs-governed. If ACS is still active in that run, both interventions changed and neither can receive causal credit; calling the arm model-upgrade merely moves the misattribution. Please say not to mix non-ACS changes into the ACS-governed comparison. If the user chooses one, create a separate arm from the same baseline, reuse the same test set, and name that arm for the agent change. The existing Step 8 rule that the governed config changes only the run ID and governed callable is the invariant to reinforce.

  2. The ACS scope sentence is too narrow. All three mirrors say ACS can only restrict or gate "an output," but this skill's own governance workflow correctly uses ACS at input/output and pre_tool_call/post_tool_call boundaries. That wording can teach the coding agent that ACS is output-only. Something like "ACS can constrain inputs, outputs, and tool calls, but it cannot add a capability the agent lacks" preserves the intended distinction without contradicting the workflow.

The three front doors otherwise carry the same attribution rule, the focused skill suite passes (68 passed), the exact head merges cleanly with current main, and pairwise synthetic merges with #336 and #337 retain the guidance in all three mirrors.

@tangym

tangym commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

I reviewed the diff, and the #336 changes merge cleanly into this PR, so no dependency-related adjustment is required. The only remaining work is the current review feedback. You can even merge it before #336.

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.

3 participants