Skip to content

Commit a02f739

Browse files
authored
refactor(test-classifier): own the skill in-repo + hide parse markers on local runs (#60)
* refactor(test-classifier): own the skill in-repo; hide parse markers locally Two local-ergonomics + provenance fixes. 1. Drop the agent-skills fetch — the bundle owns its skill. The in-repo .skills/test-classifier/SKILL.md is already the canonical, and is in fact NEWER than agent-skills@v0.1.0 (the fetch was overwriting the better in-repo copy with a staler one each run). Remove scripts/fetch-skills.sh, the .skills-vendor/ indirection, the dispatcher's vendor-preference branch, the "Vendor SKILL.md" steps in both workflows + the Jenkinsfile/README, and the .gitignore entry. SKILL_PATH_CANONICAL now always points at the in-repo copy. 2. Strip machine-only markers from terminal output. On a local run the raw <!-- AI_CLASSIFIER_JSON_* --> block and the <<<AI_REVIEW_RESULT:…>>> sentinel leaked into the terminal — meaningful only to the parser / PR-comment poster, noise to a human. strip_machine_markers() removes them from what's PRINTED; parse_result and extract_classifier_json still read the untouched classifier_output, so CLASSIFIED/NO_ACTION detection and --post-comment/--json-only are unchanged. Verified: strip leaves the human report intact (markers gone, blank lines collapsed); both scripts pass bash -n; no fetch-skills/agent-skills/skills-vendor references remain anywhere in testing/ or .github/. * feat(test-classifier): actionable terminal summary after the report The markdown report is formatted for the PR comment; locally a developer had to read the whole wall to find the verdict and what to do about it. Add a compact, colored terminal summary printed after the report on a human run: one line per failing test — VERDICT, file:line, confidence — plus the one-line action derived from the verdict (→ Fix the TEST / Fix the CODE / Re-run + deflake / Fix the ENV), mirroring SKILL.md's taxonomy table. The skill is diagnostic-only by design (it names the side to fix, never writes a patch), so the summary points; it doesn't auto-fix. Rendered from the JSON the dispatcher already extracts, printed to STDERR (never touches the parsed stdout), gated to non---json-only runs, and best-effort (skips silently without python3 or a parseable block). Verified end-to-end against a real classified diff.
1 parent 3013d26 commit a02f739

10 files changed

Lines changed: 94 additions & 57 deletions

File tree

.github/workflows/test-classifier.yml

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -196,11 +196,6 @@ jobs:
196196
cp -R .classifier-bundle/testing "$GITHUB_WORKSPACE/testing"
197197
chmod +x testing/classifier/.skills/_lib/ai-classifier-dispatch.sh
198198
chmod +x testing/classifier/.skills/test-classifier/scripts/test-classifier-dispatcher.sh
199-
chmod +x testing/classifier/scripts/fetch-skills.sh
200-
201-
- name: Vendor SKILL.md from agent-skills
202-
working-directory: testing/classifier
203-
run: scripts/fetch-skills.sh
204199
205200
# ── Bedrock auth (provider=bedrock only) ────────────────────────────────
206201
# Exchange this run's GitHub OIDC token for short-lived STS credentials by

testing/classifier/.github/workflows/ai-test-classifier.yml

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -115,11 +115,6 @@ jobs:
115115
run: |
116116
chmod +x .skills/_lib/ai-classifier-dispatch.sh
117117
chmod +x .skills/test-classifier/scripts/test-classifier-dispatcher.sh
118-
chmod +x scripts/fetch-skills.sh
119-
120-
- name: Vendor canonical skill from agent-skills
121-
working-directory: testing/classifier
122-
run: scripts/fetch-skills.sh
123118
124119
# Bedrock auth (AI_PROVIDER=bedrock only): OIDC → short-lived STS creds for
125120
# the caller-owned IAM role. Claude Code picks them up via the AWS SDK

testing/classifier/.gitignore

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,6 @@
1313
.codex/skills/
1414
.github/copilot/skills/
1515

16-
# Skills vendored from agent-skills by scripts/fetch-skills.sh.
17-
.skills-vendor/
18-
1916
# ── Classifier output ──────────────────────────────────────────────────────
2017
# The workflow writes the per-run classification (the CI artifact / audit trail)
2118
# here before uploading it as a CI artifact. Not source-of-truth — don't commit.

testing/classifier/.skills/test-classifier/scripts/test-classifier-dispatcher.sh

Lines changed: 84 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,10 @@ fi
6161
# ── Skill identity ──────────────────────────────────────────────────────────
6262
SKILL_NAME="test-classifier"
6363
SKILL_HUMAN_NAME="AI Test Classifier (application-bug / test-bug / flaky / environment)"
64-
# Skill text: prefer the copy vendored from agent-skills by fetch-skills.sh;
65-
# fall back to the in-repo .skills/ copy when the vendor dir is absent.
64+
# Skill text lives in-repo and is the canonical source of truth (this bundle owns
65+
# its skill — no external fetch).
6666
BUNDLE_ROOT="$(cd "${SKILLS_ROOT}/.." && pwd)" # .skills → classifier (bundle root)
67-
if [[ -f "${BUNDLE_ROOT}/.skills-vendor/test-classifier/SKILL.md" ]]; then
68-
SKILL_PATH_CANONICAL=".skills-vendor/test-classifier/SKILL.md"
69-
else
70-
SKILL_PATH_CANONICAL=".skills/test-classifier/SKILL.md"
71-
fi
67+
SKILL_PATH_CANONICAL=".skills/test-classifier/SKILL.md"
7268

7369
# ── Classifier-specific arg parsing ────────────────────────────────────────
7470
# We intercept our own flags first, then pass the remainder to the shared
@@ -254,9 +250,7 @@ in this repository at:
254250
255251
${SKILL_PATH_CANONICAL}
256252
257-
(The canonical skill text is published in navapbc/agent-skills and vendored here
258-
by scripts/fetch-skills.sh; when the vendored copy is absent the in-repo
259-
.skills/ copy is used. Either way, follow the file at the path above.)
253+
Follow the file at the path above.
260254
261255
Classify the failing tests for the change under test. The exact git range is in
262256
the AI_REVIEW_DIFF_RANGE env var (base→HEAD normally; base→index, i.e. committed
@@ -333,6 +327,78 @@ extract_classifier_json() {
333327
'
334328
}
335329

330+
# Strip the machine-only markers from the AI output for HUMAN display: the
331+
# JSON block (BEGIN→END inclusive) and the <<<AI_REVIEW_RESULT:…>>> line. These
332+
# exist for parsing / PR-comment posting and are noise in a local terminal run.
333+
# Parsing always uses the untouched ${classifier_output}; this only affects what
334+
# is printed. Trailing blank lines left by the removal are collapsed.
335+
strip_machine_markers() {
336+
local input="$1"
337+
printf '%s\n' "${input}" | awk '
338+
/<!-- AI_CLASSIFIER_JSON_BEGIN -->/ { skip=1; next }
339+
/<!-- AI_CLASSIFIER_JSON_END -->/ { skip=0; next }
340+
skip { next }
341+
/^<<<AI_REVIEW_RESULT:[A-Z_]+>>>[[:space:]]*$/ { next }
342+
{ print }
343+
' | awk 'NF { blanks=0; print; next } { blanks++; if (blanks<=1) print }'
344+
}
345+
346+
# Compact, terminal-native summary of the verdicts — the actionable punchline a
347+
# local developer needs, lifted out of the long markdown report. One line per
348+
# failing test: VERDICT, file:line, confidence, and the one-line "what to do"
349+
# action derived from the verdict (mirrors SKILL.md's taxonomy table). The skill
350+
# is diagnostic-only, so this points at the side to fix; it never proposes a
351+
# patch. Printed to STDERR so it never touches the parsed stdout. Best-effort:
352+
# if python3 or the JSON is unavailable, we silently skip it (the full report
353+
# above still stands).
354+
render_terminal_summary() {
355+
local classifier_json="$1"
356+
command -v python3 &>/dev/null || return 0
357+
[[ -n "${classifier_json}" ]] || return 0
358+
359+
AI_C_RED="${AI_C_RED}" AI_C_YELLOW="${AI_C_YELLOW}" AI_C_GREEN="${AI_C_GREEN}" \
360+
AI_C_BLUE="${AI_C_BLUE}" AI_C_BOLD="${AI_C_BOLD}" AI_C_RESET="${AI_C_RESET}" \
361+
python3 - "${classifier_json}" >&2 <<'PY'
362+
import json, os, sys
363+
364+
RED=os.environ.get("AI_C_RED",""); YEL=os.environ.get("AI_C_YELLOW","")
365+
GRN=os.environ.get("AI_C_GREEN",""); BLU=os.environ.get("AI_C_BLUE","")
366+
BOLD=os.environ.get("AI_C_BOLD",""); RST=os.environ.get("AI_C_RESET","")
367+
368+
try:
369+
data = json.loads(sys.argv[1])
370+
except Exception:
371+
sys.exit(0)
372+
373+
cls = data.get("classifications") or []
374+
if not cls:
375+
sys.exit(0)
376+
377+
# verdict → (color, one-line action). Mirrors SKILL.md's "What it means / action".
378+
ACTION = {
379+
"APPLICATION_BUG": (RED, "Fix the CODE — the app regressed; the test caught a real defect."),
380+
"TEST_BUG": (YEL, "Fix the TEST — the app is correct; the assertion is stale."),
381+
"FLAKY_FAILURE": (BLU, "Re-run to confirm, then deflake — not a code/test-logic patch."),
382+
"ENVIRONMENT_ISSUE":(BLU, "Fix the ENV / re-run — neither the app nor the test is at fault."),
383+
}
384+
385+
mode = data.get("mode","")
386+
print(f"\n{BOLD}─── Test Classifier — {len(cls)} classified" + (f" ({mode})" if mode else "") + f" ───{RST}")
387+
for c in cls:
388+
verdict = c.get("verdict","") or "UNKNOWN"
389+
color, action = ACTION.get(verdict, (RST, ""))
390+
loc = c.get("path","")
391+
line = c.get("line")
392+
if loc and line not in (None, ""):
393+
loc = f"{loc}:{line}"
394+
conf = c.get("confidence","")
395+
conf_s = f" ({conf})" if conf else ""
396+
print(f" {color}{verdict:<17}{RST} {loc}{conf_s}")
397+
if action:
398+
print(f" {color}→{RST} {action}")
399+
PY
400+
}
401+
336402
# Render the ONE PR comment body from the classifier JSON, including the
337403
# mandatory 👍/👎 ask. We use python3 because pure-bash JSON handling is brittle.
338404
render_pr_comment_body() {
@@ -554,7 +620,9 @@ test_classifier::run() {
554620
if (( JSON_ONLY == 1 )); then
555621
extract_classifier_json "${classifier_output}"
556622
else
557-
printf '%s\n' "${classifier_output}"
623+
# Human-facing terminal output: drop the machine-only markers (JSON block +
624+
# result sentinel). Parsing below still uses the untouched classifier_output.
625+
strip_machine_markers "${classifier_output}"
558626
ai_review::log "────────────────────────────────────────────────────────────"
559627
fi
560628

@@ -569,6 +637,11 @@ test_classifier::run() {
569637
case "${result}" in
570638
CLASSIFIED)
571639
ai_review::info "Classifier result: CLASSIFIED"
640+
# Terminal-native actionable summary (verdict + file:line + what-to-do),
641+
# for a human run only. Skipped under --json-only (machine consumption).
642+
if (( JSON_ONLY != 1 )); then
643+
render_terminal_summary "$(extract_classifier_json "${classifier_output}")"
644+
fi
572645
;;
573646
NO_ACTION)
574647
ai_review::ok "Classifier result: NO_ACTION (nothing failed)."

testing/classifier/README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ reuses its conventions exactly:
3333
`copilot`. See [`docs/BEDROCK.md`](./docs/BEDROCK.md).
3434
- The result-marker contract: the AI ends with one `<<<AI_REVIEW_RESULT:...>>>`
3535
marker the dispatcher parses.
36-
- The skill text vendored from `navapbc/agent-skills` by
37-
`scripts/fetch-skills.sh`, with the in-repo `.skills/` copy as a fallback.
36+
- The skill text at `.skills/test-classifier/SKILL.md` — canonical and in-repo
37+
(this bundle owns its skill; no external fetch).
3838
- A GitHub Actions workflow that ships **disabled**, **advisory/non-blocking**
3939
by default, with a `--gate` flag to opt into build-failing later.
4040
- Same bash style: `set -euo pipefail`, `ai_review::`-namespaced helpers,

testing/classifier/docs/LOCAL_TEST_CLASSIFIER.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,12 @@ AI_RUN_SUITE=1 test-classifier # run the suite locally and triage REAL fa
118118
and never posts a PR comment. Posting (with the mandatory 👍/👎 ask that feeds
119119
the metrics loop) happens on the PR run — Path B, or `--post-comment` against a
120120
real PR. See [`SETUP.md`](./SETUP.md).
121+
- **The actionable summary.** After the full report, the dispatcher prints a
122+
compact, colored summary — one line per failing test with its verdict,
123+
`file:line`, confidence, and the one-line *what to do* (`→ Fix the TEST`,
124+
`→ Fix the CODE`, `→ Re-run / deflake`, `→ Fix the ENV`). That's the punchline:
125+
the classifier is **diagnostic** — it tells you which side to fix, never writes
126+
the patch. Read the summary, then make the change yourself.
121127
- **INFERRED vs OBSERVED.** Without `AI_RUN_SUITE=1` the agent predicts failures
122128
from the diff (INFERRED) and never touches your toolchain. With it, the agent
123129
runs your suite (OBSERVED) — the only mode in which `FLAKY_FAILURE` /

testing/classifier/docs/PLAYBOOK.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ It mirrors the security workstream exactly in its plumbing:
5454
`set -euo pipefail`, and color helpers suppressed in CI.
5555
- The **result-marker contract**: the AI ends its output with exactly one
5656
`<<<AI_REVIEW_RESULT:...>>>` marker that the dispatcher parses.
57-
- The **skill text** vendored from `navapbc/agent-skills` by
58-
`scripts/fetch-skills.sh`, with the in-repo `.skills/` copy as a fallback.
57+
- The **skill text** at `.skills/test-classifier/SKILL.md` — canonical and
58+
in-repo (this bundle owns its skill; no external fetch).
5959
- A GitHub Actions workflow that ships **disabled** (`on: workflow_dispatch`
6060
only), is **non-blocking/advisory by default**, and flips to build-failing
6161
only behind an explicit `--gate` flag.

testing/classifier/jenkins/Jenkinsfile

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,6 @@ pipeline {
9292
esac
9393
'''
9494

95-
// Vendor the skill from agent-skills before invoking the dispatcher.
96-
dir('testing/classifier') {
97-
sh 'chmod +x scripts/fetch-skills.sh && scripts/fetch-skills.sh'
98-
}
99-
10095
// Ensure the scripts are executable (a fresh checkout may drop the bit),
10196
// then hand off to the CI adapter — it normalizes CHANGE_ID/CHANGE_TARGET
10297
// and execs the dispatcher with --pr / --against / --post-comment.

testing/classifier/jenkins/README.md

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,6 @@ stage('AI test classifier') {
103103
sh '''
104104
set -euo pipefail
105105
npm install -g @anthropic-ai/claude-code # or @openai/codex
106-
# Run fetch-skills from the classifier dir in a subshell so the cd
107-
# doesn't leak — don't rely on $WORKSPACE being stable across steps.
108-
( cd testing/classifier && chmod +x scripts/fetch-skills.sh && scripts/fetch-skills.sh )
109106
chmod +x testing/classifier/.skills/_lib/ai-classifier-dispatch.sh \\
110107
testing/classifier/.skills/test-classifier/scripts/test-classifier-dispatcher.sh \\
111108
testing/classifier/jenkins/ci-adapter.sh

testing/classifier/scripts/fetch-skills.sh

Lines changed: 0 additions & 21 deletions
This file was deleted.

0 commit comments

Comments
 (0)