fix(tooling): marker guard false-positives on changes deep inside a pre-existing block

script/upstream/analyze.test.ts

@@ -1,5 +1,5 @@
 import { describe, test, expect } from "bun:test"
-import { parseDiffForMarkerWarnings } from "./analyze"
+import { parseDiffForMarkerWarnings, computeMarkedLines } from "./analyze"
 
 // Helper to create a unified diff string from lines
 function makeDiff(hunks: string): string {
@@ -300,3 +300,93 @@ describe("parseDiffForMarkerWarnings", () => {
     expect(warnings[0].context).toContain("UpgradeIndicator")
   })
 })
+
+describe("computeMarkedLines", () => {
+  test("covers the marker lines and everything between", () => {
+    const content = ["const a = 1", "// altimate_change start — x", "const b = 2", "// altimate_change end", "const c = 3"].join("\n")
+    const marked = computeMarkedLines(content)
+    expect(marked.has(1)).toBe(false) // before block
+    expect(marked.has(2)).toBe(true) // start marker
+    expect(marked.has(3)).toBe(true) // inside
+    expect(marked.has(4)).toBe(true) // end marker
+    expect(marked.has(5)).toBe(false) // after block
+  })
+
+  test("handles nested start/end pairs with a depth counter", () => {
+    const content = [
+      "const a = 1", // 1
+      "// altimate_change start — outer", // 2
+      "const b = 2", // 3
+      "// altimate_change start — inner", // 4
+      "const c = 3", // 5
+      "// altimate_change end", // 6  (closes inner; still inside outer)
+      "const d = 4", // 7  still inside outer
+      "// altimate_change end", // 8  (closes outer)
+      "const e = 5", // 9  outside
+    ].join("\n")
+    const marked = computeMarkedLines(content)
+    expect(marked.has(1)).toBe(false)
+    expect(marked.has(3)).toBe(true)
+    expect(marked.has(5)).toBe(true)
+    expect(marked.has(7)).toBe(true) // still inside outer after inner closed — the bug a single openBlock would miss
+    expect(marked.has(8)).toBe(true) // outer end marker
+    expect(marked.has(9)).toBe(false)
+  })
+
+  test("unbalanced extra end does not drive depth negative", () => {
+    const content = ["// altimate_change end", "const a = 1", "// altimate_change start — x", "const b = 2", "// altimate_change end"].join("\n")
+    const marked = computeMarkedLines(content)
+    expect(marked.has(2)).toBe(false) // not inside any block
+    expect(marked.has(4)).toBe(true) // inside the real block
+  })
+})
+
+describe("parseDiffForMarkerWarnings + full-file coverage (context-window false positive)", () => {
+  // Reproduces the worker.ts:183 CI failure: a line is MODIFIED deep inside a
+  // pre-existing `altimate_change` block, but the block's `start` marker is
+  // further than the diff's ±context window, so it never appears in the hunk.
+  const diffModifyingInsideBlock = makeDiff(
+    `@@ -180,7 +180,7 @@
+ const ctxA = 1
+ const ctxB = 2
+ const ctxC = 3
+-const trace = oldTrace()
++const trace = newTrace()
+ const ctxD = 4
+ const ctxE = 5
+ const ctxF = 6`,
+  )
+
+  test("WITHOUT coverage map: false-positives (documents the bug)", () => {
+    const warnings = parseDiffForMarkerWarnings("worker.ts", diffModifyingInsideBlock)
+    expect(warnings).toHaveLength(1)
+    expect(warnings[0].context).toContain("trace = newTrace()")
+  })
+
+  test("WITH coverage map saying line 183 is inside a marked block: no warning", () => {
+    // Pretend the full file has a marked block spanning lines 160-210; the
+    // changed line (183) is covered even though no marker appears in the hunk.
+    const marked = new Set<number>()
+    for (let i = 160; i <= 210; i++) marked.add(i)
+    const warnings = parseDiffForMarkerWarnings("worker.ts", diffModifyingInsideBlock, marked)
+    expect(warnings).toEqual([])
+  })
+
+  test("WITH coverage map NOT covering the line: still warns (no over-suppression)", () => {
+    const marked = new Set<number>([1, 2, 3]) // unrelated lines
+    const warnings = parseDiffForMarkerWarnings("worker.ts", diffModifyingInsideBlock, marked)
+    expect(warnings).toHaveLength(1)
+  })
+
+  test("end-to-end: computeMarkedLines feeds the parser for a real reconstructed file", () => {
+    // Build a file where the change at line 183 sits inside a 160-210 block.
+    const fileLines: string[] = []
+    for (let i = 1; i <= 159; i++) fileLines.push(`const before${i} = ${i}`)
+    fileLines.push("// altimate_change start — big block") // 160
+    for (let i = 161; i <= 209; i++) fileLines.push(`const inside${i} = ${i}`)
+    fileLines.push("// altimate_change end") // 210
+    const marked = computeMarkedLines(fileLines.join("\n"))
+    const warnings = parseDiffForMarkerWarnings("worker.ts", diffModifyingInsideBlock, marked)
+    expect(warnings).toEqual([])
+  })
+})

script/upstream/analyze.ts

@@ -733,7 +733,45 @@ function isUpstreamShared(file: string, config: MergeConfig): boolean {
 }
 
 // altimate_change start — exported for unit testing
-export function parseDiffForMarkerWarnings(file: string, diffOutput: string): MarkerWarning[] {
+/**
+ * Compute the set of 1-based line numbers in `content` that are covered by an
+ * `altimate_change start … altimate_change end` block (the marker lines
+ * themselves are covered too). A depth counter handles nested blocks, so an
+ * inner start/end pair inside an outer one is tracked correctly.
+ *
+ * This is derived from the FULL file, independent of any diff context window —
+ * which is the whole point. The diff-based tracker in
+ * `parseDiffForMarkerWarnings` only sees ±N context lines around a change, so a
+ * line modified deep inside a large pre-existing marked block (its `start`
+ * marker outside the hunk) looks unmarked and false-positives. Passing the
+ * full-file coverage in lets the parser suppress those false positives.
+ */
+export function computeMarkedLines(content: string): Set<number> {
+  const marked = new Set<number>()
+  const lines = content.split("\n")
+  let depth = 0
+  for (let i = 0; i < lines.length; i++) {
+    const line = lines[i]
+    if (line.includes("altimate_change start")) {
+      depth++
+      marked.add(i + 1)
+      continue
+    }
+    if (line.includes("altimate_change end")) {
+      marked.add(i + 1)
+      depth = Math.max(0, depth - 1)
+      continue
+    }
+    if (depth > 0) marked.add(i + 1)
+  }
+  return marked
+}
+
+export function parseDiffForMarkerWarnings(
+  file: string,
+  diffOutput: string,
+  markedLines?: Set<number>,
+): MarkerWarning[] {
   const warnings: MarkerWarning[] = []
   if (!diffOutput.trim()) return warnings
 
@@ -790,7 +828,12 @@ export function parseDiffForMarkerWarnings(file: string, diffOutput: string): Ma
       if (content.startsWith("import ")) continue
       if (content.startsWith("export ")) continue
 
-      if (!inMarkerBlock) {
+      // A line is covered if either the in-hunk tracker saw its enclosing
+      // `start` marker, OR the full-file coverage map (when provided) says this
+      // line sits inside a marked block. The full-file map is what rescues
+      // changes whose `start` marker lives outside the diff's context window.
+      const coveredByFullFile = markedLines?.has(currentLine) ?? false
+      if (!inMarkerBlock && !coveredByFullFile) {
         if (!hasNewCode) {
           hasNewCode = true
           newCodeStart = currentLine
@@ -827,7 +870,22 @@ function checkFileForMarkers(file: string, base?: string): MarkerWarning[] {
     return []
   }
 
-  return parseDiffForMarkerWarnings(file, diffOutput)
+  // Coverage from the FULL post-change file, so a change deep inside a
+  // pre-existing marked block isn't false-flagged just because its `start`
+  // marker fell outside the diff's ±5-line context window. The "new" side is
+  // HEAD when diffing against a base, or the working tree otherwise — matching
+  // the side the `+`/context line numbers in the diff refer to.
+  let markedLines: Set<number> | undefined
+  try {
+    const newContent = base
+      ? execSync(`git show HEAD:"${file}"`, { cwd: root, encoding: "utf-8" })
+      : require("fs").readFileSync(require("path").join(root, file), "utf-8")
+    markedLines = computeMarkedLines(newContent)
+  } catch {
+    markedLines = undefined
+  }
+
+  return parseDiffForMarkerWarnings(file, diffOutput, markedLines)
 }
 
 function runMarkerCheck(config: MergeConfig, base?: string, strict?: boolean): number {