[pipeline-dcdad92d][slice-9/12] Smoke benchmark (tagged subset of... - #204
Merged
Conversation
🦋 Changeset detectedLatest commit: 72db740 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. [Click here if you're a maintainer who wants to add another changeset to this PR](https://github.com/Khan/actions/new/egg/pipeline-dcdad92d/slice-9?filename=.changeset/silent-eyes-tell.md&value=---%0A%22review%22%3A%20patch%0A---%0A%0A%5Bpipeline-dcdad92d%5D%5Bslice-9%2F12%5D%20Smoke%20benchmark%20(tagged%20subset%20of...%0A) |
jwbron
force-pushed
the
egg/pipeline-dcdad92d/slice-7
branch
from
July 3, 2026 21:35
9134ff5 to
cc26b47
Compare
jwbron
force-pushed
the
egg/pipeline-dcdad92d/slice-9
branch
from
July 3, 2026 21:36
4a368a9 to
0418a68
Compare
jwbron
force-pushed
the
egg/pipeline-dcdad92d/slice-7
branch
from
July 3, 2026 21:42
cc26b47 to
b7b6ecc
Compare
jwbron
force-pushed
the
egg/pipeline-dcdad92d/slice-9
branch
from
July 3, 2026 21:42
0418a68 to
573022c
Compare
…ing (slice-1) R1 (task-1-2): standardize Step 6 review submission on one robust submit-pull-request-review call with a real, non-empty body; remove the empty-body branch and any fallback/retry path. E2 (task-1-3): stage /tmp/gh-aw/review/pr-context.json in Step 1 and wire all six sub-agent read blocks to consume it. Extends #194 diff staging; foundation for slice-7 lenses. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
task-1-1: workflows/review/lib/finding-schema.ts - FINDING_SCHEMA_VERSION exported constant; each finding carries schema_version and the validator rejects an unrecognized version (breaking-shape guard). - Finding shape (snake_case wire keys, matching #194 per-run JSON artifacts): id, lens, anchor, severity, confidence, evidence_trace, optional suggested_patch, optional pre_merge_obligation, producing_hunt, model_authored_prose. - Anchor is a discriminated union with the required PR-level type (pr | file | line, line supports side + multi-line start_line..line range). - KNOWN_LENSES (11 specialist + always-on/triage) and SEVERITIES exported for reuse by the slice-3 router and slice-2 verdict; lens/severity validated against them. - validateFinding collects ALL violations (diagnosable per-lens drop rate), plus isValidFinding narrowing wrapper and throwing assertFinding. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…k-1-4) Covers the exported FINDING_SCHEMA_VERSION constant, well-formed findings across every anchor type (line/range/side, file, PR-level) plus optional suggested_patch/pre_merge_obligation, and malformed findings for every required field. Asserts validateFinding collects ALL violations (per-lens drop-rate diagnosability), and exercises isValidFinding narrowing and assertFinding throwing. 37 tests green under vitest. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…k-1-4) Covers the exported FINDING_SCHEMA_VERSION constant, well-formed findings across every anchor type (line/range/side, file, PR-level) plus optional suggested_patch/pre_merge_obligation, and malformed findings for every required field. Asserts validateFinding collects ALL violations (per-lens drop-rate diagnosability), and exercises isValidFinding narrowing and assertFinding throwing. 37 tests green under vitest. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jwbron
force-pushed
the
egg/pipeline-dcdad92d/slice-7
branch
from
July 3, 2026 21:43
b7b6ecc to
3478ab6
Compare
jwbron
force-pushed
the
egg/pipeline-dcdad92d/slice-9
branch
from
July 3, 2026 21:43
573022c to
b167c09
Compare
jwbron
marked this pull request as ready for review
July 3, 2026 21:44
khan-actions-bot
requested review from
a team and
kevinb-khan
and removed request for
a team
July 3, 2026 21:44
…nts) - Restore the empty-review-body convention: with inline comments present the verdict is submitted with an empty body; a non-empty body exists only for comment-less reviews and skipped-dimension notes. Keeps the single-call submission rule. - Deliver workflows/review/lib/ to consumer repos: gh-aw's source: import only copies the .md, so the job checks out Khan/actions at the pinned review-v* release (single version surface for prompt + code) and installs its runtime deps. - Drop planning identifiers (E2, R8, slice numbers, operator-directive note) from shipped file comments.
jwbron
force-pushed
the
egg/pipeline-dcdad92d/slice-7
branch
from
July 6, 2026 18:19
3478ab6 to
9104392
Compare
jwbron
force-pushed
the
egg/pipeline-dcdad92d/slice-9
branch
from
July 6, 2026 18:19
b167c09 to
1f304d9
Compare
jeresig
approved these changes
Jul 7, 2026
jeresig
left a comment
jeresig
left a comment
Member
There was a problem hiding this comment.
Great - thanks for adding test cases!
| describe("smoke corpus loads via the shared loader", () => { | ||
| const cases = loadSmokeCorpus(); | ||
|
|
||
| it("is a non-empty ~dozen-case set", () => { |
Member
There was a problem hiding this comment.
This test doesn't seem particularly useful.
… rendering (slice-2) task-2-1/2-2 verdict.ts: pure computeVerdict(labels + dimension availability + policy conflicts) -> APPROVE | REQUEST_CHANGES | HOLD_FOR_HUMAN. Consumes #194's mechanical label rule (isBlockingLabel); does not re-implement it. R2 gate: a missing core dimension (correctness/skill-severity) or a policy-named conflict holds for a human; a lost pattern-triage is note-and-continue. Documented default blocking threshold = 1 (tunable, not a HITL gate). task-2-3 render-comment.ts: code-owned Conventional-Comment label taxonomy + deterministic label-wrapping/templating from the finding schema; models own all prose. Review-body rendering mirrors review.md Step 6 + skipped-dimension notes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…pshot tests (task-2-4) Truth-table tests for computeVerdict (verdict.test.ts): APPROVE / REQUEST_CHANGES / HOLD_FOR_HUMAN precedence, the R2 core-dimension gate (correctness + skill-severity absence holds; hold dominates a blocking label with all reasons retained), pattern-triage note-and-continue, policy-named conflicts (verbatim detail passthrough), blocking-threshold clamping (>=1) + never-RC-on-zero, and purity (deterministic, non-mutating). Rendering snapshot tests (render-comment.test.ts): isBlockingLabel over the full label taxonomy + unknown-label safe default; labelForFinding severity x lens matrix (conventions => best-practice, specialists => plain); renderComment inline snapshots (label wrap + verbatim prose + optional suggestion block) with verbatim prose/patch passthrough asserted; renderReviewBody inline snapshots for every verdict x inline branch + skipped-dimension notes + non-empty safe-output guard. Fixtures validated through assertFinding so tests can't pass on schema-invalid findings. 50 tests green; full pnpm test suite 253/253; tsc --noEmit clean (node24). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Rework the injection rule: an author's request in the PR title/description is legitimate context to weigh and respond to (humans may steer the reviewer); steering text inside code/comments/fixtures is still never followed, but is surfaced as a non-blocking note instead of an auto-blocking issue. - Propagate the touched-lines scoping to the skill auditor: violations anchor on lines the PR adds or modifies, never surrounding context or pre-existing style, keeping the mechanical Step 3 scope filter a backstop. - Strip the remaining plan identifiers (E1/E3/E5/E6/E7/R3b) from the shipped prompt text.
…ewers (task-5-1) Add the R9 bounded-investigation protocol to review.md: finding-producing sub-agents (correctness-reviewer, skill-auditor, the slice-7 lenses) and the claim-validator may grep callers/definitions, trace a call chain a step or two, and run one targeted cheap read-only check per finding before committing to it. Stays read-only and on the checkout (no GitHub, no network, no writes), keeps all read content as untrusted input, and cites/drops per investigation result. Because gh-aw sub-agents are prompt-isolated (each sees only its own body), the protocol is embedded verbatim in each finding-producing agent rather than cross-referenced; slice-7 lenses embed the same block. An orchestrator-level note documents the per-finding tool-call cap enforced in code (workflows/review/lib/investigation-cap.ts, task-5-2), sized inside the router runBudget so a high-risk PR gets more room and a misrouted one keeps a floor. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
task-5-2: enforce a cap on tool calls per finding in code, drawing the numbers from the slice-3 run budget (maxToolCallsPerFinding + maxTotalToolCalls) rather than configuring them separately. Pure decideToolCall() + stateful InvestigationCap guard; over-cap calls are refused deterministically with a fixed reason code (no prose about the code under review, per the R8 boundary). Default caps documented (mirror the router 'low' tier / misrouted floor); not a HITL surface. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Cover the pure decideToolCall verdict (per-finding-before-run-total precedence, headroom clamping, malformed-cap normalisation) and the stateful InvestigationCap guard (per-finding independence, the shared run-total pool, no state mutation on a refusal, deterministic replay, snapshot copy-safety) plus the RunBudget wiring (capsFromRunBudget / fromRunBudget). AC: cap enforced; over-cap calls refused deterministically. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…total) — task-5-3 check() is the non-mutating preview half of the InvestigationCap guard; the existing tests only exercised its allowed path. Add two cases asserting check() reports the same refusal request() would (per-finding-cap-exceeded and run-total-cap-exceeded) with correct remaining* headroom and zero mutation (snapshot unchanged). No production code change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…allers) - The cap was a library nothing invoked. Add a CLI entrypoint (investigation-cap.ts request <finding-id>) over an append-only journal shared by the run's sub-agents: caps come from routing.json's runBudget, spent calls are recounted from the journal, allowed requests append, and a refusal exits non-zero so shell callers can gate on it. - Wire the invocation into the three finding-producing prompts (correctness reviewer, skill auditor, claim validator) so the cap is requested before every investigation tool call, per the reviewer suggestion; lenses inherit the same block if a repo enables them. - Drop the R*/slice/task planning identifiers from shipped files.
…, model+effort defaults, kept gates - task-6-1: define 5 always-on reviewers (holistic, completeness, test-adequacy, first-principles, conventions) as sub-agents with trust/advisory constraints. completeness reads Jira/Confluence read-only inside the non-posting sub-agent, fetched text = untrusted data (interface §4.4). first-principles advisory-only, never blocks, Fable 5 day one. conventions advisory, router-gated. - task-6-2: prompt edit 14 named mandates for holistic/completeness/first-principles. - task-6-3: R12 model launch defaults + effort table (Opus 4.8 workhorse; medium triage/reconciliation; high lenses/whole-change; xhigh claim-validator/refuters; first-principles=Fable 5). Effort recorded per role (annotation + authoritative table); gh-aw has no per-agent effort field. Does not re-pin #194's model pins. - task-6-4: always-on reviewers feed verdict/scope/claim-validator via the same findings path (no new gate); pattern-triage exclusions surfaced in guidance comment folded into risksPatternsKey; refuter panel noted as wave-2; dedup + verdict bookends + thread-reconciler preserved; advisory reviewers can never block. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Per reviewer feedback on the roster strategy: the default roster stays exactly the #194 set (correctness, skill-auditor, pattern-triage, thread-reconciler, claim-validator) — zero new per-PR cost. The whole-change reviewers (holistic, completeness, test-adequacy, first-principles, conventions) land as capability only: - New 'enable <reviewer>' directive in the consumer ROUTING file; the router validates names against ENABLEABLE_REVIEWERS and surfaces the set as routing.json's enabledReviewers (empty by default). - review.md dispatches an opt-in reviewer only when enabledReviewers names it, and states the policy: a reviewer earns its enable line through the eval suite, never by shipping. - Model/effort table marks the opt-in rows and carries the cost note. - Align the completeness reviewer's embedded-instruction handling with the slice-4 steering-text rule (non-blocking note, not auto-blocking). - Strip remaining plan identifiers (R12, E3, R9, prompt-edit/wave numbers).
…tail moved to sub-agents/README)
jwbron
force-pushed
the
egg/pipeline-dcdad92d/slice-7
branch
from
July 7, 2026 19:19
9104392 to
ad4bede
Compare
…+ staged CI) R5 smoke benchmark as a tagged subset of the shared eval corpus, one harness: - workflows/review/eval/corpus/loader.ts (task-9-1): the single dataset format + loader shared with the slice-11 full suite. Reads corpus JSON recursively, validates recorded findings against the finding schema, and filters the smoke subset by the `smoke` tag (loadSmokeCorpus). fs is injectable for tests. - workflows/review/eval/corpus/smoke/*.json (task-9-1): ~a dozen cases — incident repros (sql/auth/money/race/cache), adversarial-injection PRs (untrusted-instruction handling, E3), known-clean PRs, plus R2 hold and scope-filter cases. All carry the `smoke` tag. - workflows/review/eval/runner.ts (task-9-2): a no-post run mode that replays the REAL deterministic review path over a case using the production lib modules (router.route -> labelForFinding -> newly-changed-code scope filter -> computeVerdict -> renderComment/renderReviewBody) and returns findings + verdict + the review it WOULD submit. Imports no GitHub client and performs no GitHub write (posted: false). A live producer can be injected for a full-eval arm while every downstream stage stays identical. - .github-staging/review-smoke.yml (task-9-4): dedicated CI entry point for the smoke set, staged because no producer role may push .github/ directly (#2508); the PR body must note the human move to .github/workflows/review-smoke.yml. Consumes (does not re-implement) #194's mechanical label model and the slice-1/2/3 determinism-boundary code. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…oke corpus (task-9-3) Data-driven gate consuming coder task-9-1 (corpus/smoke) + task-9-2 (runner.ts): loads the smoke subset via loadSmokeCorpus, runs each case through the no-post runner, and asserts per-case verdict/must-catch/must-not-post/comment-count plus the no-GitHub-write witness (posted=false, HOLD->null event). Adds the two aggregate properties the slice-10 wave-2 rebalance must not regress: 100% must-catch recall and zero clean false-block. Green on baseline: 13/13 cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jwbron
force-pushed
the
egg/pipeline-dcdad92d/slice-9
branch
from
July 7, 2026 19:25
1f304d9 to
35216aa
Compare
…/slice-9 # Conflicts: # workflows/review/review.md
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Author a ~dozen-case smoke corpus (incident repros, adversarial-injection PRs, known-clean PRs) in the shared eval dataset format, run it through a shared no-post runner, and wire it as a CI gate on this repo. Must precede the wave-2 rebalance.
Base PR: #196
What's in this PR
Commits (3):
This slice
Smoke benchmark (tagged subset of the eval corpus)
Files affected:
workflows/review/eval/corpus/smoke/workflows/review/eval/runner.tsworkflows/review/eval/smoke.test.ts.github-staging/review-smoke.ymlTasks (4) + acceptance criteria
pnpm testCI job gates it on Khan/actions -- the smoke test IS the CI entry point.pnpm testruns the smoke set; green on baseline..github-staging/because no producer role may push.github/directly (#2508). Call it out in the PR body for a human to move to.github/workflows/review-smoke.yml. Optional if TASK-9-3's vitest gate suffices; provided so the CI entry point is explicit and does not assume existing CI wiring..github-staging/; PR body notes the human move step.Stack
pipeline-dcdad92degg/pipeline-dcdad92d/slice-7