Skip to content

Commit e007729

Browse files
authored
feat(test-classifier): local --unpushed run + align path lettering with security (#52)
Add a true no-PR local path so a developer can classify failing tests on their own machine against whatever they haven't pushed yet (committed + staged), the testing counterpart to the security runner's --unpushed: - ai-classifier-dispatch.sh: port resolve_unpushed_base(), add --unpushed (sets base + INCLUDE_STAGED), and teach the diff helpers base->index (committed + staged) vs base->HEAD. - test-classifier-dispatcher.sh: skip PR discovery on --unpushed, export AI_REVIEW_DIFF_RANGE so the AI diffs exactly what the dispatcher accounts for (incl. staged), keep the run report-only (no PR to post to), and fix --help erroring on PR lookup before reaching the help text. - docs/LOCAL_TEST_CLASSIFIER.md: new guide + one-word `test-classifier` zsh function, mirroring LOCAL_SECURITY_REVIEW.md. Renumber the paths to match the security bundle (A = local developer run, B = reusable CI workflow), so SETUP.md's "mirrors PR_REVIEW_SETUP step for step" is actually true. Reconciled every Path A/B reference across SETUP.md, README, BEDROCK.md, and LOCAL_TEST_CLASSIFIER.md; INSTALL.txt was already on this convention.
1 parent 4afea24 commit e007729

6 files changed

Lines changed: 402 additions & 136 deletions

File tree

testing/classifier/.skills/_lib/ai-classifier-dispatch.sh

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,42 @@ ai_review::err() { printf '%s[%s] ERROR: %s%s\n' "${AI_C_RED}" "${SKILL_NAME}"
9696
# AI_REVIEW_DRY_RUN ("1" or "0") — print what would happen, do not call AI
9797
# AI_REVIEW_NO_BLOCK ("1" or "0") — run classification but never exit non-zero
9898
# AI_REVIEW_AGAINST (string) — git ref to diff against (the change under test)
99+
# AI_REVIEW_INCLUDE_STAGED ("1"/"0") — when set, diff base→index (committed +
100+
# staged) instead of base→HEAD; set by --unpushed
99101
# AI_REVIEW_REMAINING (array) — any unparsed args
102+
103+
# ── Base resolution for --unpushed ──────────────────────────────────────────
104+
# Resolves the "last pushed" point so --unpushed can classify everything not yet
105+
# pushed (committed + staged). Order: the branch's upstream; else the merge-base
106+
# with the remote default branch (origin/HEAD, then origin/main, origin/master).
107+
# Prints the base ref on success; returns non-zero if none can be determined (the
108+
# caller then errors and asks for an explicit --against rather than silently
109+
# narrowing scope). Mirrors the security dispatcher's resolver.
110+
ai_review::resolve_unpushed_base() {
111+
local base def
112+
base="$(git rev-parse --abbrev-ref --symbolic-full-name '@{upstream}' 2>/dev/null || true)"
113+
if [[ -z "${base}" ]]; then
114+
# `git rev-parse --abbrev-ref origin/HEAD` echoes the literal "origin/HEAD"
115+
# when the symref is unset, which would poison the fallback — use
116+
# symbolic-ref, which fails cleanly with empty output.
117+
def="$(git symbolic-ref --short -q refs/remotes/origin/HEAD 2>/dev/null || true)"
118+
if [[ -z "${def}" ]] && git rev-parse --verify --quiet origin/main >/dev/null 2>&1; then
119+
def="origin/main"
120+
fi
121+
if [[ -z "${def}" ]] && git rev-parse --verify --quiet origin/master >/dev/null 2>&1; then
122+
def="origin/master"
123+
fi
124+
[[ -n "${def}" ]] && base="$(git merge-base HEAD "${def}" 2>/dev/null || true)"
125+
fi
126+
[[ -n "${base}" ]] || return 1
127+
printf '%s' "${base}"
128+
}
129+
100130
ai_review::parse_args() {
101131
AI_REVIEW_DRY_RUN=0
102132
AI_REVIEW_NO_BLOCK=0
103133
AI_REVIEW_AGAINST=""
134+
AI_REVIEW_INCLUDE_STAGED=0
104135
AI_REVIEW_REMAINING=()
105136

106137
while [[ $# -gt 0 ]]; do
@@ -125,6 +156,17 @@ ai_review::parse_args() {
125156
AI_REVIEW_AGAINST="${1#*=}"
126157
shift
127158
;;
159+
--unpushed)
160+
# Classify everything not yet pushed: committed + staged, i.e. base→index.
161+
# No PR required — this is the local backstop (report-only; nothing posts).
162+
if ! AI_REVIEW_AGAINST="$(ai_review::resolve_unpushed_base)"; then
163+
ai_review::err "--unpushed: couldn't determine what's been pushed (no upstream and no remote default branch)."
164+
ai_review::log " Re-run with an explicit base, e.g. --against main"
165+
exit 2
166+
fi
167+
AI_REVIEW_INCLUDE_STAGED=1
168+
shift
169+
;;
128170
-h|--help)
129171
ai_review::print_help
130172
exit 0
@@ -158,6 +200,11 @@ Options:
158200
--against <ref> Classify failures relative to the change between <ref> and
159201
HEAD (the "change under test"), e.g. --against origin/main
160202
or --against HEAD~1.
203+
--unpushed Classify everything not yet pushed (committed + staged),
204+
resolving the base from the branch's upstream or the
205+
merge-base with the remote default branch. No PR needed —
206+
the run is report-only (nothing is posted). The local
207+
backstop, mirroring the security runner's --unpushed.
161208
-h, --help Show this help and exit.
162209
163210
Environment variables:
@@ -227,30 +274,43 @@ ai_review::require_cli() {
227274
# ── Diff collection ─────────────────────────────────────────────────────────
228275
# Determines whether there is a "change under test" to reason about.
229276
# When AI_REVIEW_AGAINST is empty, falls back to the staged diff (local use).
230-
# When AI_REVIEW_AGAINST is set, uses the diff between that ref and HEAD.
277+
# When AI_REVIEW_AGAINST is set: base→HEAD (committed range), or base→index
278+
# (committed + staged) under --unpushed (AI_REVIEW_INCLUDE_STAGED=1).
231279
ai_review::has_changes() {
232280
if [[ -n "${AI_REVIEW_AGAINST}" ]]; then
233281
if ! git rev-parse --verify --quiet "${AI_REVIEW_AGAINST}^{commit}" >/dev/null; then
234282
ai_review::err "Git ref not found: ${AI_REVIEW_AGAINST}"
235283
exit 1
236284
fi
237-
! git diff --quiet "${AI_REVIEW_AGAINST}" HEAD --
285+
if [[ "${AI_REVIEW_INCLUDE_STAGED:-0}" == "1" ]]; then
286+
! git diff --cached --quiet "${AI_REVIEW_AGAINST}" --
287+
else
288+
! git diff --quiet "${AI_REVIEW_AGAINST}" HEAD --
289+
fi
238290
else
239291
! git diff --cached --quiet
240292
fi
241293
}
242294

243295
ai_review::changed_files() {
244296
if [[ -n "${AI_REVIEW_AGAINST}" ]]; then
245-
git diff --name-only "${AI_REVIEW_AGAINST}" HEAD --
297+
if [[ "${AI_REVIEW_INCLUDE_STAGED:-0}" == "1" ]]; then
298+
git diff --cached --name-only "${AI_REVIEW_AGAINST}" --
299+
else
300+
git diff --name-only "${AI_REVIEW_AGAINST}" HEAD --
301+
fi
246302
else
247303
git diff --cached --name-only
248304
fi
249305
}
250306

251307
ai_review::diff_command_description() {
252308
if [[ -n "${AI_REVIEW_AGAINST}" ]]; then
253-
echo "git diff ${AI_REVIEW_AGAINST} HEAD"
309+
if [[ "${AI_REVIEW_INCLUDE_STAGED:-0}" == "1" ]]; then
310+
echo "git diff --cached ${AI_REVIEW_AGAINST} (committed + staged, unpushed)"
311+
else
312+
echo "git diff ${AI_REVIEW_AGAINST} HEAD"
313+
fi
254314
else
255315
echo "git diff --cached"
256316
fi

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

Lines changed: 43 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
# test-classifier-dispatcher.sh # auto-discover PR; print only
3131
# test-classifier-dispatcher.sh --pr 1234 --post-comment # post the PR comment
3232
# test-classifier-dispatcher.sh --against origin/main # explicit base ref
33+
# test-classifier-dispatcher.sh --unpushed # local: committed+staged, NO PR (report-only)
3334
# test-classifier-dispatcher.sh --json-only # emit only the JSON block
3435
# test-classifier-dispatcher.sh --gate # exit 1 on CLASSIFIED
3536
# test-classifier-dispatcher.sh --dry-run # show plan, no AI call
@@ -78,12 +79,15 @@ fi
7879
# --gate Exit 1 if the result is CLASSIFIED (CI-blocking mode)
7980
# --json-only Print only the JSON block (machine consumption)
8081
#
81-
# All other flags (--dry-run, --no-block, --against) fall through to the lib.
82+
# All other flags (--dry-run, --no-block, --against, --unpushed) fall through to
83+
# the lib. --unpushed classifies the local committed+staged diff with NO PR —
84+
# PR discovery is skipped and the run is report-only (nothing posts).
8285

8386
PR_NUMBER=""
8487
POST_COMMENT=0
8588
GATE_MODE=0
8689
JSON_ONLY=0
90+
WANT_HELP=0
8791
REMAINING_FOR_LIB=()
8892

8993
while [[ $# -gt 0 ]]; do
@@ -116,6 +120,14 @@ while [[ $# -gt 0 ]]; do
116120
JSON_ONLY=1
117121
shift
118122
;;
123+
-h|--help)
124+
# Defer: PR discovery runs before the lib's parser, so a bare --help left
125+
# for the lib would error on PR lookup before reaching the help text. We
126+
# can't print here either — the lib (which defines print_help) is sourced
127+
# below. Flag it and emit right after the source.
128+
WANT_HELP=1
129+
shift
130+
;;
119131
*)
120132
REMAINING_FOR_LIB+=("$1")
121133
shift
@@ -148,8 +160,14 @@ ai_review::discover_pr_context() {
148160
# both the PR number and the base ref (e.g. the Jenkins adapter passing
149161
# CHANGE_ID + CHANGE_TARGET) supplies both flags and we skip `gh pr view`
150162
# entirely — gh is then needed only to post the comment.
163+
# --against (explicit base) and --unpushed (local committed+staged, no PR)
164+
# both fix the diff range without needing `gh pr view`. In either case skip
165+
# PR discovery: the library's parser resolves the range. With --unpushed there
166+
# is no PR, so comment posting stays disabled (the --post-comment guard below
167+
# errors on the empty PR number) — a local run is report-only by design.
151168
for arg in "${REMAINING_FOR_LIB[@]+"${REMAINING_FOR_LIB[@]}"}"; do
152-
if [[ "${arg}" == "--against" ]] || [[ "${arg}" == --against=* ]]; then
169+
if [[ "${arg}" == "--against" ]] || [[ "${arg}" == --against=* ]] \
170+
|| [[ "${arg}" == "--unpushed" ]]; then
153171
if [[ -n "${PR_NUMBER}" ]]; then
154172
AI_REVIEW_PR_NUMBER="${PR_NUMBER}"
155173
fi
@@ -240,11 +258,12 @@ in this repository at:
240258
by scripts/fetch-skills.sh; when the vendored copy is absent the in-repo
241259
.skills/ copy is used. Either way, follow the file at the path above.)
242260
243-
Classify the failing tests for the change under test — the diff between
244-
AI_REVIEW_AGAINST and HEAD:
261+
Classify the failing tests for the change under test. The exact git range is in
262+
the AI_REVIEW_DIFF_RANGE env var (base→HEAD normally; base→index, i.e. committed
263+
+ staged, for a local --unpushed run). Use it verbatim:
245264
246-
git diff "\$AI_REVIEW_AGAINST" HEAD --unified=5
247-
git diff "\$AI_REVIEW_AGAINST" HEAD --name-only
265+
git diff \$AI_REVIEW_DIFF_RANGE --unified=5
266+
git diff \$AI_REVIEW_DIFF_RANGE --name-only
248267
249268
Follow the skill instructions in test-classifier/SKILL.md exactly:
250269
@@ -295,6 +314,13 @@ PROMPT
295314
# shellcheck source=../../_lib/ai-classifier-dispatch.sh
296315
source "${LIB_PATH}"
297316

317+
# --help was requested during arg parsing above; the lib (which defines the help
318+
# text) is now loaded, so emit it and exit before any PR discovery / AI call.
319+
if (( WANT_HELP == 1 )); then
320+
ai_review::print_help
321+
exit 0
322+
fi
323+
298324
# ── Helpers: JSON extraction and PR-comment posting ────────────────────────
299325

300326
# Pull the JSON block between AI_CLASSIFIER_JSON_BEGIN/END markers out of stdin.
@@ -511,6 +537,16 @@ test_classifier::run() {
511537

512538
export AI_REVIEW_AGAINST
513539

540+
# The git range the AI should diff, matching the dispatcher's own accounting:
541+
# --unpushed (INCLUDE_STAGED) → `--cached <base>` (committed + staged)
542+
# otherwise → `<base> HEAD` (committed range)
543+
if [[ "${AI_REVIEW_INCLUDE_STAGED:-0}" == "1" ]]; then
544+
AI_REVIEW_DIFF_RANGE="--cached ${AI_REVIEW_AGAINST}"
545+
else
546+
AI_REVIEW_DIFF_RANGE="${AI_REVIEW_AGAINST} HEAD"
547+
fi
548+
export AI_REVIEW_DIFF_RANGE
549+
514550
local classifier_output
515551
local invoke_rc=0
516552
classifier_output="$(ai_review::invoke_ai)" || invoke_rc=$?
@@ -555,7 +591,7 @@ test_classifier::run() {
555591
ai_review::info "Result is NO_ACTION — nothing to triage, so no PR comment is posted."
556592
else
557593
if [[ -z "${AI_REVIEW_PR_NUMBER:-}" ]]; then
558-
ai_review::err "--post-comment requires a discoverable PR. Use --pr <number> or ensure 'gh pr view' resolves."
594+
ai_review::err "--post-comment requires a discoverable PR. Use --pr <number> or ensure 'gh pr view' resolves. (A --unpushed local run has no PR, so it is report-only — drop --post-comment.)"
559595
exit 1
560596
fi
561597
local json_block

testing/classifier/README.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@ testing/classifier/
5959
├── INSTALL.txt ← quick install steps
6060
├── docs/
6161
│ ├── PLAYBOOK.md ← the prescriptive pilot playbook
62-
│ └── SETUP.md ← reusable workflow (recommended) + local + vendored setup
62+
│ ├── SETUP.md ← reusable workflow (recommended) + local + vendored setup
63+
│ └── LOCAL_TEST_CLASSIFIER.md ← one-word local run (zshrc function, --unpushed)
6364
├── .github/
6465
│ ├── copilot-instructions.md ← Copilot test-classification instructions
6566
│ └── workflows/
@@ -116,8 +117,9 @@ dispatcher contract.
116117

117118
1. Read **`docs/PLAYBOOK.md`** — the four-verdict taxonomy, the metrics loop,
118119
and the embedded security-considerations section.
119-
2. Follow **`docs/SETUP.md`** — Path A (reusable workflow, recommended), Path B
120-
(run locally), Path C (vendored workflow fallback). On **Jenkins**, follow
120+
2. Follow **`docs/SETUP.md`** — Path A (run locally), Path B (reusable workflow,
121+
the recommended CI backstop), Path C (vendored workflow fallback). On
122+
**Jenkins**, follow
121123
**`jenkins/README.md`** instead (the non-Actions CI path).
122124
3. Use **`INSTALL.txt`** for the fast-path file-copy steps.
123125

testing/classifier/docs/BEDROCK.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ mandatory, not optional.
233233
Two ways, matching the two consumption paths in
234234
[`SETUP.md`](./SETUP.md).
235235

236-
### Path A — Reusable workflow (recommended)
236+
### Path B — Reusable workflow (recommended)
237237

238238
In your caller workflow (`.github/workflows/ai-test-classifier.yml`), set the
239239
provider inputs. The example below uses **`aws-auth: static`** — one Bedrock API
@@ -353,7 +353,7 @@ region = "<your region>"
353353
- **`Could not load credentials from any providers` / role not assumed** — the
354354
OIDC step failed. Check the role ARN, that the OIDC provider exists in the
355355
account, and that the trust policy's `:sub` matches `repo:<OWNER>/<REPO>:...`.
356-
Confirm `id-token: write` is present **in the caller** (Path A) — the reusable
356+
Confirm `id-token: write` is present **in the caller** (Path B) — the reusable
357357
workflow can't add it.
358358
- **`on-demand throughput isn't supported`** — you passed a bare model ID. Use a
359359
cross-region **inference profile** ID (the `us.` prefix), which is the default.

0 commit comments

Comments
 (0)