diff --git a/changelog.d/gsta-167-nonreview.md b/changelog.d/gsta-167-nonreview.md index 3ad92da1e..a5a479858 100644 --- a/changelog.d/gsta-167-nonreview.md +++ b/changelog.d/gsta-167-nonreview.md @@ -3,6 +3,6 @@ ### Fixed - **`gstack-review-dispatch collect` needs the clean marker a reader's own brief asks for before it counts a reply with no findings.** That is `NO FINDINGS` alone on a line for the specialists, the batched specialist, the targeted reader, the Red Team and fix-check; the `Recommendation:` line for the outside voice; and either one for the native adversarial voice. A reply with no finding and no marker (a refusal, an empty reply, a reader that never got the diff) is recorded with `review: false`, prints `COLLECT_NOT_REVIEWED` and exits 1. Re-sending the brief and collecting a real review voids it, and the outside voice, whose failure never blocks, is collected with `--unavailable ''` and verifies as `UNAVAILABLE`, never as a review. A failure text that still holds findings is refused there and collected normally, so nothing is lost. - **`gstack-review-dispatch verify` reports that reader's brief `NOT_COLLECTED` and its output `NOT_REVIEWED`, so VERIFY reads dirty.** When the output file is still there and unchanged, verify checks the marker in it again, so a ledger written before this change is judged by the same rule. The GSTA-189 replay's combined ledger still verifies clean, with five `NO FINDINGS` specialists and one outside voice ending in `Recommendation:`. -- **A severity word used as a word is no longer a finding.** A fix-check reader on gstack-evals#94 closed with "Neither finding is CRITICAL.", `collect` stored that line as a CRITICAL finding, and `judge` would not let the lead drop it. Now a label that is negated ("not CRITICAL", "no High findings", "Neither finding is CRITICAL"), used as a yardstick ("below CRITICAL"), quoted or in code formatting, or listed as vocabulary ("Critical|High") is skipped, and so is a closing "Overall:" or "Summary:" line after the findings unless it says `Blocks: yes`, names a file and line, or carries a label stronger than every finding before it. A label the line opens with, a `Severity:` field, a table cell, a parenthesized classification like "(`FIXABLE`)", and a label whose negation describes the defect ("Without auth this is CRITICAL", "None of the inputs are sanitized so this is CRITICAL") still count. +- **A severity word used as a word is no longer a finding.** A fix-check reader on gstack-evals#94 closed with "Neither finding is CRITICAL.", `collect` stored that line as a CRITICAL finding, and `judge` would not let the lead drop it. Now a label that is negated ("not CRITICAL", "no High findings", "Neither finding is CRITICAL"), used as a yardstick ("below CRITICAL"), quoted or code-formatted in the middle of a sentence, or listed as vocabulary ("Critical|High") is skipped, and so is a closing "Overall:" or "Summary:" line after the findings unless it says `Blocks: yes`, names a file and line, or carries a label stronger than every finding before it. A label the line opens with, a `Severity:` field, a table cell, a quoted label at the end of its line like "(`FIXABLE`)" or "— `CRITICAL`", any label on a line naming a file and line that only a negated subject would cancel, and a label whose negation describes the defect ("Without auth this is CRITICAL", "None of the inputs are sanitized so this is CRITICAL") still count. - **A paragraph finding keeps its detail bullets.** The GSTA-196 native adversarial voice wrote four findings, each with unlabeled bullets and then a `Severity:` line. Every bullet closed the open finding, so each `Severity:` line opened a new one: 4 findings collected as 8. They now collect as 4, each at the stronger of its two labels. Only an unnumbered bullet with no file and line of its own counts as detail; a numbered item is still its own finding. - **No blocker is lost.** Main's parser and this one were run over every recorded reader output: 25 raw outputs from the GSTA-189 and GSTA-196 replays and 2,172 finding records from arms C6, L6, A3668 (both suites), G1 and L. The 8 GSTA-196 lines merge into 4 findings, none at a weaker label. One INVESTIGATE finding, recorded in both arm G1 and arm L, loses a phantom CRITICAL ("no CRITICAL" inside its sentence), and another loses a phantom High ("resolves below High"). None of the 260 records matched to a judged blocker changes. diff --git a/docs/review-policy.md b/docs/review-policy.md index bb436feb3..1ddbeb07e 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -477,10 +477,11 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends: by the word right before it ("not CRITICAL", "not **CRITICAL**", "no High findings"), said of a negated subject in the same clause ("Neither finding is CRITICAL", "None of the read and write paths are High"; "so", "but", "because" and the like, or an "and"/"or" that opens a clause with its own subject, end the subject, so "None of the inputs are sanitized so this is CRITICAL" keeps its label), a - yardstick ("below CRITICAL", "worse than High"; "up to" and "at most" are not yardsticks), in quotes or code formatting ("maps - `FIXABLE` to `CRITICAL`"), or listed as vocabulary ("Critical|High"). A negation that describes the defect ("Without auth this is - CRITICAL") leaves the label standing; a label the line opens with, a parenthesized classification ("(`FIXABLE`)") and a label on a - `Severity:` or `Blocks:` field line always count, and table cells are never vocabulary for each other. A closing "Overall:", "Summary:", + yardstick ("below CRITICAL", "worse than High"; "up to" and "at most" are not yardsticks), quoted or code-formatted with words after it + ("maps `FIXABLE` to `CRITICAL` in a comment"), or listed as vocabulary ("Critical|High"). A negation that describes the defect + ("Without auth this is CRITICAL") leaves the label standing, and a negated subject never cancels a label on a line naming a file:line; + a label the line opens with, a quoted label at the end of its line ("— `CRITICAL`", "(`FIXABLE`)"), a label on a `Severity:` or + `Blocks:` field line and a table cell always count. A closing "Overall:", "Summary:", "In summary" or "Verdict:" line after the findings starts none unless it carries `Blocks: yes`, a file:line, or a label stronger than every finding before it (GST-435). An unnumbered, unlabeled bullet with no file:line under a paragraph finding whose Severity or Blocks line has not come yet is that finding's detail, so the field line after it lands on the finding; a numbered item is its own finding. diff --git a/lib/review-ledger.ts b/lib/review-ledger.ts index e554e4abb..2690c82c1 100644 --- a/lib/review-ledger.ts +++ b/lib/review-ledger.ts @@ -65,18 +65,20 @@ const OPENS = /^(?:[-*+>]\s+|\d+[.)]\s+|#{1,6}\s*)?[\s[(*_`"'“‘|]*$/; * A label word used as a word, not as a label (GST-435): negated by the words right before it ("not CRITICAL", "no High findings", * "isn't a CRITICAL", "doesn't rise to CRITICAL", "no CRITICAL or High"), said of a negated subject through a copula with no * conjunction between ("Neither finding is CRITICAL", "None of these is High", "Nothing here is CRITICAL"), used as a yardstick - * ("below CRITICAL", "worse than High"), in quotation marks or code formatting ("rated it 'High'", "maps `FIXABLE`") or listed as - * vocabulary ("Critical|High"); bold or code marks between the negation and the label change nothing. A negation that describes the - * defect ("Without auth this is CRITICAL", "None of the inputs are sanitized so this is CRITICAL") leaves the label standing; a - * parenthesized classification ("(`FIXABLE`)") and a Severity or Blocks field line keep their quoted labels, and table cells are never - * vocabulary for each other. The clause runs back to the last sentence, colon, comma, dash, bracket or table bar. + * ("below CRITICAL", "worse than High"), quoted or code-formatted inside a sentence, with words after it ("rated it 'High'; the fix + * closes it", "maps `FIXABLE` to `CRITICAL` in a comment") or listed as vocabulary ("Critical|High"); bold or code marks between the + * negation and the label change nothing. A negation that describes the defect ("Without auth this is CRITICAL", "None of the inputs + * are sanitized so this is CRITICAL") leaves the label standing, and a negated subject never cancels a label on a line naming a + * file:line. A quoted label at the end of its line or before only a file:line ("— `CRITICAL`", "(`FIXABLE`)"), on a Severity or Blocks + * field line or in a table row counts, and table cells are never vocabulary for each other. The clause runs back to the last + * sentence, colon, comma, dash, bracket or table bar. */ const NEGATOR = "not|no|never|nothing|without|zero|nor|isn't|aren't|wasn't|weren't|doesn't|don't|didn't|cannot|can't|won't"; const FILLER = '(?:a|an|any|rated|at|be|really|quite|even|rises?\\s+to|reach(?:es)?|counts?\\s+as)'; const MARKS = "[*_`\"'“‘]*"; const NEGATED = new RegExp(`\\b(?:${NEGATOR})\\s+(?:${FILLER}\\s+)*(?:${MARKS}(?:${LABELS})${MARKS}\\s*(?:or|nor|and|/)\\s*)*${MARKS}$`, 'i'); /** A new clause after a negated subject: a subordinating conjunction, or "and"/"or" that opens a clause with its own subject. */ -const CLAUSE_BREAK = '\\b(?:so|but|yet|because|since|while|whereas|although|though|if|when)\\b|\\b(?:and|or)\\s+(?:the|this|that|these|those|it|its|there|a|an|we|i|you|they)\\b'; +const CLAUSE_BREAK = '\\b(?:so|but|yet|because|since|while|whereas|although|though|if|when|which)\\b|\\b(?:and|or)\\s+(?:the|this|that|these|those|it|its|there|a|an|we|i|you|they)\\b'; const NEGATED_SUBJECT = new RegExp(`^\\s*(?:neither|none|nothing|no\\s+(?:[\\w-]+\\s+)?(?:findings?|issues?|items?|defects?|problems?))\\b(?:(?!${CLAUSE_BREAK}).)*\\b(?:is|are|was|were|rises?\\s+to|reach(?:es)?|counts?\\s+as|rated)\\s+(?:(?:a|an)\\s+)?${MARKS}$`, 'i'); const YARDSTICK = new RegExp(`\\b(?:below|under|than|short of|instead of)\\s+${MARKS}$`, 'i'); const QUOTES: Record = { '"': '"', "'": "'", '“': '”', '‘': '’', '`': '`' }; @@ -86,12 +88,13 @@ function aWord(t: string, at: number, len: number): boolean { const before = t.slice(0, at); if (OPENS.test(before)) return false; const after = t.slice(at + len); + const table = t.startsWith('|'); const quote = QUOTES[before.slice(-1)]; - const quoted = quote !== undefined && after.startsWith(quote) && !(before.endsWith('(' + before.slice(-1)) && after.startsWith(quote + ')')); - if (!META.test(t) && quoted) return true; - if (!t.startsWith('|') && (VOCAB_AFTER.test(after) || VOCAB_BEFORE.test(before))) return true; + const mid = quote !== undefined && after.startsWith(quote) && /[A-Za-z]/.test(after.slice(1).replace(new RegExp(LOCUS.source, 'g'), '')); + if (mid && !table && !META.test(t)) return true; + if (!table && (VOCAB_AFTER.test(after) || VOCAB_BEFORE.test(before))) return true; const clause = ' ' + before.slice(Math.max(...['.', ';', ':', '!', '?', ',', '—', '–', '(', ')', '|'].map(c => before.lastIndexOf(c))) + 1); - return NEGATED.test(clause) || NEGATED_SUBJECT.test(clause) || YARDSTICK.test(clause); + return NEGATED.test(clause) || (!LOCUS.test(t) && NEGATED_SUBJECT.test(clause)) || YARDSTICK.test(clause); } /** * A closing summary line ("Overall: …", "In summary, …", "Verdict: …") after the findings restates them: it starts no finding unless diff --git a/test/review-dispatch.test.ts b/test/review-dispatch.test.ts index 74202c66f..b0febcad4 100644 --- a/test/review-dispatch.test.ts +++ b/test/review-dispatch.test.ts @@ -436,7 +436,10 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change ['No bounds check lets CRITICAL data leak (a.ts:3)', 'CRITICAL'], ['|auth.ts:9|token never expires|CRITICAL|', 'CRITICAL'], ['None of the inputs are sanitized so this is CRITICAL (a.ts:3)', 'CRITICAL'], ['Neither path checks auth so this is CRITICAL (a.ts:3)', 'CRITICAL'], ['No defects in parsing but the upload handler is CRITICAL.', 'CRITICAL'], ['No defects in parsing and the upload handler is CRITICAL.', 'CRITICAL'], ['A retry storm could escalate up to CRITICAL.', 'CRITICAL'], - ['|1|FIXABLE|High|race in queue.ts:78|', 'High'], ['**Token leak in log.ts:3**\nSeverity: `Critical`', 'Critical']]; + ['|1|FIXABLE|High|race in queue.ts:78|', 'High'], + ['| a.ts:3 | race | `High` |', 'High'], ['|a.ts:3|leak|`CRITICAL`|', 'CRITICAL'], ['| a.ts:3 | "CRITICAL" |', 'CRITICAL'], + ['- Token leak in log.ts:3 — `CRITICAL`', 'CRITICAL'], ['1. Token leak (log.ts:3): `High`', 'High'], + ['Nothing checks the token which is CRITICAL (a.ts:3)', 'CRITICAL'], ['Neither path checks auth and auth is CRITICAL (a.ts:3)', 'CRITICAL'], ['**Token leak in log.ts:3**\nSeverity: `Critical`', 'Critical']]; for (const [line, label] of kept) expect([line, parseFindings('outside-voice', line).map(f => f.label)]).toEqual([line, [label]]); // Pre-landing review (#3677 Step 11): a summary-led line with a locus or Blocks: yes, or a stronger label than any finding so far, // still collects; a backticked native classification still collects as CRITICAL.