Version Packages#259
Merged
Merged
Conversation
e199dc5 to
8db74b2
Compare
8db74b2 to
cdf7305
Compare
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
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.
This PR was opened by the Changesets release GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated.
Releases
review@1.5.0
Minor Changes
adversarial-injection-approve,golden-request-changes-authz, the sql-index replay, and production main's own baseline arm). Before setting a line-anchored finding aside, the gate now snaps it to the nearest changed line in the same file under two windows: a near-miss window of 3 lines (the unified diff's context width, so the snap never reaches past the context block the reviewer was reading), and an overflow window for anchors past the end of the file itself (a line number that does not exist, per the file's real length read from the checkout) and past every line the file's hunks show, by no more than the file's diff-text overhead (headers + hunk headers + removed lines — exactly the amount diff-text counting overshoots by, so such an anchor can only be the counting mis-anchor; an anchor on a line that exists never overflow-snaps, since unshown code below a hunk can carry genuine observations that must keep demoting to advisory). Snapping is RIGHT-side only, matching the RIGHT-side-only precomputed table the gate step reads. Equidistant ties break toward the lower line (the pathology overshoots). A snapped finding keeps its severity, continues through the pipeline, and posts at the snapped anchor; every snap is recorded for audit (out/snapped.jsonin the production run artifact,snappedByProvenancein eval run results). A finding outside both windows keeps today's behavior: demoted to advisory, recorded in the artifact, never posted — the gate's purpose is unchanged, and an out-of-scope finding in an untouched region still cannot carry a blocking label.provenance.jsonnow carries a precomputed per-filesnaplookup (plus per-filelastShownLine/textOverheadfacts from the diff parser), so review.md's gate step applies the rule as a pure dictionary lookup with no model-side line arithmetic. The live A/B prices the change per arm: the deterministic gate emulates each arm's own review.md gate version, keyed on the literalanchor-snapmarker in the gate step, so a pre-snap baseline replays the pre-snap gate. A new deterministic smoke case (provenance-anchor-snap-rescued, the observed production anatomy) pins the rescue, the verdict, and the unchanged far-anchor set-aside in the per-push CI gate.annotateDiffLineNumbers(lib/diff.ts) prefixes each hunk content line with its line number (+/context lines carry the NEW-file RIGHT-side number,-lines the OLD-file LEFT-side number) while keeping the diff marker in column one, so annotated text still splits into file sections. The provenance CLI writesfull-stripped-annotated.diffbeside the raw stripped diff, and a newannotate <in> <out>subcommand producespr-annotated.diffafter Phase 1 buildspr.diff(the scoped and flip-gated depths refresh the annotated copies the same way). Every finding-producing reviewer (correctness, skill-auditor, conventions, the four whole-change reviewers, and all eleven specialist lenses via the shared disciplines block) now reads the annotated copy, takesanchor.linefrom the printed number, and strips the prefix when quoting code or authoring asuggested_patch. Everything that PARSES a diff keeps reading the raw files: provenance, re-review hunk fingerprints (whose signatures must not shift), scoped staging, and pattern-triage/claim-validator are untouched. The eval stages the annotated siblings for both arms unconditionally, and only a review.md version that names them reads them, so the A/B against a pre-annotation baseline is a pure prompt delta with no staging flag. The measurement instrument rides along: per-case anchor-snap counts (perCase.snapped) in the arm report and a pooled "Findings anchor-snapped" row in the aggregate, version-tolerant of older artifacts — if annotation works, candidate-arm snaps fall to zero because anchors arrive correct, with the anchor-snap gate remaining as the deterministic backstop.eval/live-match.tsmaps 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.tsruns the model sub-agents from two review.md versions (the merge-base baseline viagit 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.tsgains avalidationoverride onRunOptionsso 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 toeval/judge-live-model.ts, now shared withlive-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.liveblock 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>.jsonlayout), 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 thelivetag, a cleanly-parseable diff, and every non-removed changed file in its tree; the loader enforces all of it andloadLiveCorpus()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.eval/agent-extract.tsparsesreview.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 agit showof the merge-base; parsing is strict and lists every malformed section at once, and an integration test runs it over the realreview.md.eval/live-stage.tsmaterializes 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.tsdispatches 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 exactRecordedFinding/CaseVerificationshapes the deterministic runner consumes, stagesclaims.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.tsis 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).out_of_lane_observations[](path, optional line, the concern stated concretely, a required concretefailure_scenario, optionalsuggested_lane) for real concerns their own mandate does not let them report. The orchestrator converts each one into a candidate comment with the code-assigned labelquestion (non-blocking)(a handoff is not a vetted finding and can never block on its own; the claim-validator never upgrades severity) and routes it through the identical provenance gate, scope filter, claims.json, and validation path as every other candidate. The shape is validated by the newvalidateOutOfLaneObservationinlib/finding-schema.ts(a sibling type, soFINDING_SCHEMA_VERSIONstays 2: no serialized finding is invalidated).thread-reconciler's keep/resolve lists by the newlib/rereview.ts(never composed by the model): each still-unaddressed prior thread is enumerated as a link to its earlier comment ("as of \<sha>", blocking first, with a verbatim excerpt of the earlier comment's first line), with the resolved count stated; an approval that resolved the last open threads says every prior thread is resolved.threads.jsonstaging gains aurlfield (the thread's first commenthtml_url) to power the links,renderReviewBodygains arereviewSectionslot, and the redundant-approval skip treats a non-empty section as content. Fail-open: a missing or unparseable staging input renders an empty section, leaving the body exactly as before.live.rereviewblock that makes it an open-PR snapshot instead of a first review, staging the prior review's unresolved threads (author replies included), a stamped prior review derived frompriorDiff(the same hidden-comment mechanics production reads), and the mode dial's depth plan. The live producer dispatches thethread-reconcileron such cases at every depth and sizes the finder roster by the plan (scopedkeeps the roster over the scoped diff,flip-gatedkeeps the correctness pass,fastreconciles only);eval/rereview-match.tsscores thread-resolution accuracy per ground truth, the flip-gate input, and duplicate comments on kept threads, and the A/B runner prices a mode via--re-review-modeon the candidate arm. The deterministic replay learns the same rules:computeVerdictgainskeptBlockingCount(the flip rule as code: an approval is floored at REQUEST_CHANGES while a prior blocking thread stays open) and re-review cases render the accountability section into the planned body. Ships with thegolden-retention-lifecycle-1/2/3chain, a sanitized port of a measured seeded live trial: planted bugs, a bad partial fix with added bugs (kept threads plus fresh defects), then the full fix that must flip to APPROVE. A mode sweep (eval/rereview-sweep.ts) prices every dial setting in one command: it runs the working tree's reviewer over the rereview cases at each mode and reports recall, thread resolution, flip-gate correctness, duplicates, and dollars per mode, with the executed depth per case (the tripwire may override the dial).golden-retention-fix-pushis the under-threshold pricing case: a one-hunk fix push (unreviewed share 0.2) whose fix resolves the prior blocking thread but plants a fresh defect inside the new hunk, so scoped and flip-gated must catch it and fast definitionally cannot. The sweep is dispatchable in CI (review-rereview-sweep.yml, manual only since it spends model dollars); the table lands in the job summary and the JSON report in the run artifact. Every model-spending eval now has the same manual trigger surface: PR labels (rereview-sweep,live-judge,full-evalnow firing on the labeling event itself) plusworkflow_dispatch; and the A/B short-circuits before spending whenreview.mdis byte-identical in both arms, posting a "no reviewable delta" verdict and running nothing (--force-armskeeps deliberate wobble controls possible).re-reviewline in.github/aw/review/ROUTING(full|scoped|flip-gated|fast, defaultfull) decides how much of the roster a repeat review of the same PR runs.scopedruns the whole roster staged only the hunks new since the last fully-reviewed fingerprint;flip-gatedruns reconciliation plus the correctness pass, whose validated blocking findings veto a REQUEST_CHANGES→APPROVE flip;fastis reconcile-only. Three deterministic guards (lib/rereview-mode.ts): the anchoring full review is taken at ready-for-review (never on a draft skeleton); the flip gate; and a divergence tripwire that stamps a content-hashed hunk signature into the review body as a hidden comment (surviving cache eviction, dismiss-stale-approvals, and COMMENTED-only histories) and re-arms full review when the unreviewed share reaches 0.4. Adversarial lifecycle cases (rewrite-after-approval, sparse-PR-then-payload) live ineval/lifecycle/and are replayed deterministically; run cost now aggregates per executed depth (costByRereviewDepth) so the live A/B can pricescopedagainstfullon recall and dollars. Ships default-full: no consumer's behavior changes until its repo adds a mode line. Motivated by measured lifecycle costs on the seeded trial (v1.3.1: $7.19/$8.77/$9.14; v1.4.0: $8.31/$9.87/$11.29 per push), where a fresh seeded defect was caught on a re-review push that a reconcile-only path would have missed; the case forscopedas the recommended reduced depth. Supersedes the consumer-side stopgap in Khan/frontend#13420.conventions). The shed order is now explicit, lowest value first: conventions, first-principles, holistic, completeness/test-adequacy, and path-triggered specialist lenses last, since a matched lens is the most targeted signal in the run. The same ranking read from the other end (defaults, lenses, targeted opt-ins, generic opt-ins) is now the mandated dispatch order whenmaxReviewerInvocationscannot fit the full roster, with every undispatched reviewer recorded as a planned shed. And reviewer requests (Step 8) are removed from the shed list entirely: pulling a human in matters most on exactly the run whose own coverage is partial. The ranking is a first-cut editorial ordering; replace it with measured per-dimension must-catch contribution from the eval corpus when that data is compiled.evidence_trace, but evidence traces never reach the PR: the author reads onlymodel_authored_prose, so they see a paraphrase of a rule they cannot check. The finding schema gains an optionalrule_quotefield (the exact rule text, verbatim from the skill file; validated non-empty when present, and optional, soFINDING_SCHEMA_VERSIONstays 2), each lens's lens-owned-skills discipline says to fill it, andrenderCommentplus the orchestrator's normalization step surface it into the posted comment as a> **Rule:** …blockquote between the prose and any suggestion block. Only the wrapping is code-owned; the quote is skill-file text copied verbatim.Patch Changes
25a1ef1: Clamp the router's run budget to the effective per-run AI-credits cap, and make the cap visible to the run. The tier budget table is sized inside the workflow's assumed $10 ceiling, but the cap is enforced runner-side by the firewall api-proxy and was invisible in-container, so a consumer with a tighter
max-ai-creditsgot soft targets that promised more work than the cap could pay for; the graceful-degradation path never fired and the run died at the api-proxy with findings in hand (observed on a 400-credit behavior test: three finders completed, ~390 credits spent, killed mid-persist before validation, nothing posted). The frontmatter now setsmax-ai-credits: 1000explicitly and mirrors it into the agent environment asREVIEW_MAX_AI_CREDITS(kept in sync by consumers that override the cap); the router resolves the cap from that mirror (falling back toGH_AW_MAX_AI_CREDITS, a visible awf config, then gh-aw's 1000-credit default) and scales the selected tier's soft targets proportionally, floored at the trivial tier's values, markingrunBudget.capClampedandrunBudget.effectiveCreditCapin routing.json. The budget prompt gains a matching estimated-credits proxy (sum of in-bandsubagent_tokensover completed sub-agents, divided by 5,000) and two mandatory proxy checkpoints, the critical one immediately after the last finder returns and before claim validation, so a tight run sheds and lands a partial review instead of dying at the cap.53b6815: Dispatch-tax trim: dedupe the specialist-lens discipline snippets. The v1.4.0 re-run's cost premium vs the v1.3.1 baseline sat at dispatch time (uncached input +51-65%, cache writes +18-34% per run), partly because the shared discipline blocks (bounded investigation, untrusted input, lens-owned skills with quote-the-rule, the schema-rules trailer, the tri-state hunt contract) were stamped verbatim into every one of the eleven specialist-lens definitions and paid on every dispatch. The shared text now lives once, in a marker-delimited "REVIEW DISCIPLINES" section of review.md's main body; Step 1 stages it to
/tmp/gh-aw/review/disciplines.mdwith one whole-line-anchoredsedextraction of the engine-provided rendered prompt ($GH_AW_PROMPT), verified by grep with a byte-for-byte heredoc fallback, and each lens carries only a pointer plus its own domain notes (its investigation examples, review rules, hunts, and output JSON, all unchanged). Behavior-neutral by construction: the instruction content is unchanged, the three variants that had already drifted apart are unified on the fullest wording, and the label-shape reviewers (whose variants differ materially) keep their own copies. Pinned bylib/disciplines.test.ts: the section extracts cleanly under the exact sed range semantics, every lens points at the staged file and carries no residual copy, and the label-shape reviewers still carry theirs.6b1b3c7: Fail-fast case selection in the live A/B runner. An explicit
--caseslist is now an exact selection: it bypasses--smoke-only(which scopes unscoped runs only), preserves the requested order, runs duplicate ids once, and throws before any model spend when an id matches no live case. Previously the smoke scope filtered the corpus before the case filter, so a dispatch naming a non-smoke case silently dropped it: the 2026-07-10 anchor-snap powered run named two cases and the paid report covered one without saying so, and a typo'd case id would shrink a measurement the same way. The workflow needs no change; it already passes both flags and the case list now wins.2868f95: Make the live eval instrument a measurement tool, not just a tripwire (the tuning memo's rev-2 sizing items).
eval/aggregate.tspools Nlive-ab-report.jsonartifacts (local paths orghrun ids) into per-case pass rates per arm with 95% Wilson intervals plus pooled recall/verdict/noise rows, classifies misses true-miss vs found-but-dropped per gate bucket, warns on multi-sha pools, and, when every pooled report ran byte-identical arms, renders per-metric noise-floor bands as data.live-ab.tsgains--repeats <n>(the ~$29 targeted-repeats powered run: each arm runs every selected case n times in one dispatch, budget split per arm-run, report rendered through the aggregation core; the adversarial gate is decided by strict majority across repeats instead of the single-run best-of-three retry) and the A/B workflow exposescases/repeats/force_armsdispatch inputs. A new scheduled workflow (review-eval-drift.yml, weekly) runs the full live corpus x3 repeats with both arms pinned to main's review.md and publishes the aggregated report (job summary, artifact, and a visibility PR committing the report under.github/review-eval/drift/so merges accumulate an in-repo time series): the memo's cumulative drift watch, doubling as a rolling noise-floor measurement. Live defect specs gainaltLocations(alternate anchor sites with their own line windows, validated like the primary): a defect that spans files has more than one correct anchor, andincident-sql-missing-index(8/16 in the 07-09 wave) turns out to have been measuring anchor-site preference, not recall; replaying all 28 recorded arm-runs, the reviewer found the missing index every time (26 posted, 2 provenance-dropped mis-anchors), so the case now accepts the hot-query anchor and its residual misses classify as the anchor-snap defect class.eval/match-arbiter.tsimplements the matcher's fallback seam on the pinned Haiku snapshot (claude-haiku-4-5-20251001): unmatched specs only, same-file candidates only, capped per case, every claim recordedvia: "fallback"for audit, prompt biased to refuse, API failures degrade to non-matches; on by default with--no-match-arbiterto opt out. Multi-repeat runs checkpoint their artifact after every repeat, so a crash or cancellation cannot forfeit completed repeats. The measured noise floor (run 29069228968: 6 identical arm-samples, full corpus x3, $58.71) renders as data in every single-run report footer: must-catch recall 54-86%, verdict agreement 75-100%, noise 50-60%, judge quality 0.82-0.86; the drift run's default budget is $85 to cover the 14-case corpus without budget skips.eval/README.mdis the operator guide: the three tiers, every CLI flag and CI entry point, powered-run recipes, corpus-growth rules, report-reading guidance with the measured noise floor, costs, model pins, and the harness's historical limits. Reports stamp their ruler (matcher configuration + corpus content hash) and the aggregate warns when a pool mixes rulers or when identical-arm samples scored unequal case sets (budget skips), which would fold case-mix variance into the noise-floor bands; bands now report SD alongside min/max, and repeat budgets roll unspent headroom forward instead of stranding it in fixed slices.d26b4e0: CI wiring for the live A/B (live A/B plan, phase 4): the
Review Eval A/Bworkflow runs on every non-draft PR touchingworkflows/review/**(plusworkflow_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; afull-evallabel (or the dispatch input) lifts that to every live case, askip-live-evallabel 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-onlyand writes a markdown sibling of the JSON report for the comment step.82aed74: Document the
GH_AW_OTEL_SENTRY_ENDPOINT/GH_AW_OTEL_SENTRY_AUTHORIZATIONsecrets as a hard install prerequisite (README required-secrets section plus the comment on theobservability:block). A consuming repo without them does not degrade gracefully: the compiled lock feeds the empty endpoint into the MCP gateway's OTLP config, whose schema rejects it, and the agent job dies at startup (observed on Enable the automatic AI PR reviewer on this repo #241). Repos without Sentry must comment out theobservability:block in their installed review.md and recompile.2f003f7: 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) againstallowed-pathswith a fully anchored matcher; the absolute pattern/tmp/gh-aw/review/out/**therefore matched nothing, every run annotatedERR_VALIDATION: upload_artifact: no files matched the selection criteria, and no sub-agent outputs orpre-existing.jsonreached the run artifacts, blocking post-hoc claim auditing, the live counters, and the A/B runner, which all read them.allowed-pathsnow lists the staging-relativeout/**(plus the absolute form, since the filter is an OR, in case a future gh-aw matches original paths). Step 9 also copiesclaims.jsonintoout/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).adb052f: Require the introduce-vs-amplify call in the posted comment prose, not just the finding. The discipline already made finders classify a defect as introduced or as a pre-existing mechanism amplified, but nothing required the classification to survive into the text the author reads: on the amplification behavior test the posted comment argued the distinction in substance (named the pre-existing default and attributed the regression to the guard removal) without ever stating it. The finder rule now requires a plain clause in the
discussion("introduced by this change", "pre-existing; this change amplifies it by removing the guard"), including the boundary case where the enabling mechanism predates the diff but the defect is new, and the claim-validator's amplification rule extends itscorrectedpath to add the missing clause on either shape.8a5783a: Budget and shed tuning from the 2026-07-10 production sheds (the reviews on Khan/actions review: add the live A/B eval plan #232 and review: add the review-trial skill (live A/B phase 5) #238, which shed claim validation and three whole-change dimensions on substantial PRs that had routed to the bottom tiers). Three changes. The tier budget table is recalibrated against measured runs: wall clocks now account for the ~3 minutes of fixed staging/router/triage overhead a run spends before the first reviewer returns (trivial 3->6, low 6->10, medium 12->15), and the low/medium invocation caps now fit the standard seven-reviewer whole-change roster (low 4->8, medium 8->10), which the old low cap of 4 deterministically shed on every run. The invocation cap is now explicitly finding-producers only: pattern-triage, thread-reconciler, and the claim-validator are pipeline steps and never consume a slot (the 07-10 runs counted them inconsistently). And the claim-validator moves to the back of the shed order with a hard-ceiling gate: it may be shed only when the job timeout or credits cap is genuinely close, never at a mere soft-target breach, since its cost tracks the visible candidate count rather than the diff. Skipped-dimension notes now distinguish the two causes: a planned budget shed reads
shed under the <tier>-tier run budgetand theoutput unavailablewording is reserved for a sub-agent that was dispatched but returned nothing parseable, so an operator can tell budget arithmetic from an infrastructure failure. (This repo's ROUTING also re-raises.claude/skills/**andworkflows/review/eval/**/*.mdfrom the broad trivial-docs rule; that file is consumer config, not part of the package.)d615f20: Add three live-enabled eval corpus cases porting the Khan/webapp#40678 seeded-defect trial into the review-workflow corpus. All three are sanitized structural rewrites: fresh Go code around a generic "notes retention" feature that reproduces the trial's defect mechanisms, carrying no webapp code, paths, or identifiers.
trial-retention-deletion(incident-repro): the deletion-path seeds; a flag-gated compliance deletion, a query-default limit of 1, a reimplemented deletion helper, an env-interface widening flagged in both files, and a swallowed prune error.trial-retention-prune-tests(incident-repro): the prune-and-tests seeds; an off-by-one retention cap, a vacuous cap test that passes for a no-op prune, a suite-wide flag-ON mock hiding the flag-off path, and a full-entity fetch where keys-only suffices.trial-batch-delete-wrapper(clean): the trial's deliberate non-defect as a must-not-flag trap; a large single DeleteMulti call that looks over the datastore's 500-entity cap but is chunked internally by the (unchanged, in-tree) datastore wrapper. The recorded false block is refuted by validation and the case must approve.