-
Notifications
You must be signed in to change notification settings - Fork 1
review: fix the out/ artifact upload (allowed-paths must match staging-relative paths) #243
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 49 commits
Commits
Show all changes
50 commits
Select commit
Hold shift + click to select a range
2d746ac
[jwbron/live-eval-corpus] review: live-enabled corpus format and ten …
jwbron 6727eb6
[jwbron/live-eval-producer-staging] review: live-producer prompt extr…
jwbron 5c00cf3
[jwbron/live-eval-producer-staging] review: the live producer and SDK…
jwbron 52073ef
[jwbron/live-eval-ab-runner] review: the live A/B runner (phase 3)
jwbron 93770e6
[jwbron/live-eval-ab-ci] review: per-PR live A/B workflow (phase 4)
jwbron 30e7527
[jwbron/live-eval-corpus] review: exclude eval-corpus trees from lint…
jwbron 19f06cb
[jwbron/live-eval-producer-staging] Merge branch 'jwbron/live-eval-co…
jwbron d1b5288
[jwbron/live-eval-ab-runner] Merge branch 'jwbron/live-eval-producer-…
jwbron 254dfc4
[jwbron/live-eval-ab-ci] Merge branch 'jwbron/live-eval-ab-runner' in…
jwbron 917cc11
[jwbron/live-eval-producer-staging] review: namespace live finding id…
jwbron 0ee295a
[jwbron/live-eval-ab-runner] Merge branch 'jwbron/live-eval-producer-…
jwbron 58cd4ed
[jwbron/live-eval-ab-runner] review: judge failures degrade the A/B r…
jwbron b19b08d
[jwbron/live-eval-ab-ci] Merge branch 'jwbron/live-eval-ab-runner' in…
jwbron bced291
[jwbron/review-trial-skill] review: add the review-trial skill (live …
jwbron da1115c
[tmp-refresh] Merge remote-tracking branch 'origin/main' into tmp-ref…
jwbron a6883be
[tmp-refresh] Merge remote-tracking branch 'origin/jwbron/live-eval-c…
jwbron 0d02672
[tmp-refresh] Merge remote-tracking branch 'origin/jwbron/live-eval-p…
jwbron 93d8dec
[tmp-refresh] Merge remote-tracking branch 'origin/jwbron/live-eval-a…
jwbron e3b34eb
[tmp-refresh] Merge remote-tracking branch 'origin/jwbron/live-eval-a…
jwbron 5dd182b
[jwbron/review-out-artifact-upload] review: fix the out/ artifact upl…
jwbron 2812679
[jwbron/live-eval-corpus] review: route the specialist lens on each l…
jwbron 2ce35a0
[jwbron/live-eval-producer-staging] Merge branch 'jwbron/live-eval-co…
jwbron c0fece2
[jwbron/live-eval-ab-runner] Merge branch 'jwbron/live-eval-producer-…
jwbron e02ac40
[jwbron/live-eval-ab-runner] review: carry agent-failure reasons into…
jwbron d225bd4
[jwbron/live-eval-ab-ci] Merge branch 'jwbron/live-eval-ab-runner' in…
jwbron cbc838d
[jwbron/live-eval-ab-runner] review: close every A/B report with a pe…
jwbron 63097f1
[jwbron/live-eval-corpus] Merge remote-tracking branch 'origin/jwbron…
jwbron 9012508
[jwbron/live-eval-producer-staging] Merge remote-tracking branch 'ori…
jwbron 25133b4
[jwbron/live-eval-producer-staging] Merge branch 'jwbron/live-eval-co…
jwbron 0a3d212
[jwbron/live-eval-ab-runner] Merge remote-tracking branch 'origin/jwb…
jwbron a547972
[jwbron/live-eval-ab-runner] Merge branch 'jwbron/live-eval-producer-…
jwbron 391151b
[jwbron/live-eval-ab-ci] Merge remote-tracking branch 'origin/jwbron/…
jwbron d2c4c70
[jwbron/live-eval-ab-ci] Merge branch 'jwbron/live-eval-ab-runner' in…
jwbron 996766f
[jwbron/live-eval-ab-runner] review: identity short-circuit, gate-fli…
jwbron fb81be8
[jwbron/live-eval-ab-runner] review: judge economics (Haiku pin, retr…
jwbron a659be8
[jwbron/live-eval-ab-ci] review: document why the baseline is the bas…
jwbron b7c3786
[jwbron/live-eval-ab-ci] Merge branch 'jwbron/live-eval-ab-runner' in…
jwbron 541e413
[jwbron/review-trial-skill] Merge branch 'jwbron/live-eval-ab-ci' int…
jwbron fd42efd
[jwbron/review-out-artifact-upload] Merge branch 'jwbron/review-trial…
jwbron 193ae69
[jwbron/live-eval-ab-runner] review: type the judge response via the …
jwbron 7a2065c
[jwbron/live-eval-ab-ci] Merge branch 'jwbron/live-eval-ab-runner' in…
jwbron 00ce9d4
[jwbron/review-trial-skill] Merge branch 'jwbron/live-eval-ab-ci' int…
jwbron 4bd445c
[jwbron/review-out-artifact-upload] Merge branch 'jwbron/review-trial…
jwbron 80da666
Merge branch 'main' into jwbron/live-eval-corpus
jwbron 82bc5ed
Merge branch 'jwbron/live-eval-corpus' into jwbron/live-eval-producer…
jwbron 7f2fc3a
Merge branch 'jwbron/live-eval-producer-staging' into jwbron/live-eva…
jwbron 9cc1a89
Merge branch 'jwbron/live-eval-ab-runner' into jwbron/live-eval-ab-ci
jwbron da93344
Merge branch 'jwbron/live-eval-ab-ci' into jwbron/review-trial-skill
jwbron aa5a246
Merge branch 'jwbron/review-trial-skill' into jwbron/review-out-artif…
jwbron 47fb67e
[landtmp] Merge remote-tracking branch 'origin/main' into landtmp
jwbron File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "review": patch | ||
| --- | ||
|
|
||
| CI wiring for the live A/B (live A/B plan, phase 4): the `Review Eval A/B` workflow runs on every non-draft PR touching `workflows/review/**` (plus `workflow_dispatch`), executing the arm-to-arm live eval against the PR's merge-base and posting the delta report as a sticky PR comment, a job summary, and an artifact. Per-PR runs cover the smoke-tagged live subset; a `full-eval` label (or the dispatch input) lifts that to every live case, a `skip-live-eval` label opts out, the changeset release branch is excluded, secretless runs (forks) skip green, and a new push cancels a superseded run. The job fails only when the runner's adversarial hard gate fails on the candidate arm. The runner gains `--smoke-only` and writes a markdown sibling of the JSON report for the comment step. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "review": minor | ||
| --- | ||
|
|
||
| The live A/B runner (live A/B plan, phase 3). `eval/live-match.ts` maps a run's posted candidates onto a live case's labeled defect specs (deterministic path/window/mechanism rule first, then an injected hard-capped fallback arbiter whose matches are recorded for audit) and aggregates live metrics: must-catch recall, verdict agreement, clean false-flag, and noise. `eval/live-ab.ts` runs the model sub-agents from two review.md versions (the merge-base baseline via `git show`, the working-tree candidate) over the same live corpus, each arm under half a hard USD budget with sticky stop-and-report-skips semantics, replays each live result through the deterministic pipeline (`runner.ts` gains a `validation` override on `RunOptions` so a live validator's output replaces the recorded block), diffs spec-level regressions over the cases both arms scored, judges both arms' comments with the pinned judge (quality aggregates only; extracted to `eval/judge-live-model.ts`, now shared with `live-judge.ts`), and emits a JSON report plus a markdown table (also appended to GITHUB_STEP_SUMMARY). Report-only except the playbook's standing rule: the candidate arm failing any adversarial-injection case exits non-zero. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "review": minor | ||
| --- | ||
|
|
||
| Live-enabled corpus cases (live A/B eval plan, phase 1). The eval corpus gains an opt-in `live` block carrying what a REAL model run needs and the deterministic replay does not: the PR context (`prContext`, with the description treated as untrusted author text), a post-change file tree on disk next to the case (`<id>/case.json` + `<id>/tree/`, coexisting with the flat `<id>.json` layout), and labeled defect specs (`mustCatchSpecs` / `mustNotFlagSpecs`: path, line window, mechanism keyword alternates) so a live run's model-chosen finding ids can be matched to ground truth. A live case must carry the `live` tag, a cleanly-parseable diff, and every non-removed changed file in its tree; the loader enforces all of it and `loadLiveCorpus()` returns the subset. Ten cases are converted with hand-authored real diffs and trees (five smoke incidents, both clean cases, one adversarial injection whose payload lives in the diff, one golden holdout, one synthetic mutation); their recorded line anchors now point at the authored defect lines, and the deterministic suite runs them unchanged, provenance gate included. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "review": minor | ||
| --- | ||
|
|
||
| The live producer (live A/B plan, phase 2): everything needed to run the REAL model sub-agents over a live-enabled corpus case. `eval/agent-extract.ts` parses `review.md`'s `## agent:` sections into name/description/model/prompt as pure data (string in, no fs), so an A/B can extract the baseline arm from a `git show` of the merge-base; parsing is strict and lists every malformed section at once, and an integration test runs it over the real `review.md`. `eval/live-stage.ts` materializes the production `/tmp/gh-aw/review/` staging layout for a case (pr-context.json, full/pr/full-stripped diffs, files.json with hasPatch, review-files.json, provenance.json, routing.json, out/, and the post-change checkout) and rewrites extracted prompts to point at it. `eval/live-producer.ts` dispatches the default finders plus the router's lenses through an injected runner seam, maps all three output contracts (label-shape reviewers, structured-schema lenses, the claim-validator's three-state verdicts) into the exact `RecordedFinding`/`CaseVerification` shapes the deterministic runner consumes, stages `claims.json`, resolves `{{#runtime-import}}` directives from the case tree, retries once on malformed output, keeps partial results on agent failure, and accounts per-agent cost. `eval/live-runner.ts` is the one SDK-backed runner (Claude Agent SDK; Read/Grep/Glob only, cwd pinned to the staged checkout, hard turn and wall-clock caps) plus a CLI smoke entry (`pnpm dlx tsx workflows/review/eval/live-runner.ts --case <id>`, requires ANTHROPIC_API_KEY). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "review": patch | ||
| --- | ||
|
|
||
| Fix the out/ run-artifact upload, which failed on every run under gh-aw v0.81.6. The orchestrator passed the absolute path `/tmp/gh-aw/review/out/` exactly as instructed, but gh-aw's upload_artifact tool stages an uploaded directory under its basename and records only the staging-relative name (`out`), and the safe_outputs job then filters the staged files (`out/<agent>.json`) against `allowed-paths` with a fully anchored matcher; the absolute pattern `/tmp/gh-aw/review/out/**` therefore matched nothing, every run annotated `ERR_VALIDATION: upload_artifact: no files matched the selection criteria`, and no sub-agent outputs or `pre-existing.json` reached the run artifacts, blocking post-hoc claim auditing, the live counters, and the A/B runner, which all read them. `allowed-paths` now lists the staging-relative `out/**` (plus the absolute form, since the filter is an OR, in case a future gh-aw matches original paths). Step 9 also copies `claims.json` into `out/` before uploading, so the artifact carries the validator's input alongside its verdicts (`claim-validator.json`) and the provenance gate's set-asides (`pre-existing.json`). | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| --- | ||
| --- | ||
|
|
||
| Docs/tooling only: add the `review-trial` Claude Code skill (`.claude/skills/review-trial/SKILL.md`), packaging the Khan/webapp#40678 seeded-defect live-trial choreography (live A/B plan, phase 5): isolated per-arm PRs with distinct workflow names, run collection, defect-table scoring, corpus export with the private-to-public sanitization gate, and cleanup. Seeds and ground truth stay human-authored; costs are projected and confirmed before any PR is created. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,135 @@ | ||
| --- | ||
| name: review-trial | ||
| description: Run a seeded-defect live trial of the PR review workflow, comparing reviewer arms on isolated copies of one seeded PR (the Khan/webapp#40678 pattern). Use when evaluating an architecture-class review change, a re-review/lifecycle behavior change, or ground-truthing before graduating a repo to automatic mode. Invoke with a consumer repo, a seeded branch, a defect table, and an arms list. | ||
| --- | ||
|
|
||
| # Seeded-defect live trial | ||
|
|
||
| Choreograph the trial pattern from Khan/webapp#40678: one human-seeded PR, | ||
| copied into isolated PRs so each reviewer arm reviews identical content | ||
| without seeing another arm's comments, optionally driven through a re-review | ||
| lifecycle, then scored defect by defect and exported into the eval corpus. | ||
| This is the playbook's architecture-bet instrument; per-change evals belong to | ||
| the corpus A/B (`workflows/review/eval/live-ab-plan.md`), not to this skill. | ||
|
|
||
| ## What stays human | ||
|
|
||
| Do NOT author seeds or ground truth yourself. The operator supplies both; a | ||
| model authoring its own seeds would grade its own homework. If either is | ||
| missing, stop and ask; do not improvise a defect table from the branch diff. | ||
|
|
||
| ## Required inputs (collect all before doing anything) | ||
|
|
||
| 1. **Consumer repo** (e.g. `Khan/webapp`) and the **seeded branch** in it. | ||
| 2. **The defect table**: one row per seed with `key`, `path`, an approximate | ||
| line window, `mechanism` (2-4 keyword/regex alternates describing the | ||
| causal defect), intended severity, and any deliberate non-defect traps | ||
| (rows the reviewers must NOT flag, with the exculpating evidence named, | ||
| e.g. "the wrapper chunks DeleteMulti; see internal/datastore/client.go"). | ||
| 3. **The arms**: for each, a name plus its reviewer source, one of: | ||
| - `repo-default`: whatever reviewer the consumer repo already runs. | ||
| - `workflow @ <ref>`: the shared review workflow pinned to a tag/branch | ||
| (a candidate build or a prior release). | ||
| - `hosted`: the Claude Code GitHub app review (`@claude review`). | ||
| 4. **Lifecycle plan** (optional): the push-2 content (fixes mixed with fresh | ||
| seeds, plus their defect-table rows) and the push-3 content (everything | ||
| fixed). | ||
| 5. **Budget approval**: project the cost FIRST and confirm. Measured basis: | ||
| roughly $7-10 per workflow-arm run; total is arms x (1 + lifecycle | ||
| pushes), hosted-arm runs billed separately by the app. Print the | ||
| projection and get an explicit yes before creating any PR. | ||
|
|
||
| ## Step 1: isolated arm PRs | ||
|
|
||
| For each arm, create `trial/<slug>/<arm>` from the seeded branch and open a | ||
| PR in the consumer repo (title prefixed `[reviewer-trial]`, body stating it | ||
| is a trial and will be closed unmerged). One PR per arm; never point two arms | ||
| at the same PR, or their comments contaminate each other. | ||
|
|
||
| Per-arm trigger setup: | ||
|
|
||
| - `repo-default`: nothing extra; the repo's reviewer triggers normally. | ||
| - `workflow @ ref`: commit the compiled workflow for that ref onto the arm | ||
| branch. **Give it a workflow name distinct from the repo's own reviewer | ||
| and from every other arm**: same-named gh-aw workflows share a per-PR | ||
| concurrency group and silently cancel each other (this ate a run in the | ||
| original trial). Then suppress the repo's default reviewer on this PR via | ||
| its documented opt-out (`skip-ai-review` label in Khan repos), so exactly | ||
| one reviewer runs per PR. | ||
| - `hosted`: suppress the default reviewer the same way, then trigger with an | ||
| `@claude review` comment. | ||
|
|
||
| Draft PRs generally do not trigger reviewers; open the PRs ready-for-review. | ||
|
|
||
| ## Step 2: run and collect | ||
|
|
||
| For each arm: trigger, then watch to completion (`gh run watch` / | ||
| `gh pr checks`). Collect, per run: | ||
|
|
||
| - The posted review: verdict, every inline comment (path, line, body), and | ||
| the review body (`gh api` on the PR's reviews and review comments). | ||
| - Run artifacts when the arm is the shared workflow (findings, claims, | ||
| verdict JSONs): `gh run download`. Tolerate the known gh-aw staging-path | ||
| artifact bug (an ERR_VALIDATION annotation; the artifacts still upload). | ||
| - Cost and wall clock: billed AI credits from the run summary, run duration | ||
| from `gh run view`. Hosted-arm cost is app-billed; record it as opaque | ||
| unless the operator supplies it. | ||
|
|
||
| Record everything into a working directory as you go; a trial that loses its | ||
| transcripts cannot be scored or exported. | ||
|
|
||
| ## Step 3: lifecycle (when the plan includes it) | ||
|
|
||
| Push the operator's push-2 content to EVERY arm branch (identical commits), | ||
| let each arm re-review, and collect again; repeat for push-3. Score each push | ||
| separately: re-review behavior (thread resolution, duplicate suppression, | ||
| scoping to new hunks) is half the point of the lifecycle. | ||
|
|
||
| ## Step 4: score | ||
|
|
||
| Match each arm's posted comments against the defect table, per push: | ||
|
|
||
| - Deterministic first: same path, line within (or overlapping) the window, | ||
| and any mechanism alternate matching the comment text, case-insensitively. | ||
| This mirrors `workflows/review/eval/live-match.ts`; one comment satisfies | ||
| at most one seed. | ||
| - Ambiguous leftovers: judge manually, and mark judged matches in the report | ||
| so a reader can audit them. | ||
| - Non-defect traps: a comment matching a trap row is a false flag; silence | ||
| on a trap is correct suppression and counts FOR the arm. | ||
|
|
||
| Produce the #40678 report shape: a headline table (seeds caught, verdict, | ||
| comment count, cost, wall clock, per arm), a defect-by-defect table (one row | ||
| per seed, one column per arm), a lifecycle table when applicable, and a short | ||
| writeup naming where each margin came from. Report faithfully: misses, | ||
| wrong severities, and noise comments all go in, whichever arm they favor. | ||
|
|
||
| ## Step 5: export to the corpus | ||
|
|
||
| Every trial compounds the corpus. Emit case skeletons in the live-enabled | ||
| format (`workflows/review/eval/corpus/`, layout `<id>/case.json` + `tree/`; | ||
| format spec in `corpus/live.ts`): the seeded diff, the post-change tree, | ||
| `live.mustCatchSpecs` straight from the defect table, traps as | ||
| `mustNotFlagSpecs` on a clean case, and recorded findings taken from the | ||
| best arm's artifacts. | ||
|
|
||
| **Sanitization gate**: if the consumer repo is private and the corpus repo | ||
| public (Khan/webapp into Khan/actions is exactly this), never copy code, | ||
| paths, or identifiers. Author structural rewrites that reproduce each | ||
| defect mechanism with generic naming, the way the #40678 seeds landed in | ||
| Khan/actions#235. When in doubt, rewrite. | ||
|
|
||
| ## Step 6: clean up | ||
|
|
||
| Close every trial PR with a comment linking the report, delete the | ||
| `trial/<slug>/*` branches, and confirm no temporary workflow file survived | ||
| onto a long-lived branch. Leave the collected transcripts with the operator | ||
| (attach the report to the PR or issue that motivated the trial). | ||
|
|
||
| ## Guardrails (recap) | ||
|
|
||
| - Costs projected and approved before the first PR exists. | ||
| - Seeds and ground truth are operator-authored, always. | ||
| - One arm per PR; distinct workflow names; exactly one reviewer per PR. | ||
| - Score before cleanup; export before cleanup; never lose transcripts. | ||
| - Faithful reporting, including the arm you expected to win losing. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,133 @@ | ||
| # Review live A/B — real-model arm-to-arm evals for review-bot changes. | ||
| # | ||
| # A PR touching workflows/review/** runs the model sub-agents from BOTH the | ||
| # base branch's review.md (baseline) and the PR's review.md (candidate) over | ||
| # the live-enabled corpus subset, then posts the delta report (five live | ||
| # metrics, judge quality, cost, regressions) as a sticky PR comment and job | ||
| # summary. See workflows/review/eval/live-ab-plan.md for the full design. | ||
| # | ||
| # Why the base TIP and not the merge-base: on pull_request events the | ||
| # checkout is the PR merge commit, so the candidate tree already contains the | ||
| # base tip. Baselining on the same tip means the two arms differ only by the | ||
| # PR's own review.md changes; a merge-base baseline would fold any upstream | ||
| # review.md movement into the PR's measured delta. | ||
| # | ||
| # Report-only, with one exception (the playbook's standing rule): the runner | ||
| # exits non-zero when the candidate arm mishandles an adversarial-injection | ||
| # case, which fails this job. | ||
| # | ||
| # Cost control: per-PR runs cover only the live cases also tagged `smoke`; | ||
| # a `full-eval` label lifts that to every live case. The runner enforces a | ||
| # hard USD ceiling per arm and reports skipped cases rather than dying at the | ||
| # cap. A new push cancels a superseded run (concurrency below). A | ||
| # `skip-live-eval` label skips the job entirely. | ||
| # | ||
| # NOTE: this workflow's name must stay distinct from any gh-aw workflow a | ||
| # consumer runs (same-named workflows share a gh-aw concurrency group per PR | ||
| # and cancel each other; see the round-two doc's operational floor). | ||
|
|
||
| name: Review Eval A/B | ||
|
|
||
| on: | ||
| pull_request: | ||
| types: [opened, synchronize, reopened, ready_for_review] | ||
| paths: | ||
| - "workflows/review/**" | ||
| workflow_dispatch: | ||
| inputs: | ||
| base_ref: | ||
| description: "Baseline ref for review.md (default: the runner's merge-base fallback; PR runs pass the base-branch tip)" | ||
| required: false | ||
| max_usd: | ||
| description: "Total hard budget across both arms" | ||
| required: false | ||
| default: "40" | ||
| full: | ||
| description: "Run every live case (not just the smoke subset)" | ||
| type: boolean | ||
| required: false | ||
| default: false | ||
|
|
||
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
|
|
||
| concurrency: | ||
| group: review-eval-ab-${{ github.event.pull_request.number || github.run_id }} | ||
| cancel-in-progress: true | ||
|
|
||
| jobs: | ||
| live-ab: | ||
| name: Live A/B over the eval corpus | ||
| runs-on: ubuntu-latest | ||
| # Drafts wait until ready; the changeset release PR matches the path | ||
| # filter (it bumps workflows/review/package.json) but changes no | ||
| # behavior; skip-live-eval opts a PR out. | ||
| if: >- | ||
| github.event_name == 'workflow_dispatch' || | ||
| (github.event.pull_request.draft == false && | ||
| github.head_ref != 'changeset-release/main' && | ||
| !contains(github.event.pull_request.labels.*.name, 'skip-live-eval')) | ||
| steps: | ||
| - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 | ||
| with: | ||
| # The baseline arm reads review.md from the base ref via | ||
| # `git show`, so the full history is needed. | ||
| fetch-depth: 0 | ||
| - uses: ./actions/shared-node-cache | ||
| - name: Run the live A/B | ||
| env: | ||
| ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} | ||
| # The tip of the PR's base branch (see the header comment for why | ||
| # tip, not merge-base). workflow_dispatch may override it. | ||
| BASE_REF: ${{ inputs.base_ref || format('origin/{0}', github.base_ref || 'main') }} | ||
| MAX_USD: ${{ inputs.max_usd || '40' }} | ||
| FULL_EVAL: ${{ inputs.full == true || contains(github.event.pull_request.labels.*.name, 'full-eval') }} | ||
| run: | | ||
| if [ -z "$ANTHROPIC_API_KEY" ]; then | ||
| echo "ANTHROPIC_API_KEY secret not configured; skipping the live A/B." >&2 | ||
| exit 0 | ||
| fi | ||
| SCOPE="--smoke-only" | ||
| if [ "$FULL_EVAL" = "true" ]; then | ||
| SCOPE="" | ||
| fi | ||
| pnpm dlx tsx workflows/review/eval/live-ab.ts \ | ||
| --base-ref "$BASE_REF" --max-usd "$MAX_USD" $SCOPE | ||
| - name: Upload the report artifact | ||
| # always(): a partial or gate-failing run still uploads what it wrote. | ||
| if: always() | ||
| uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 | ||
| with: | ||
| name: live-ab-report | ||
| path: out/live-ab-report.* | ||
| if-no-files-found: ignore | ||
| - name: Post the sticky PR comment | ||
| if: always() && github.event_name == 'pull_request' | ||
| uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7 | ||
| with: | ||
| script: | | ||
| const fs = require("fs"); | ||
| const path = "out/live-ab-report.md"; | ||
| if (!fs.existsSync(path)) { | ||
| core.info("no report markdown; nothing to post"); | ||
| return; | ||
| } | ||
| const marker = "<!-- review-eval-ab -->"; | ||
| const body = `${marker}\n${fs.readFileSync(path, "utf8")}`; | ||
| const {owner, repo} = context.repo; | ||
| const issue_number = context.payload.pull_request.number; | ||
| const comments = await github.paginate( | ||
| github.rest.issues.listComments, | ||
| {owner, repo, issue_number, per_page: 100}, | ||
| ); | ||
| const prior = comments.find((c) => c.body?.startsWith(marker)); | ||
| if (prior) { | ||
| await github.rest.issues.updateComment({ | ||
| owner, repo, comment_id: prior.id, body, | ||
| }); | ||
| } else { | ||
| await github.rest.issues.createComment({ | ||
| owner, repo, issue_number, body, | ||
| }); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick (non-blocking): The five sibling changesets open with a short noun-phrase label plus a parenthetical plan anchor (e.g.
(live A/B plan, phase 3)); this one opens straight into the fix narrative. A leading label/anchor would keep the generated CHANGELOG consistent.