diff --git a/docs/review-policy.md b/docs/review-policy.md index c8c71a472..758145cd8 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -714,7 +714,8 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends: lands on the finding; a numbered item is its own finding. A reader's own no-defect closing sentence is no finding at the label it denies (GST-435): a prose line with no list, heading, bold or lead label, no `Severity` or `Blocks` field, no FIXABLE or INVESTIGATE, no file:line or path - and no exception clause, that does not continue a sentence from the line above, and whose every labelled sentence is + and no exception clause, with no line above it in its paragraph that runs on (no closing period) or names an + exception, and whose every labelled sentence is wholly a denial ("I found nothing CRITICAL.", "I found no defect that is CRITICAL.", "No P0/P1 issues.", "Neither finding is CRITICAL."), a count ("I found three INFORMATIONAL issues and nothing CRITICAL" collects at INFORMATIONAL) or "nothing blocks", with labels spelled as collect spells them. Such a line collects nothing when it diff --git a/lib/review-ledger.ts b/lib/review-ledger.ts index 11df41a70..bea5628c0 100644 --- a/lib/review-ledger.ts +++ b/lib/review-ledger.ts @@ -103,7 +103,7 @@ export const summaryCandidate = (t: string) => ANY_LABEL.test(t) && !ANY_LABEL.t && t.split(SENTENCE).filter(s => ANY_LABEL.test(s)).every(s => !PATH_LOCUS.test(s) && !EXCEPTION.test(s)); /** * A reader's own no-defect closing sentence (GST-435: gstack-evals #194 "I found nothing CRITICAL.", #202 "I found no defect that is - * CRITICAL..."): a prose line that does not continue a sentence from the line above, with no list, heading, bold or lead label, no + * CRITICAL..."): a prose line whose paragraph has no line above it that runs on or names an exception, with no list, heading, bold or lead label, no * Severity or Blocks field, no FIXABLE or INVESTIGATE, no file:line or path and no exception clause, whose every labelled sentence is * wholly one of these clauses, its labels spelled as ALL_LABELS spells them: a denial (`no CRITICAL issues`, `no defect that is * CRITICAL`, `nothing at CRITICAL`, `there are no High findings`, `neither finding is CRITICAL`), a count (`I found three @@ -198,7 +198,7 @@ export function parseFindings(reader: string, output: string, fixableRaises = tr if (!t) { gap = true; prev = ''; continue; } if (!treeSeen && isReviewedTreeLine(t)) { treeSeen = true; continue; } const afterGap = gap; - const continued = !HEADING.test(prev) && (/[,;(]$/.test(prev) || (/[\p{L}\p{N}]$/u.test(prev) && /^\p{Ll}/u.test(t))); + const continued = !!prev && !HEADING.test(prev) && (EXCEPTION.test(prev) || !/[.!?…}]["'”’)\]*_`]*$/.test(prev)); prev = t; gap = false; if (/^NO FINDINGS\.?$/.test(t) || /^Recommendation:/i.test(t)) { current = null; continue; } diff --git a/test/review-closing-summary.test.ts b/test/review-closing-summary.test.ts index 5bf8d9bc2..3fc18879c 100644 --- a/test/review-closing-summary.test.ts +++ b/test/review-closing-summary.test.ts @@ -96,6 +96,11 @@ describe("a reader's no-defect closing sentence is not a finding (GST-435)", () expect(labels('I found nothing CRITICAL. Here is what I checked:\n- **Auth:** the token check at auth.ts:9 is CRITICAL and missing.')).toEqual(['INFORMATIONAL', 'CRITICAL']); expect(labels('No FIXABLE findings.', 'adversarial-native')).toEqual(['CRITICAL']); expect(labels('Apart from the SQL injection in db.ts:40, I found\nnothing CRITICAL.')).toEqual(['CRITICAL']); + for (const wrap of ['Apart from the SQL injection in db.ts:40 —\nnothing CRITICAL.\nRecommendation: request changes because the SQL injection exposes the database', + 'Apart from the SQL injection in db.ts:40\nI found nothing CRITICAL.', 'Apart from the SQL injection in `db.ts:40`\nnothing CRITICAL.', 'The guard was removed (see auth)\nno CRITICAL issue.', + 'Apart from the SQL injection in db.ts:40, the rest is fine.\nI found nothing CRITICAL.', '1. auth.ts:3 LOW: log line too verbose\nNo CRITICAL issues remain.']) + expect([wrap, labels(wrap).at(-1)]).toEqual([wrap, 'CRITICAL']); + expect(labels('The rest of the diff is fine.\nI found nothing CRITICAL.')).toEqual([]); expect(labels('The webhook check is gone and I found\nno CRITICAL issues.')).toEqual(['CRITICAL']); expect(labels('I found nothing CRITICAL and two high issues.')).toEqual(['CRITICAL']); expect(labels('## Summary\nI found nothing CRITICAL.')).toEqual([]); diff --git a/test/review-dispatch.test.ts b/test/review-dispatch.test.ts index ac76810ad..3301e3a1a 100644 --- a/test/review-dispatch.test.ts +++ b/test/review-dispatch.test.ts @@ -370,7 +370,7 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change expect(parseFindings('r', 'Nothing CRITICAL / HIGH: race in q.ts:7').map(f => f.label)).toEqual(['CRITICAL']); expect(parseFindings('r', 'No CRITICAL lock guards the write in Makefile:7\nSeverity: CRITICAL').map(f => f.label)).toEqual(['CRITICAL']); expect(parseFindings('r', '1. auth.ts:3 LOW: log line too verbose\nNo CRITICAL lock guards the write in Makefile:7\nSeverity: CRITICAL').map(f => [f.label, f.raw])).toEqual([['LOW', '1. auth.ts:3 LOW: log line too verbose'], ['CRITICAL', 'No CRITICAL lock guards the write in Makefile:7 | Severity: CRITICAL']]); - expect(parseFindings('r', '1. auth.ts:3 LOW: log line too verbose\nNo CRITICAL issues remain.').map(f => [f.label, f.raw])).toEqual([['LOW', '1. auth.ts:3 LOW: log line too verbose']]); + expect(parseFindings('r', '1. auth.ts:3 LOW: log line too verbose\nNo CRITICAL issues remain.').map(f => f.label)).toEqual(['LOW', 'CRITICAL']); expect(parseFindings('outside-voice', '- a.ts:3 race. Severity: Low. Blocks: no\n Separately, nothing CRITICAL about b.ts:9, but it leaks fds.\n Severity: High').map(f => [f.label, f.raw])).toEqual([['Low', '- a.ts:3 race. Severity: Low. Blocks: no'], ['High', 'Separately, nothing CRITICAL about b.ts:9, but it leaks fds. | Severity: High']]); expect(parseFindings('r', '1. A CRITICAL race in queue.ts:78 that no lock covers').map(f => f.label)).toEqual(['CRITICAL']); expect(parseFindings('r', '- A divide by zero CRITICAL bug in parse()').map(f => f.label)).toEqual(['CRITICAL']); diff --git a/test/review-ledger-invariant.test.ts b/test/review-ledger-invariant.test.ts index 82de0b054..8cae63c3f 100644 --- a/test/review-ledger-invariant.test.ts +++ b/test/review-ledger-invariant.test.ts @@ -19,7 +19,7 @@ const LEAD_NO_FIXABLE = new RegExp(LEAD.source.replace('|FIXABLE|INVESTIGATE', ' const NO_DEFECT_SUMMARIES: Record = { 'Nothing CRITICAL or HIGH remains.': [], 'No **CRITICAL** issues found.': [], 'No `CRITICAL` issue.': [], 'Nothing at CRITICAL severity remains.': [], 'Nothing CRITICAL to report.': [], 'No CRITICAL/HIGH findings.': [], 'No P0/P1 issues.': [], 'No CRITICAL, HIGH or MEDIUM issues.': [], - '1. auth.ts:3 LOW: log line too verbose\nNo CRITICAL issues remain.': ['LOW'], 'There are no CRITICAL findings.': [], 'No CRITICAL or High findings to report.': [], + '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.': [], 'Nothing here is CRITICAL.': [], 'Neither fix is CRITICAL.': [], 'Nothing in this diff is CRITICAL.': [], 'review-ledger/geva-77-fix-check.txt': Array(3).fill('INFORMATIONAL'), 'review-ledger/harn15-ship2-p2-red-team.txt': Array(4).fill('INFORMATIONAL'), 'review-ledger/harn15-ship2-p2-fix-check.txt': Array(5).fill('INFORMATIONAL'),