diff --git a/test/helpers/e2e-helpers.test.ts b/test/helpers/e2e-helpers.test.ts index a5d252daf..723e835bb 100644 --- a/test/helpers/e2e-helpers.test.ts +++ b/test/helpers/e2e-helpers.test.ts @@ -107,6 +107,30 @@ describe('matchesUnnegated', () => { )).toBe(true); }); + test('does not treat a bare "no" idiom as a negation ("no blocker", "no downside")', () => { + // "No blocker here: create a separate tier model" and "there's no + // downside to promoting the payload" are positive-framing idioms in + // review prose, not negations of the recommendation that follows. + expect(matchesUnnegated( + 'No blocker here: create a separate tier model.', + /create[^.]{0,60}separate[^.]{0,20}tier[^.]{0,20}model/i, + )).toBe(true); + expect(matchesUnnegated( + 'There is no downside to promoting the payload into explicit columns.', + /promoting[^.]{0,60}payload[^.]{0,60}column/i, + )).toBe(true); + }); + + test('treats a newline + markdown bullet as a clause boundary', () => { + // "I would not keep this inline.\n- Separate tier model" is common LLM + // formatting — without treating the bullet as a boundary, the "not" from + // the prose sentence above would leak into the bullet line below it. + expect(matchesUnnegated( + 'I would not keep this inline.\n- Separate subscription tier model', + /separate[^.]{0,20}subscription[^.]{0,20}tier[^.]{0,20}model/i, + )).toBe(true); + }); + test('catches a positive match even when an unrelated negation appears earlier in the text', () => { expect(matchesUnnegated( 'This is not a JSONField concern. Separately, I recommend you promote the payload into explicit columns.', diff --git a/test/helpers/e2e-helpers.ts b/test/helpers/e2e-helpers.ts index c675d0319..0c10bfcb1 100644 --- a/test/helpers/e2e-helpers.ts +++ b/test/helpers/e2e-helpers.ts @@ -187,22 +187,33 @@ export function matchesUnnegated(text: string, pattern: RegExp): boolean { // curly one (’, U+2019) — LLM prose commonly renders contractions with the // curly form, and this codebase already hit this exact gotcha once before // (see the apostrophe wildcard note in test/spec-template-invariants.test.ts). - const NEGATION = /\b(not|no|never|cannot|without|against|rather than|instead of)\b|[a-z]+n['’]t\b/i; + // Deliberately does NOT include a bare "no" — "no blocker here: create a + // separate model" and "no downside to promoting the payload" are common + // POSITIVE-framing idioms in review prose, not negations of what follows; + // "not"/"never"/"cannot"/the "-n't" family are far more reliable signals. + const NEGATION = /\b(not|never|cannot|without|against|rather than|instead of)\b|[a-z]+n['’]t\b/i; const flags = pattern.flags.includes('g') ? pattern.flags : pattern.flags + 'g'; const re = new RegExp(pattern.source, flags); // Real sentence-ending punctuation is followed by whitespace + a capital // letter, or sits at the end of the string — this excludes periods inside // abbreviations ("e.g.", "i.e.", "etc."), decimals, and version strings, // which a bare lastIndexOf('.') would wrongly treat as a sentence break - // and use to hide an earlier negation word from the lookback window. - const SENTENCE_END = /[.!?](?=\s+[A-Z]|\s*$)/g; + // and use to hide an earlier negation word from the lookback window. A + // newline followed by a markdown bullet/numbered-list marker is ALSO a + // clause boundary — "I would not keep this inline.\n- Separate tier model" + // is common LLM formatting, and without this the "not" from the prose + // sentence above would otherwise leak into the bullet below it. + const SENTENCE_END = /[.!?](?=\s+[A-Z]|\s*$)|\n\s*(?:[-*+]|\d+[.)])\s/g; let m: RegExpExecArray | null; while ((m = re.exec(text))) { let sentenceStart = 0; let em: RegExpExecArray | null; SENTENCE_END.lastIndex = 0; while ((em = SENTENCE_END.exec(text)) && em.index < m.index) { - sentenceStart = em.index + 1; + // em[0].length, not a bare +1 — the bullet-marker alternative above is + // multiple characters wide ("\n- ", "\n1. "), so a fixed +1 would leave + // part of the marker inside the scanned "preceding" window. + sentenceStart = em.index + em[0].length; } const preceding = text.slice(sentenceStart, m.index); if (!NEGATION.test(preceding)) return true; diff --git a/test/skill-e2e-plan.test.ts b/test/skill-e2e-plan.test.ts index 76dfe110b..1883b1109 100644 --- a/test/skill-e2e-plan.test.ts +++ b/test/skill-e2e-plan.test.ts @@ -762,7 +762,7 @@ when the verification email link is clicked. const acceptsInlineAddition = matchesUnnegated(review, /(keep|stay|remain|fine|appropriate|correct|reasonable|no need)[^.]{0,60}(inline|as[- ]is|single column|one column|single field)/i) || matchesUnnegated(review, /(single|one)\s+(trivial\s+)?field[^.]{0,60}(no|without)[^.]{0,40}(independent|separate)/i) || - /doesn'?t (need|require|warrant)[^.]{0,40}(a\s+)?(separate|new|dedicated)[^.]{0,40}(model|table)/i.test(review); + /doesn['’]?t (need|require|warrant)[^.]{0,40}(a\s+)?(separate|new|dedicated)[^.]{0,40}(model|table)/i.test(review); recordE2E(evalCollector, '/plan-eng-review-data-model-minimal-change', 'Plan Eng Review Data-Model Minimal Change E2E', result, { passed: