From d40e14e2ffbbce665a0a7a036b917cc5b43dfe39 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 31 Oct 2026 16:38:23 +0000 Subject: [PATCH] fix(ship): no closing-summary template word sets the negation apart from an earlier blocker (GST-435 rule (b')) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- docs/review-policy.md | 5 +++-- lib/review-ledger.ts | 11 ++++++----- test/review-dispatch.test.ts | 5 ++++- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/docs/review-policy.md b/docs/review-policy.md index 43d8b8364..ae72bd655 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -471,12 +471,13 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends: label-only heading takes the stronger of the two). A label is dropped only from a closing summary, a sentence that as a whole is one of a closed set of templates (GST-435): an optional "I found" or "there's", a negator, an optional review subject (finding, fix, change, here, - else, "in this diff"), the label words and a short tail, as in "I found three INFORMATIONAL + "in this diff"), the label words and a short tail, as in "I found three INFORMATIONAL issues and no CRITICAL ones", "Neither fix is CRITICAL" or "No CRITICAL or High findings to report". A sentence that names a file:line, holds an exception or contrast word (except, besides, but, only), runs over 100 characters or sits in a table row keeps every label, and so does any phrasing the templates leave out ("This is not CRITICAL" stays a phantom for lead - judgment to fold). A kept finding never takes a weaker label than the strongest one left on its line. A `Severity:` or `Blocks:` line + judgment to fold). No template word sets the negation apart from something named earlier, so + "Nothing else is CRITICAL" or "No other issue is CRITICAL" after a blocker keeps its label. A kept finding never takes a weaker label than the strongest one left on its line. A `Severity:` or `Blocks:` line continues the finding above it until a list item, heading, bold paragraph opener or a second line for the same field starts another: `Severity` raises its label, `Blocks: yes` makes it CRITICAL and `Blocks: no` never lowers it; an inline diff --git a/lib/review-ledger.ts b/lib/review-ledger.ts index b72b8e426..046f0b451 100644 --- a/lib/review-ledger.ts +++ b/lib/review-ledger.ts @@ -62,9 +62,10 @@ const ALL_LABELS = new RegExp(ANY_LABEL.source, 'g'); /** * GST-435 rule (b'): a severity word is dropped only from a closing summary, a sentence that as a whole is one of a closed set of * templates: an optional "I found" (or "there's", or "N INFORMATIONAL issues and"), a negator (no, nothing, none, neither, not), an - * optional review subject (finding, fix, change, here, else, "of these", "in this diff"), an optional is/are, the label words and a - * short tail ("findings", "to report"). "I found three INFORMATIONAL issues and no CRITICAL ones", "Neither fix is CRITICAL", "Nothing - * in this diff is CRITICAL". A sentence that names a file:line, holds an exception or contrast word, runs over 100 characters or sits + * optional review subject (finding, fix, change, here, "of these", "in this diff"), an optional is/are, the label words and a + * short tail ("findings", "to report"): "I found three INFORMATIONAL issues and no CRITICAL ones", "Neither fix is CRITICAL", "Nothing + * in this diff is CRITICAL". No template word sets the negation apart from something named earlier (else, other, remaining, left), so + * "Nothing else is CRITICAL" after a blocker keeps its label. A sentence that names a file:line, holds an exception or contrast word, runs over 100 characters or sits * in a table row keeps every label, and so does any phrasing the templates do not list: a phantom costs a fold, a lost one the gate. */ const QUOTE = '[`*"\'\u201c\u201d\u2018\u2019]*'; @@ -73,8 +74,8 @@ const NOUN = '(?:findings?|issues?|defects?|gaps?|nits?|notes?|items?|problems?| const LABEL_LIST = `${QUOTE}(?:${LABELS})${QUOTE}(?:\\s*(?:,|\\bor\\b|\\band\\b)\\s*${QUOTE}(?:${LABELS})${QUOTE})*`; const CLOSING = new RegExp(`^(?:so\\s+)?(?:(?:I|we|(?:the|this)\\s+[\\w-]+(?:\\s+[\\w-]+)?\\s+(?:pass|review))\\s+(?:found|find|see|saw|have|has)\\s+|there(?:['\u2019]s|\\s+is|\\s+are|\\s+was|\\s+were)\\s+)?` + `(?:${NUM}\\s+${QUOTE}(?:${LABELS})${QUOTE}\\s+${NOUN}(?:,|\\s+and)\\s+)?(?(?:no|nothing|none|neither|not)` - + `(?:\\s+(?:of\\s+(?:these|them|which|the(?:\\s+${NUM})?\\s+(?:findings|issues|fixes|changes))|findings?|issues?|items?|problems?|fix(?:es)?|changes?|blockers?|here|else|other|in\\s+this\\s+(?:diff|pr|change)))?` - + `(?:\\s+(?:is|are|was|were|remains?))?(?:\\s+at)?\\s+${LABEL_LIST}(?:\\s+${NOUN})?(?:\\s+(?:found|remaining|left|to\\s+report|in\\s+this\\s+(?:diff|pr|change)))?)` + + `(?:\\s+(?:of\\s+(?:these|them|which|the(?:\\s+${NUM})?\\s+(?:findings|issues|fixes|changes))|findings?|issues?|items?|problems?|fix(?:es)?|changes?|blockers?|here|in\\s+this\\s+(?:diff|pr|change)))?` + + `(?:\\s+(?:is|are|was|were))?(?:\\s+at)?\\s+${LABEL_LIST}(?:\\s+${NOUN})?(?:\\s+(?:found|to\\s+report|in\\s+this\\s+(?:diff|pr|change)))?)` + `(?:,\\s+so\\s+the\\s+(?:fix|fixes|change|changes)\\s+(?:is|are)\\s+sound)?$`, 'di'); const EXCEPTION = /\b(?:except|besides|apart\s+from|other\s+than|but|however|only|though|although|unless)\b/i; const LEAD = /^\s*(?:(?:[-*+]|\d+[.)])\s+)?(?:\**(?:(?:[\w-]+\s+){0,2}(?:pass|review|check)\s+is\s+done|overall|summary|verdict|in\s+short|bottom\s+line):\**\s+)?(?:(?:none|nothing|no|neither)\b[^:;]*?,\s+so\s+)?/i; diff --git a/test/review-dispatch.test.ts b/test/review-dispatch.test.ts index 9bef421a0..7e45e8120 100644 --- a/test/review-dispatch.test.ts +++ b/test/review-dispatch.test.ts @@ -444,7 +444,7 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change expect(judge(readJsonl(merged), readJsonl(judged)).ok).toBe(true); for (const line of ['There are no CRITICAL findings.', 'There are no `CRITICAL` findings.', 'No CRITICAL or High findings to report.', 'I found nothing CRITICAL.', "So there's nothing at CRITICAL.", 'Neither finding is CRITICAL.', 'Neither of the two findings is CRITICAL.', 'Nothing here is CRITICAL.', 'None of these is High.', 'Neither fix is CRITICAL.', - 'Nothing in this diff is CRITICAL.', 'Nothing else is CRITICAL.', 'Overall: no CRITICAL findings.']) + 'Nothing in this diff is CRITICAL.', 'Overall: no CRITICAL findings.']) expect([line, parseFindings('outside-voice', line)]).toEqual([line, []]); for (const reader of ['adversarial-native', 'red-team']) expect(parseFindings(reader, 'Nothing here is FIXABLE.')).toEqual([]); for (const [line, label] of [['This is not CRITICAL.', 'CRITICAL'], ["This doesn't rise to CRITICAL.", 'CRITICAL'], ['Two nits, none of which is High.', 'High'], @@ -471,6 +471,9 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change ['Nothing else is CRITICAL except the token leak in log.ts:3.', 'CRITICAL'], ['No other issue is CRITICAL besides the auth bypass in auth.ts:9.', 'CRITICAL'], ['None of the other findings are CRITICAL, but the SQL injection in db.ts:12 is.', 'CRITICAL'], ['Nothing here is CRITICAL apart from the unescaped query (db.ts:12).', 'CRITICAL'], ['Nothing else is CRITICAL except the token leak.', 'CRITICAL'], + ['Fix the token leak in log.ts:3 first; nothing else is CRITICAL.', 'CRITICAL'], ['The token leak in log.ts:3 blocks the merge. Nothing else is CRITICAL.', 'CRITICAL'], + ['1. Token leak in log.ts:3 must be fixed before landing.\n\nNothing else is CRITICAL.', 'CRITICAL'], ['The token leak blocks the merge.\nNo other issue is CRITICAL.', 'CRITICAL'], + ['The token leak in log.ts:3 blocks the merge. None of the other findings are CRITICAL.', 'CRITICAL'], ['Nothing else is CRITICAL.', 'CRITICAL'], ['No input validation is CRITICAL (upload.ts:4)', 'CRITICAL'], ['No CSRF token is a CRITICAL gap at form.ts:12', 'CRITICAL'], ['Nothing escaped here is CRITICAL: stored XSS at view.ts:8', 'CRITICAL'], ['No timeout here is High (fetch.ts:3)', 'High']]; for (const [line, label] of kept) expect([line, parseFindings('outside-voice', line).map(f => f.label)]).toEqual([line, [label]]);