From db1405e8bb6625c4314bb58a0c2ff14a58dc6822 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 31 Oct 2026 16:16:25 +0000 Subject: [PATCH] fix: pre-landing review fixes (Step 11: key the first-pass cut on the step history too; CRITICAL-path wording; repin lib/review-depth.ts under no-loss replay V7) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- changelog.d/gsta-167-first-pass-red-team.md | 4 ++-- docs/review-policy.md | 11 +++++++---- lib/review-depth.ts | 8 ++++---- test/review-depth.test.ts | 4 ++++ test/review-packets-removed.test.ts | 2 +- 5 files changed, 18 insertions(+), 11 deletions(-) diff --git a/changelog.d/gsta-167-first-pass-red-team.md b/changelog.d/gsta-167-first-pass-red-team.md index d821662cd..f9950bbdf 100644 --- a/changelog.d/gsta-167-first-pass-red-team.md +++ b/changelog.d/gsta-167-first-pass-red-team.md @@ -1,7 +1,7 @@ -**`/ship` no longer runs the standalone Red Team on the first full review pass of an invocation at gate class or past 200 runtime lines.** The preregistered no-loss replay (GSTA-167 Round 7, handoffs/GSTA-167/noloss7-results.md) removed those Red Team lines from the recorded reviewer outputs. The rest of the stack still found 32 of the 41 panel blockers against 33 with them, a one-sided non-inferiority p of 0.00074 at a margin of 4. The Red Team still runs on a small diff raised to standard, on every post-fix full pass and on the scoped pass. A first pass that logs a CRITICAL gets it on its post-fix pass, because `/ship` passes `--critical-found` only to the repeat plan. A new `/ship` after a stop starts counting passes again, so its first full pass has none either. +**`/ship` no longer runs the standalone Red Team on the first full review pass of an invocation at gate class or past 200 runtime lines.** The preregistered no-loss replay (GSTA-167 Round 7, handoffs/GSTA-167/noloss7-results.md) removed those Red Team lines from the recorded reviewer outputs. The rest of the stack still found 32 of the 41 panel blockers against 33 with them, a one-sided non-inferiority p of 0.00074 at a margin of 4. The Red Team still runs on a small diff raised to standard, on every post-fix full pass and on the scoped pass. A first pass that logs a CRITICAL gets it only if a post-fix full or scoped pass follows, because `/ship` passes `--critical-found` only to the repeat plan. A new `/ship` after a stop starts counting passes again, so its first full pass has none either. ### Changed -- **`lib/review-depth.ts`**: on a first full pass (`fullPasses` 0), the gate-class term (a depth error included, since it runs as gate class) and the more-than-`RED_TEAM_LINES` term no longer add the Red Team. The plan prints `SKIP: red-team (first full pass; Round 7 no-loss)`. The CRITICAL and raised small-diff terms are unchanged, and post-fix full passes keep every term. The scoped pass, the fix-check and light depth's batched red-team checklist are unchanged. +- **`lib/review-depth.ts`**: on a first full pass (`fullPasses` 0 and no earlier step, so a repeat plan that leaves out `--pass` keeps the Red Team), the gate-class term (a depth error included, since it runs as gate class) and the more-than-`RED_TEAM_LINES` term no longer add the Red Team. The plan prints `SKIP: red-team (first full pass; Round 7 no-loss)`. The CRITICAL and raised small-diff terms are unchanged, and post-fix full passes keep every term. The scoped pass, the fix-check and light depth's batched red-team checklist are unchanged. - **`RED_TEAM_FIRST_PASS`** (false) joins the `DEPTH_RULES` hash, so the hash moves. ### Docs diff --git a/docs/review-policy.md b/docs/review-policy.md index 12ee9cb8b..acd7efffb 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -188,8 +188,8 @@ in a data file still schedules the outside voice. |---|---|---| | **light**, runtime 0 | docs and tests only, or only registered outputs | the core checklist only; the outside voice when the diff publishes numbers | | **light**, runtime 1–50 | `SMALL_DIFF_LINES` or fewer runtime lines | core, one **batched** specialist (the batched checklists below plus the Red Team prompt in one context), the native adversarial pass (kept: backtest inconclusive, GSTA-147 D5) and the outside voice on numbers | -| **standard** | more than 50 runtime lines | one batched specialist at `medium`; the Red Team on a CRITICAL or a raised small diff, and past 200 runtime lines only after a fix ([first full pass](#first-pass-red-team)); the batched reader leaves docs and registered outputs out of its diff; adversarial and outside voice read everything | -| **full** | gate class, or any classifier error | one batched specialist on the whole diff, no hit-rate gating; the Red Team on every post-fix full pass and on a CRITICAL, never on the first full pass otherwise ([first full pass](#first-pass-red-team)) | +| **standard** | more than 50 runtime lines | one batched specialist at `medium`; the Red Team on a small diff raised to standard, never past 200 runtime lines on pass 1 ([first full pass](#first-pass-red-team)); the batched reader leaves docs and registered outputs out of its diff; adversarial and outside voice read everything | +| **full** | gate class, or any classifier error | one batched specialist on the whole diff, no hit-rate gating; no Red Team on pass 1 ([first full pass](#first-pass-red-team)) | **Batched checklists (the lean ensemble, GSTA-167 part 2).** One specialist subagent reads Testing and Security on every code or gate diff, plus @@ -209,7 +209,8 @@ is the same at every depth. **The first full pass has no standalone Red Team (GSTA-167 Round 7).** When an -invocation's first full pass (`fullPasses` 0) is gate class or has more than +invocation's first full pass (`fullPasses` 0 and no earlier step, so a repeat +plan that leaves out `--pass` still counts as post-fix) is gate class or has more than `RED_TEAM_LINES` (200) runtime lines, the plan skips the Red Team and says why: `SKIP: red-team (first full pass; Round 7 no-loss)`. A full plan reached through a classifier error (`DEPTH_ERROR`) or `--depth-floor full` runs as gate class, so the same rule covers it; @@ -217,7 +218,9 @@ Round 7 unit 3540 (an unmatched `gstack-diff-scope` path) is that case. The Red still runs on a small diff raised to standard, on every post-fix full pass and on the scoped pass, and the plan adds it whenever `--critical-found` is given. In `/ship` that flag reaches only the repeat plan, so a first pass that logs a -CRITICAL gets the Red Team on its post-fix pass, not on pass 1. `fullPasses` +CRITICAL gets the Red Team only if a post-fix full or scoped pass follows. When +that CRITICAL is deferred or noted (`done`), fixed below the floor (a fix-check) +or left open (a stop), no Red Team reads the change. `fullPasses` counts passes in one `/ship` invocation, so a new `/ship` after a stop (a fix made by hand between runs) starts at 0 and its first full pass has no Red Team either. A fix-check never had one. `RED_TEAM_FIRST_PASS` (false) records the rule in diff --git a/lib/review-depth.ts b/lib/review-depth.ts index 8a11ed5d0..21ed1f521 100644 --- a/lib/review-depth.ts +++ b/lib/review-depth.ts @@ -28,9 +28,9 @@ export const SMALL_DIFF_LINES = 50; export const RED_TEAM_LINES = 200; export const LARGE_DIFF_LINES = 100; /** - * Whether a change's first full pass dispatches the standalone Red Team for gate class (a depth error included) or more than - * RED_TEAM_LINES runtime lines. Off since the Round 7 no-loss replay (handoffs/GSTA-167/noloss7-results.md); a CRITICAL, a - * raised small diff, post-fix full passes and the scoped pass keep it. + * Whether an invocation's first full pass (no earlier step) dispatches the standalone Red Team for gate class (a depth error + * included) or more than RED_TEAM_LINES runtime lines. Off since the Round 7 no-loss replay (handoffs/GSTA-167/noloss7-results.md); + * criticalFound, a raised small diff, post-fix full passes and the scoped pass keep it. */ export const RED_TEAM_FIRST_PASS = false; /** The largest fix range, in runtime lines, a pass 2 after a blocking fix reviews as a scoped pass (GSTA-152 D6); a starting value. */ @@ -413,7 +413,7 @@ export function planReview(i: PlanInput): ReviewPlan { if (checklists.length) add('specialist', 'batched', specialist, full ? [] : docsAndOutputs, { checklists }); const codeLike = dcls === 'code' || dcls === 'gate'; const sized = dcls === 'gate' || (dcls === 'code' && lines > RED_TEAM_LINES); - const firstPassCut = sized && h.fullPasses === 0 && !RED_TEAM_FIRST_PASS; + const firstPassCut = sized && h.fullPasses === 0 && h.last === 'none' && !RED_TEAM_FIRST_PASS; if (codeLike && (!!i.criticalFound || raised || (sized && !firstPassCut))) add('red-team', 'red-team', 'full'); else if (firstPassCut) skipped.push('red-team (first full pass; Round 7 no-loss)'); else skipped.push(`red-team (${dcls} class${dcls === 'code' ? `, ${lines} runtime lines, no critical` : ''})`); diff --git a/test/review-depth.test.ts b/test/review-depth.test.ts index 34b989bf3..8d4712029 100644 --- a/test/review-depth.test.ts +++ b/test/review-depth.test.ts @@ -552,6 +552,10 @@ describe('the first full pass drops the standalone Red Team (GSTA-167 Round 7 no const touched = planReview(base({ diffLines: 20, gateTagged: 1, history: { fullPasses: 1, last: 'full', fixedBelow: true, touchedGate: true } })); expect([touched.next.step, rt(touched)]).toEqual(['full', true]); }); + test('a repeat plan that leaves out the pass count still keeps the Red Team (the step history decides pass 1)', () => { + const p = planReview(base({ diffLines: 20, gateTagged: 1, history: { fullPasses: 0, last: 'full', fixedBlocking: true } })); + expect([p.next.step, rt(p)]).toEqual(['full', true]); + }); test('a scoped pass keeps the Red Team; a fix-check never had one', () => { const scoped = planReview(base({ diffLines: 201, history: { ...postFix, rangeRuntimeLines: 24, pass1Depth: 'standard' }, adversarialRan: true })); expect([scoped.next.step, rt(scoped)]).toEqual(['scoped', true]); diff --git a/test/review-packets-removed.test.ts b/test/review-packets-removed.test.ts index 24b78749a..3039100fb 100644 --- a/test/review-packets-removed.test.ts +++ b/test/review-packets-removed.test.ts @@ -78,7 +78,7 @@ const REVIEWER_BRIEFS: Record = { 'review/specialists/testing.md': { sha: 'd6cc2bef7be562387d0c0a22fefe646dff9c295f1d34d8453a6cb496584150e4', replay: '#3638' }, 'review/specialists/targeted.md': { sha: '7f92a0d0a8319c3c0e076fb3b1a4689c45120b2be2c14dfc7d4cb03aafa9d6a5', replay: 'GST-257' }, 'scripts/resolvers/fix-check.ts': { sha: '712bf5b0c439e96bac32f19cbe25394dabb4c94aa66600100bfde9b100e15a08', replay: '#3638' }, - 'lib/review-depth.ts': { sha: '807309b28c604951f0f2584d5d23f0fe54652fcdc87ad981094888c9df0a5c9a', replay: 'V7' }, + 'lib/review-depth.ts': { sha: 'a64130f2f2fd13416994dde01f4ed53349013f7134c41f87d43f59d73c448d0a', replay: 'V7' }, }; describe('reviewer briefs stay at their no-loss-tested bytes (GSTA-167)', () => {