From 3a15cb0f405d678f68f7ead69faf67c349e60070 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sun, 25 Oct 2026 02:15:40 +0000 Subject: [PATCH] test(eng-first-review): plan finding detectors accept CRITICAL/INFORMATIONAL labels (#3349) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 13 +++++++++++++ test/eng-first-review.test.ts | 16 ++++++++++++++++ test/helpers/eng-cache-writer-decision.ts | 2 +- test/helpers/pty/boundaries.ts | 2 +- 4 files changed, 31 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b0fb7b918..96bd92c78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,18 @@ # Changelog +## [Unreleased] + +**/ship's review army and /plan-eng-review now ask for one severity scale, CRITICAL or INFORMATIONAL, the same scale /review, the specialist JSON and the review checklist already use (#3349).** The shared confidence rubric those two skills render told the model to label findings `[P1]` and `[P2]` and to report a 1-2 confidence finding "only if severity would be P0", while the rest of /ship's Step 9 (Pass 1 CRITICAL, Pass 2 INFORMATIONAL, `"severity":"CRITICAL|INFORMATIONAL"`) used the other scale. A finding from one skill couldn't be compared, deduplicated or gated against a finding from another. Garry picked CRITICAL/INFORMATIONAL for plan-eng-review on 2026-10-24. + +### Fixed +- **`scripts/resolvers/confidence.ts`** (default branch, rendered into `ship/sections/review-army.md` and `plan-eng-review/sections/review-sections.md`): every finding carries a severity, CRITICAL (blocks shipping) or INFORMATIONAL (lower severity, still actioned). The format is `[CRITICAL|INFORMATIONAL] (confidence: N/10) file:line — description` with CRITICAL and INFORMATIONAL examples. A 1-2 score is reported only for a suspected release-blocking catastrophe, at CRITICAL severity and marked explicitly speculative, the rule /review already uses. Repeated sentences in the verification gate and the calibration-learning note were shortened, so ship's skeleton-plus-sections size falls by 77 bytes (313,204 against its 313,281 cap). + +### Tests +- `test/eng-first-review.test.ts`: the two captured plan-eng-review first-review questions (`eng-annotated-cache-au`, `eng-declarative-as`) still open review when their `[P1]` label becomes `[CRITICAL]` or `[INFORMATIONAL]`. They stay closed for `[HIGH]`, `[critical]`, `[Critical]`, `[P4]`, `[CRITICAL|INFORMATIONAL]` and no label. + +### Gate integrity +- One accepted-input change to review: the paid-test detectors `test/helpers/eng-cache-writer-decision.ts` and `test/helpers/pty/boundaries.ts` accept `[CRITICAL]` and `[INFORMATIONAL]` alongside `[P0]`-`[P3]` in a plan finding's annotation, because the skill now asks for those labels. Without it, both captures relabelled `[CRITICAL]` are rejected. Verdict: not a weakening. The label is metadata, and every substantive check (decision, assessment, actors, options) is unchanged. Mutation checks: restoring `P[0-3]` alone fails the positive cases; `[^\]]+`, `P[0-9]` or a case-insensitive CRITICAL each fail the negative controls. No threshold, budget, timeout, trial count or case kind changed. + ## [1.91.191.0] - 2026-10-25 **`gstack-version-bump write --changelog` moves the branch's `[Unreleased]` entry under the new version, and /ship Step 12 stops treating a provisional queue pick as a verified slot.** Stamping by hand meant writing VERSION with `gstack-version-bump write` and then renaming the `## [Unreleased]` heading yourself, because only `gstack-restamp` re-headed the CHANGELOG. Separately, when `gstack-next-version` could not read some open PRs' VERSION it returned `complete:false` and a provisional pick, and Step 12 printed the warning and then wrote that version as if the queue had been checked (left over from #3258, noted in GSTA-45). diff --git a/test/eng-first-review.test.ts b/test/eng-first-review.test.ts index 26d19c8bc..26e74de94 100644 --- a/test/eng-first-review.test.ts +++ b/test/eng-first-review.test.ts @@ -1467,3 +1467,19 @@ describe('Y whole-plan complexity setup decision', () => { }); }); }); + +describe('plan finding severity labels (#3349)', () => { +const relabel = (captured: { call: unknown }, label: string) => { + const c = structuredClone(captured.call) as NativePlanQuestionCall, q = c.questions[0]!, picked = c.answers![q.question]!; + q.question = label ? q.question.replace('[P1] (confidence', `${label} (confidence`) : q.question.replace('[P1] ', ''); + c.answers = { [q.question]: picked }; + return engFirstReviewAUQ(nativePlanCallFingerprint(c, Date.parse(c.answeredAt!), true)); +}; +const captures = [['eng-annotated-cache-au', captured_eng_annotated_cache_au], ['eng-declarative-as', captured_eng_declarative_as]] as const; +test.each(captures)('%s opens review under either severity scale', (_, captured) => { + for (const label of ['[P1]', '[CRITICAL]', '[INFORMATIONAL]']) expect(relabel(captured, label)).toBe(true); +}); +test.each(captures)('%s rejects labels outside both scales', (_, captured) => { + for (const label of ['[HIGH]', '[critical]', '[Critical]', '[P4]', '[CRITICAL|INFORMATIONAL]', '']) expect(relabel(captured, label)).toBe(false); +}); +}); diff --git a/test/helpers/eng-cache-writer-decision.ts b/test/helpers/eng-cache-writer-decision.ts index a39ecbc9d..50686b8d5 100644 --- a/test/helpers/eng-cache-writer-decision.ts +++ b/test/helpers/eng-cache-writer-decision.ts @@ -6,7 +6,7 @@ export function engCacheWriterDecision(q: NativePlanQuestion): boolean { const lines = q.question.split('\n'); // Severity, confidence and source citations annotate an owned issue; they // never replace its current defect, assessment or opposed choices. - const annotated = /^D([1-9]\d*) [—–:-] Issue ([1-9]\d*) \[P[0-3]\] \(confidence (?:10|[1-9])\/10\) [A-Za-z][\w./-]*:[1-9]\d*(?:-[1-9]\d*)?(?: \+ :[1-9]\d*(?:-[1-9]\d*)?)? [—–:-] ([A-Za-z_$][\w$]*) and ([A-Za-z_$][\w$]*) both mutate one module-level ([A-Za-z_$][\w$]*) that does not serialize mutations\. How should the shared cache be wired\?$/.exec(lines[0] ?? ''); + const annotated = /^D([1-9]\d*) [—–:-] Issue ([1-9]\d*) \[(?:P[0-3]|CRITICAL|INFORMATIONAL)\] \(confidence (?:10|[1-9])\/10\) [A-Za-z][\w./-]*:[1-9]\d*(?:-[1-9]\d*)?(?: \+ :[1-9]\d*(?:-[1-9]\d*)?)? [—–:-] ([A-Za-z_$][\w$]*) and ([A-Za-z_$][\w$]*) both mutate one module-level ([A-Za-z_$][\w$]*) that does not serialize mutations\. How should the shared cache be wired\?$/.exec(lines[0] ?? ''); const architecture = /^D([1-9]\d*) [—–:-] Architecture issue ([1-9]\d*): global mutable ([A-Za-z_$][\w$]*) shared by two services with unserialized mutations\.?$/.exec(lines[0] ?? ''); const declared = annotated || architecture; const ordinal = declared?.[1] ?? /^D([1-9]\d*) [—–:-] Who is allowed to write to the auth cache\?$/.exec(lines[0] ?? '')?.[1]; diff --git a/test/helpers/pty/boundaries.ts b/test/helpers/pty/boundaries.ts index 8c87100c9..c5520118d 100644 --- a/test/helpers/pty/boundaries.ts +++ b/test/helpers/pty/boundaries.ts @@ -234,7 +234,7 @@ function engNumberedFindingAUQ(fp: AskUserQuestionFingerprint): boolean { // A declarative severity-labelled issue can own the same cache repair as // an interrogative title. Require its native decision, current assessment, // and opposed implementation options; metadata alone never opens review. - const declaredCache = /^(?:Issue|Finding) ([1-9]\d*) \[P[0-3]\] \(confidence (?:10|[1-9])\/10\) [A-Za-z][\w./-]*:[1-9]\d*(?:-[1-9]\d*)?: ([A-Za-z_$][\w$]*) and ([A-Za-z_$][\w$]*) share a global mutable ([A-Za-z_$][\w$]*) via module-level export, and both mutate it\.$/.exec(title); + const declaredCache = /^(?:Issue|Finding) ([1-9]\d*) \[(?:P[0-3]|CRITICAL|INFORMATIONAL)\] \(confidence (?:10|[1-9])\/10\) [A-Za-z][\w./-]*:[1-9]\d*(?:-[1-9]\d*)?: ([A-Za-z_$][\w$]*) and ([A-Za-z_$][\w$]*) share a global mutable ([A-Za-z_$][\w$]*) via module-level export, and both mutate it\.$/.exec(title); if (declaredCache) return engDeclaredCacheFinding(q, declaredCache); if (engCacheWriterDecision(q)) return true; const libraryHooks = engLibraryHooksFinding(q, title);