fix(ship-patch): only the branch's own lines count toward the scoped-review section cap (GST-1226)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

changelog.d/gst-1226-scoped-base-lines.md

@@ -0,0 +1,7 @@
+**A base merge next to a reviewed hunk no longer forces a second full review because of the base's own lines.** On founderstar#515, a base merge put 62 lines of another PR, already merged at base, next to the PR's 76 reviewed and unchanged lines in `tests/ui_shots.gd`. The scoped-review section went past its 200-line limit, so `gstack-ship-release finish` took the behavior route, which a review-loop exception can't accept, and asked for Steps 5 to 11.5 again. #3923 hit the same limit when only one trailing context line changed (GST-1226).
+
+### Fixed
+- **The scoped-review packet counts only the branch's own lines toward its 200-line section limit.** The base's change and the base-added lines that appear as context in the current hunks are still shown to the reader but no longer count, in finish and in the receipt carry. A section the branch's own lines push past the limit still takes the full loop. Context-only drift is not carried silently: it still gets the scoped read.
+
+### Tests
+- `test/ship-rereview-triggers.test.ts`: 210 lines another PR merged at base next to the branch's 76 reviewed lines, and a changed trailing context line beside a 221-line base rewrite, both reach the scoped read (both took the behavior route on main). Negative control: a section the branch's own 230 lines push past the limit still takes the behavior route.

docs/troubleshooting.md

@@ -6031,7 +6031,11 @@ hunk). The merge may have spliced the two, as in a JSON entry where the branch
 changed one key and the base the next, so the receipt does not carry silently.
 The refusal names the paths and a packet file with the hunks that differ and
 the base's own change. The same lines moved somewhere else are
-`RECEIPT_CARRY_PRODUCT_CHANGED`, not this code.
+`RECEIPT_CARRY_PRODUCT_CHANGED`, not this code. A packet section over 200
+lines takes the full loop instead, but only the branch's own lines count toward
+that limit: the base's change and the base-added lines that sit as context in
+the current hunks are shown and not counted (GST-1226), so a large base edit
+next to the branch's hunk still gets the scoped read.
 
 **Fix.** Dispatch one fix-check reviewer
 ([review-policy.md](review-policy.md)) on the packet. When it comes back clean,

lib/ship-patch.ts

@@ -130,13 +130,15 @@ export const PACKET_SECTION_CAP = 200;
 /**
  * The scoped-review packet: per context path, the hunks that differ between the two branch patches and the base's own
  * change; per test path, its test hunks. `over` names every path with a section past PACKET_SECTION_CAP: one reviewer
- * cannot be shown all of it, so the caller sends it to the full loop.
+ * cannot be shown all of it, so the caller sends it to the full loop. Only the branch's own lines count toward the cap
+ * (GST-1226): the base's +/- lines, already merged at base, and the base-added lines that sit as context in the current
+ * hunks are shown but not counted.
  */
 export function contextPacket(repoRoot: string, s: PatchSides, paths: readonly string[], tests: readonly string[] = []): { text: string; over: string[] } {
   const over = new Set<string>();
   let current = '';
-  const fence = (label: string, body: string) => {
-    if (body.split('\n').length > PACKET_SECTION_CAP) over.add(current);
+  const fence = (label: string, body: string, counts: (line: string) => boolean = () => true) => {
+    if (body.split('\n').filter(counts).length > PACKET_SECTION_CAP) over.add(current);
     return `${label}\n\`\`\`diff\n${body.trimEnd()}\n\`\`\`\n`;
   };
   const parts = [`# Scoped review: base context moved next to the reviewed hunks\n\nreviewed ${s.reviewed.slice(0, 12)} against ${s.reviewedBase.slice(0, 12)}; current ${s.current.slice(0, 12)} against ${s.currentBase.slice(0, 12)}. The branch's +/- lines are unchanged; read whether the merge kept their meaning next to the base's edit. Report clean, or findings at the blocking floor.\n`];
@@ -145,8 +147,10 @@ export function contextPacket(repoRoot: string, s: PatchSides, paths: readonly s
     current = rel;
     const r = hunkTexts(pathDiff(repoRoot, s.reviewedBase, s.reviewed, rel, 3) ?? '');
     const c = hunkTexts(pathDiff(repoRoot, s.currentBase, s.current, rel, 3) ?? '');
-    parts.push(`## ${rel}\n`, fence('Reviewed hunks', r.filter(h => !c.includes(h)).join('')), fence('Current hunks', c.filter(h => !r.includes(h)).join('')),
-      fence("The base's change", hunkTexts(pathDiff(repoRoot, s.reviewedBase, s.currentBase, rel, 3) ?? '').join('')));
+    const b = hunkTexts(pathDiff(repoRoot, s.reviewedBase, s.currentBase, rel, 3) ?? '').join('');
+    const merged = new Set(b.split('\n').filter(l => l.startsWith('+')).map(l => ` ${l.slice(1)}`));
+    parts.push(`## ${rel}\n`, fence('Reviewed hunks', r.filter(h => !c.includes(h)).join('')), fence('Current hunks', c.filter(h => !r.includes(h)).join(''), l => !merged.has(l)),
+      fence("The base's change", b, l => !/^[+-]/.test(l)));
   }
   return { text: parts.join('\n'), over: [...over] };
 }

test/ship-rereview-triggers.test.ts

@@ -869,3 +869,50 @@ describe('finish after a completed fix-check (GST-1130, GEVA-218)', () => {
     expect(r.status).toBe(3);
   });
 });
+
+// Value: protects=a base merge that puts lines already merged at base next to the branch's reviewed, unchanged hunk reaches
+//   the scoped read even when those base lines push a packet section past PACKET_SECTION_CAP (founderstar#515, #3923); only the
+//   branch's own lines count toward the cap; fails_when=finish forces the behavior route for a section that is over only
+//   because of base lines, or lets a section the branch's own lines push over reach the scoped read; seam=contextPacket's count
+describe('base lines do not count toward the scoped-review cap (GST-1226, founderstar#515, #3923)', () => {
+  const block = (name: string, n: number) => Array.from({ length: n }, (_, i) => `export const ${name}${i + 1} = ${i + 1};`);
+  const join = (rows: string[]) => rows.join('\n') + '\n';
+  const at = (n: number, after: number, add: string[]) => { const rows = lines(n).trimEnd().split('\n'); rows.splice(after, 0, ...add); return rows; };
+  const scoped = (r: { status: number | null; stdout: string }) => [r.status, lineOf(r.stdout, 'ROUTE').replace(/^.* behavior=/, 'behavior='), r.stdout.includes('STOP: base context moved next to the reviewed hunks in src/shots.ts'), r.stdout.includes('SCOPED_REVIEW: too large')];
+
+  test('210 lines another PR merged at base next to the branch\'s 76 reviewed lines reach the scoped read', () => {
+    const pr = block('pr', 76), other = block('other', 210);
+    const f = fx({ 'src/shots.ts': lines(20) }, { 'src/shots.ts': join(at(20, 10, pr)) });
+    const reviewed = wtree(f), reviewedBase = mergeBase(f), post = snapshot(f);
+    f.advanceBase({ 'src/shots.ts': join(at(20, 11, other)) });
+    const merged = at(20, 11, other); merged.splice(10, 0, ...pr);
+    mergeMain(f, { 'src/shots.ts': join(merged) });
+    expect(f.git('show', 'HEAD:src/shots.ts').stdout).toBe(join(merged));
+    expect(scoped(finish(f, reviewed, post, ['--reviewed-base', reviewedBase]))).toEqual([3, 'behavior=0 docs=0 metadata=0 tests=0 base=0 context=1', true, false]);
+  });
+
+  test('a changed trailing context line with the same +/- lines reaches the scoped read beside a 221-line base rewrite', () => {
+    const pr = block('pr', 76);
+    const f = fx({ 'src/shots.ts': lines(300) }, { 'src/shots.ts': join(at(300, 10, pr)) });
+    const reviewed = wtree(f), reviewedBase = mergeBase(f), post = snapshot(f);
+    const edits: Record<number, string> = { 12: 'export const l12 = 120;' };
+    for (let i = 60; i <= 280; i++) edits[i] = `export const l${i} = ${i * 10};`;
+    f.advanceBase({ 'src/shots.ts': lines(300, edits) });
+    const merged = lines(300, edits).trimEnd().split('\n'); merged.splice(10, 0, ...pr);
+    mergeMain(f, { 'src/shots.ts': join(merged) });
+    expect(f.git('show', 'HEAD:src/shots.ts').stdout).toBe(join(merged));
+    expect(scoped(finish(f, reviewed, post, ['--reviewed-base', reviewedBase]))).toEqual([3, 'behavior=0 docs=0 metadata=0 tests=0 base=0 context=1', true, false]);
+  });
+
+  test('a section the branch\'s own 230 lines push past the cap still takes the behavior route', () => {
+    const pr = block('pr', 230), other = block('other', 5);
+    const f = fx({ 'src/shots.ts': lines(20) }, { 'src/shots.ts': join(at(20, 10, pr)) });
+    const reviewed = wtree(f), reviewedBase = mergeBase(f), post = snapshot(f);
+    f.advanceBase({ 'src/shots.ts': join(at(20, 11, other)) });
+    const merged = at(20, 11, other); merged.splice(10, 0, ...pr);
+    mergeMain(f, { 'src/shots.ts': join(merged) });
+    const r = finish(f, reviewed, post, ['--reviewed-base', reviewedBase]);
+    expect(scoped(r)).toEqual([3, 'behavior=1 docs=0 metadata=0 tests=0 base=0 context=0', false, true]);
+    expect(r.stdout.includes('STOP: behavior, tests or build inputs changed since review (src/shots.ts)')).toBe(true);
+  });
+});