fix: widen matchesUnnegated to sentence boundary, catch contrastive phrasing

Ship-workflow review re-dispatched the testing specialist against the full
updated diff (including the new minimal-change eval) and found 3 more real
bugs in the same family as the earlier fix pass:

- matchesUnnegated's fixed 20-char negation lookback was too narrow: the
  patterns it guards have their own internal gaps up to 80 chars, so "I do
  not think it's worth extracting the payload into columns" (negation >20
  chars before the match) would be misread as an unnegated recommendation.
  Now scans back to the start of the current sentence (last ./!/? before the
  match) instead of a fixed count — matches the same period-bounded
  assumption the calling patterns already make.
- Verified during the fix: neither "not"/"never"/etc. nor the wider window
  catches contrastive phrasing ("add this field to the model RATHER THAN
  create a separate table") — the rejected alternative matches the pattern
  just as strongly as a genuine recommendation, with no negation word
  anywhere nearby. Added "rather than"/"instead of" to the shared negation
  list, benefiting all four callers, not just the new eval.
- The new minimal-change eval's acceptsInlineAddition check was missing the
  matchesUnnegated guard entirely on its first alternative (unlike its
  sibling recommendsSeparateModel a few lines above), and
  recommendsSeparateModel's verb list missed common recommendation phrasings
  (introduce, build, spin out, break out) — deliberately did NOT add "add",
  since "add this new field to the existing model" is exactly how the
  CORRECT answer gets phrased.

Added 3 unit tests reproducing the sentence-boundary and contrastive-phrasing
fixes directly (not just via the paid E2E path).

bun test test/helpers/e2e-helpers.test.ts test/skill-e2e-plan.test.ts
test/skill-validation.test.ts test/parity-suite.test.ts test/touchfiles.test.ts
test/gen-skill-docs.test.ts — 781 pass, 42 skip (paid E2E), 0 fail.

test/helpers/e2e-helpers.test.ts

@@ -44,6 +44,24 @@ describe('matchesUnnegated', () => {
     )).toBe(false);
   });
 
+  test('ignores a match that is the rejected half of a "rather than" contrast', () => {
+    // "Keep it on the existing model RATHER THAN promote the payload into
+    // columns" — the rejected alternative matches the pattern just as
+    // strongly as a real recommendation would, with no single negation
+    // word (not/never/without/...) anywhere near it.
+    expect(matchesUnnegated(
+      'Keep this field on the existing model rather than promote the payload into explicit columns.',
+      splitPattern,
+    )).toBe(false);
+  });
+
+  test('ignores a match that is the rejected half of an "instead of" contrast', () => {
+    expect(matchesUnnegated(
+      'Keep this on the existing model instead of splitting the payload into columns.',
+      splitPattern,
+    )).toBe(false);
+  });
+
   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.',
@@ -67,8 +85,23 @@ describe('matchesUnnegated', () => {
     expect(matchesUnnegated('This review has nothing to do with payloads or columns.', splitPattern)).toBe(false);
   });
 
+  test('ignores a negation far earlier in the same (period-free) sentence', () => {
+    // The negation-lookback scans to the sentence boundary, not a fixed
+    // character count — this is the scenario that motivated that design.
+    // "not" sits well over 20 characters before "extract...payload...columns"
+    // but is still part of the same sentence (no period between them).
+    expect(matchesUnnegated(
+      "I do not think it's worth the added complexity of maintaining a separate table just to extract the payload into columns for these rare cases.",
+      splitPattern,
+    )).toBe(false);
+  });
+
   test('does not infinite-loop on a zero-width-adjacent pattern', () => {
-    // Guards the re.lastIndex++ zero-width-match protection.
+    // Guards the re.lastIndex++ zero-width-match protection. Note: a bare
+    // quantifier pattern like /x*/ always produces an empty match at index 0
+    // with nothing preceding it (so it's trivially "unnegated" and returns
+    // true immediately) — this test's value is that it terminates at all,
+    // not that it exercises the negated branch of the loop.
     const zeroWidthish = /x*/i;
     expect(() => matchesUnnegated('no x here', zeroWidthish)).not.toThrow();
   });

test/helpers/e2e-helpers.ts

@@ -139,15 +139,34 @@ export function setupPlanEngReviewFixture(tmpPrefix: string, planMarkdown: strin
  * Plain substring/regex matching can't tell "extract the payload into
  * columns" from "would NOT extract the payload into columns" — this walks
  * every match and inspects the text just before it for a negation cue.
+ *
+ * The negation lookback scans back to the start of the CURRENT SENTENCE
+ * (the last `.`/`!`/`?` before the match) rather than a fixed character
+ * count. A fixed window (originally 20 chars) is too narrow: the patterns
+ * this is used with have their own internal gaps of up to 80 chars (e.g.
+ * `(verb)[^.]{0,80}payload[^.]{0,80}column`), so a negation word like "not"
+ * can legitimately sit further back in the same sentence than a small fixed
+ * window would see — "I do not think it's worth extracting the payload into
+ * columns" would otherwise be misread as an unnegated recommendation.
+ * Scanning to the sentence boundary matches the same period-bounded
+ * assumption the calling patterns already make.
  */
 export function matchesUnnegated(text: string, pattern: RegExp): boolean {
-  const NEGATION = /\b(not|n't|no|never|don't|doesn't|didn't|shouldn't|wouldn't|isn't|without|against)\b/i;
+  // "rather than"/"instead of" catch contrastive phrasing ("add this field
+  // to the model RATHER THAN create a separate table") — the rejected
+  // alternative can otherwise match the pattern just as strongly as a real
+  // recommendation would, with no single negation word anywhere near it.
+  const NEGATION = /\b(not|n't|no|never|don't|doesn't|didn't|shouldn't|wouldn't|isn't|without|against|rather than|instead of)\b/i;
   const flags = pattern.flags.includes('g') ? pattern.flags : pattern.flags + 'g';
   const re = new RegExp(pattern.source, flags);
   let m: RegExpExecArray | null;
   while ((m = re.exec(text))) {
-    const windowStart = Math.max(0, m.index - 20);
-    const preceding = text.slice(windowStart, m.index);
+    const sentenceStart = Math.max(
+      text.lastIndexOf('.', m.index),
+      text.lastIndexOf('!', m.index),
+      text.lastIndexOf('?', m.index),
+    ) + 1;
+    const preceding = text.slice(sentenceStart, m.index);
     if (!NEGATION.test(preceding)) return true;
     if (re.lastIndex === m.index) re.lastIndex++; // avoid infinite loop on zero-width matches
   }

test/skill-e2e-plan.test.ts

@@ -760,13 +760,16 @@ Focus specifically on the data model design in the plan. Apply the data model re
     const discussesTheField = lower.includes('email_verified_at') || lower.includes('verification');
     // matchesUnnegated so a CORRECT "I would NOT extract a separate model
     // here" answer doesn't trip this check just for containing the words.
+    // Verb list deliberately excludes "add" — "add this new field to the
+    // existing model" is exactly how a CORRECT (inline) recommendation gets
+    // phrased, so including it would false-positive on the desired answer.
     const recommendsSeparateModel = matchesUnnegated(
       review,
-      /(extract|split|promote|create)[^.]{0,60}(separate|new|dedicated)[^.]{0,40}(model|table)/i,
+      /(extract|split|promote|create|introduce|build|spin[- ]?out|break[- ]?out)[^.]{0,60}(separate|new|dedicated)[^.]{0,40}(model|table)/i,
     ) || matchesUnnegated(review, /separate\s+email\s*verification\s*model/i);
     const acceptsInlineAddition =
-      /(keep|stay|remain|fine|appropriate|correct|reasonable|no need)[^.]{0,60}(inline|as[- ]is|single column|one column|single field)/i.test(review) ||
-      /(single|one)\s+(trivial\s+)?field[^.]{0,60}(no|without)[^.]{0,40}(independent|separate)/i.test(review) ||
+      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);
 
     recordE2E(evalCollector, '/plan-eng-review-data-model-minimal-change', 'Plan Eng Review Data-Model Minimal Change E2E', result, {