fix(evaluating-skill-quality): re-scope check_skill_shape.py rows 5 and 7 (#577)

[tvna]

Summary

Re-scopes check_skill_shape.py rows 5 and 7 from #192 (both had a
"None identified" residual-risk claim that direct inspection of the
real target files found wrong). Row 5 ships as a narrowed
dimension-quote-exemption-cross-reference check; row 7 is dropped,
not implemented, with the reason recorded on #192. Also brings in
origin/main's shape_checks/ package split (#1450/#1330), ports the
new check onto that layout, and retires that PR's own one-time,
now-unrepeatable output-diff verification harness (see Facts).

Facts

• skills/battle-testing-a-skill/references/adversarial-dimensions.md
  has 22 numbered dimensions; a per-dimension backtick-coverage scan
  (reproduced independently, matches the table in chore(evaluating-skill-quality): re-scope check_skill_shape.py rows 5 and 7 from #192 #577's own body)
  found 14 of 22 with zero backtick content anywhere in their section,
  and 0/22 Pass examples with backtick content. Only dimension 14
  carries an explicit "evidenced by inspecting the target's evals/
  directory, not by quoting a line from its SKILL.md" exemption.
• skills/battle-testing-a-skill/SKILL.md's own Procedure requires
  every finding to "quote the exact offending line," with dimension 14
  named as the one exception -- the real chore(retrospective): merge retrospective for PR #75 #79/PR feat(battle-testing-a-skill): add dimensions 11-17 (cross-skill, supply-chain, memory, regression corpus, multi-turn, encoding, structured-output) #75 defect was this
  exemption drifting out of sync between the two files.
• Neither worked-example-self-review.md nor
  worked-example-explaining-the-work.md (evaluating-skill-quality)
  uses a closed per-dimension status vocabulary; only the whole-skill
  ## Verdicts section's four tokens are closed vocabulary, answering
  a different question than row 7 asked.
• origin/main merged PR refactor(evaluating-skill-quality): split gitapex_check_skill_shape.py into shape_checks package (#1330) #1450 (issue refactor(evaluating-skill-quality): split gitapex_check_skill_shape.py, already excluded from the complexity gate at 6149 lines #1330) mid-session, splitting
  the ~6300-line gitapex_check_skill_shape.py monolith into a
  shape_checks/ package. That PR also added
  verify_shape_check_output_diff.py + its test, a one-time proof
  (pinned to a hardcoded pre-split BASE_SHA) that the split itself
  was a pure code-motion. Its own _assert_pre_split_source guard
  rejects any post-split BASE_SHA, so that pin can never legitimately
  advance -- test_main_end_to_end_against_the_real_repository cannot
  pass again after any future real check_shape() detection-logic
  change. This PR is that first change, so it retires both files
  (confirmed via independent review: no reference beyond one now-fixed
  docstring pointer in shape_checks/orchestrator.py, not registered
  in .gitapex/ssot.json, fully recoverable via
  git show aa33e6f7:skills/evaluating-skill-quality/scripts/verify_shape_check_output_diff.py).
  A follow-up gate-proposal issue ("one-time migration-proof tests must
  be self-retiring") will be filed separately.

Assumptions

• The repository owner's direct confirmation (during this session) that
  row 5 should be narrowed to the real defect class rather than
  widened to cover all 22 dimensions, and that row 7 should be dropped
  rather than retrofitted, is treated as the scoping decision chore(evaluating-skill-quality): re-scope check_skill_shape.py rows 5 and 7 from #192 #577
  itself asked for -- not re-litigated here.
• The repository owner's separate direct confirmation to retire
  verify_shape_check_output_diff.py/its test (after an independent
  subagent analysis of the alternatives) is treated as authorization
  for that deletion, since it removes another recently-merged PR's
  artifact.

Risk / blast radius

• New check runs unconditionally inside check_shape(), which every
  skill directory in the repo is scanned with (CI's
  tests/test_gitapex_repository_skill_shape.py sweep, and the
  skill-shape-check pre-commit hook). Verified it passes against
  every real skill in the repo today (0 failures), including its own
  motivating example, battle-testing-a-skill.
• Deleting verify_shape_check_output_diff.py removes dead-end
  verification tooling only; no other script imports or invokes it.

Rollback

• Revert this PR. The retired verification harness is separately
  recoverable via git show aa33e6f7:skills/evaluating-skill-quality/scripts/verify_shape_check_output_diff.py
  (and its test file at the same commit) if ever needed again.

Verification

Acceptance Criteria Map (from #577, re-verified against the real repo):

Criterion	Interpretation	Planned ops	Proof method	Residual risk
Row 5 has a viable, corpus-compatible design	Resolved as (a). Narrow the check to the real #79 defect class (a SKILL.md-named quoted-line exemption drifting out of sync with its references/ catalog), not a blanket per-example backtick requirement.	Added _dimension_quote_exemption_offenders/_dimension_quote_exemption_checks in skills/evaluating-skill-quality/scripts/shape_checks/citation_checks.py, registered in check_shape()	7 new unit tests (including 2 code-review defeat tests below); tests/test_gitapex_repository_skill_shape.py's real-skill sweep passes for every skill, battle-testing-a-skill included	None identified against the current corpus; intentionally narrower catch rate than #79's original ask
Row 7 has a viable, corpus-compatible design	Resolved as (a): dropped. No closed per-dimension status vocabulary exists in either worked-example file to check against.	None -- no check implemented	N/A	None -- recorded as a decision on #192

Commands run and their output:

$ uv run pytest -q --no-cov
6917 passed in 85.35s

$ uv run ruff check . && uv run ruff format --check .
All checks passed! / files already formatted

$ uv run mypy skills/evaluating-skill-quality/scripts/gitapex_check_skill_shape.py
Success: no issues found in 1 source file

$ python3 .github/scripts/gitapex_gate_local_preflight.py
local preflight: all 42 wired gate(s) passed.

Real-corpus proof (row 5's own proof-method requirement -- the check
does not fail on the current, already-reviewed corpus):

>>> check_shape(Path("skills/battle-testing-a-skill"))
CheckResult(name='dimension-quote-exemption-cross-reference', passed=True,
            evidence='none')
# every other skill in the repo: 0 failures for this check name

Checklist

• Tests pass locally
• Docs updated if behavior changed (N/A -- no user-facing docs; issue
  chore(evaluating-skill-quality): re-scope check_skill_shape.py rows 5 and 7 from #192 #577 and feat(check-skill-shape): additional static SKILL.md checks (retro triage) #192 updated with the resolved ACM/decision)
• Issue number cited in every commit
• If this PR adds/modifies a skills/*/SKILL.md, a
  docs/superpowers/specs/*.md design doc, a security-relevant
  skill, or a deterministic checker script
  (skills/*/scripts/*.py, evals/scripts/*.py,
  .github/scripts/*.py), a ## Skill audit evidence section
  discloses the required verdicts/waivers -- see below
• If this PR adds a new Kept-edit-log entry to any evals/*/split.md
  -- N/A, no such entry added
• If this PR adds or increases a skills/*/SKILL.md's Stop-boundary
  bullets or named dispatch branches -- N/A, no SKILL.md changed

Skill audit evidence

checker-script-adversarial-review: RAN -- /code-review (high effort)
against the new dimension-quote-exemption-cross-reference detection
logic found and this PR fixed two real defects before merge: (1)
QUOTED_LINE_RULE_RE/DIMENSION_QUOTE_EXEMPTION_RE used literal
spaces, so they never matched battle-testing-a-skill's own real
SKILL.md, which hard-wraps the rule phrase across a line break -- the
check silently no-opped on its own motivating example; (2) SKILL.md
text was scanned without fenced-block/inline-code stripping, unlike
every other check built on _citation_sources, so a "do not write
like this" illustration would have been read as a real assertion. Both
fixed with \s+-based regexes and the standard
_strip_illustrative_spans(_blank_fenced_blocks(...)) pipeline.

defeat-test-disclosure: RAN -- two tests specifically constructed to
defeat (not just exercise the happy path of) the new detection logic:
test_dimension_quote_exemption_line_wrapped_phrase_matches (a
line-wrapped fixture reproducing defect 1 above) and
test_dimension_quote_exemption_fenced_illustration_excluded (a
fenced-illustration fixture reproducing defect 2 above), plus two
existing-shape mismatch-direction tests
(test_dimension_quote_exemption_skillmd_names_exempt_catalog_silent_fails,
test_dimension_quote_exemption_catalog_marks_exempt_skillmd_silent_fails).

Independent review verdict

• Verdict: CLEAN
• Verified commit: 8af21e1

Outer layer: neither Anthropic's Claude Code Review GitHub App nor
GitHub Copilot's review is configured/reachable for this repository --
request_copilot_review produced no review (only CodeRabbit's
disabled-for-repos-under-10-stars notice and Codecov's coverage report
landed as PR comments, neither a code review). Disclosed as absent, not
run.

Inner layer (reviewing-an-artifact, low effort): the diff splits into
two parts per that skill's Step 0 mixed-target rule.

• New deterministic-gate detection logic
  (_dimension_quote_exemption_offenders/_dimension_quote_exemption_checks
  in shape_checks/citation_checks.py, plus the four new regex
  constants in shape_checks/constants.py, registered in
  check_shape()) deferred to the named specialist,
  evaluating-deterministic-gate-quality, run in an isolated dispatch
  (CLAUDE.md was auto-injected into that dispatch's own system prompt
  by the harness despite the exclusion instruction -- disclosed rather
  than silently assumed clean). Verdict: well-formed and
  well-placed. Live-verified both defects the prior /code-review
  pass had already fixed (the \s+ line-wrap fix and the
  fenced-block-stripping fix) by executing the actual functions
  against the real battle-testing-a-skill files and synthetic
  adversarial fixtures, and ran the real-repo sweep (29/29 skills
  pass) plus all 7 bundled unit tests. Zero confirmed findings. Two
  minor, non-blocking notes: (1) offender messages name the mismatch
  but not the literal corrective edit (dead-end-risk, minor); (2)
  whether the CI job is a required branch-protection status check
  could not be verified from local files (indeterminate, a
  platform-config question outside that review's read-only scope).
• The remainder of the diff (deleting
  verify_shape_check_output_diff.py and its test, the
  shape_checks/orchestrator.py docstring update, and the 7
  new/moved unit tests) classified safe under that skill's own
  Step 1 (dead-code removal, a doc update, and added tests only, no
  security-tier signal anywhere) -- Steps 2-5 skipped per that Step's
  own rule.

Zero confirmed findings from either half -> CLEAN, per
drafting-a-pr-to-merge Step 8's own outcome table.

Duplicate-PR-waiver: PR #1499 also cites #577, but only in passing, in
its own Facts bullet ("Rows 5 and 7 were found infeasible ... and
re-scoped into issue #577 (open, pending owner design decision,
untouched by this PR)"). #1499 is titled "docs(design): scope issue
#192 to items 4 and 6" and its own Related Issue line is "Refs #192" --
it implements #192's items 4 and 6 (a different pair of rows) as a
design doc only, with zero file overlap with this PR's diff. This PR
is the one that actually resolves #577 (rows 5 and 7).

Related Issue

Closes #577

[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: 7ca00a26-c1a7-4f84-a05f-1d3aa6cb2f03

---

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 (aa33e6f) to head (8af21e1).

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #1500    +/-   ##
========================================
  Coverage   99.56%   99.57%            
========================================
  Files         147      145     -2     
  Lines       24042    23858   -184     
  Branches     2863     2857     -6     
========================================
- Hits        23938    23756   -182     
+ Misses        104      102     -2     

☔ 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.