test(contracts): guard source-slice windows against silently widening

docs/branch-review-records/642a0ca61b346fbb0b4dd8bda07b9e763f3acdf9d7d26bff9e0017e1cdd58c60.record.md

@@ -0,0 +1 @@
+| 2026-08-15 | claude/test-source-contract-hardening | 22aa7c627e4d3ce220c741d4246d998956249fd2 | Guarded source-slice test windows: shared helper plus three migrations (#1985) | Shipped; closes a silent-pass class in the contract tests, remainder queued | verify:pr-local all 9 selected gates passed — unit suite 6608 passed / 4 skipped; helper's own test pins the raw idiom returning out-of-window content; migrated files re-run green (17/17, 23/23) |

docs/branch-review-records/cd495affb93b35a4c9c27292318bff9bf09cbe3e2cf670a084930b870cd6fed6.record.md

@@ -0,0 +1 @@
+| 2026-08-15 | claude/test-source-contract-hardening | 0c1459ca2985e409d330d290a6518fdf877d3255 | review-and-fix | Fixed two P2 fail-open cases in the new source-window helper: overlapping repeated starts and empty markers; merged latest main | focused Vitest 49/49; format:changed; changed-file ESLint; source typecheck; outstanding-issues and ledger guards |

docs/outstanding-issues-inbox/6e354901-7694-40e0-b03d-8c398cec7b05.json

@@ -0,0 +1,13 @@
+{
+ "version": 1,
+ "id": "6e354901-7694-40e0-b03d-8c398cec7b05",
+ "createdOn": "2026-08-15",
+ "action": "add",
+ "payload": {
+ "pri": "P2",
+ "type": "task",
+ "summary": "Route the remaining ~22 unguarded source-slice test windows through the guarded helper",
+ "detail": "PARTIALLY DONE 2026-08-15 by PR #1985, which added tests/helpers/source-contract.ts and migrated the three worst files. The hazard this closes is a silent pass, not fragility: the idiom source.slice(source.indexOf(start), source.indexOf(end)) returns -1 for a missing end marker, and slice(n, -1) does not throw — it returns the rest of the file bar one character. A renamed end marker therefore converts a scoped assertion into a whole-file assertion and every positive toContain in it keeps passing for the wrong reason. The mirror case, a missing start, yields slice(-1, n) so every negative assertion passes vacuously. Neither shows up as a failure. The helper throws on a missing start, a missing end, and an AMBIGUOUS start (a window anchored on a string that appears twice silently covers only the first hit). MIGRATED: search-route-ownership.test.ts (three windows, including one whose end marker is an indentation depth and one anchored on a comment string), document-detail-performance.test.ts (end marker was the next literal 'useEffect' token, of which DocumentViewer has several), therapy-compass-responsive-contract.test.ts. REMAINING, roughly 22 windows across ~11 files, none migrated: audit-navigation-auth-regressions.test.ts is the densest and was deliberately skipped because PR #1983 edits the same file and the anti-churn rule prefers one late sync to a merge fight — do it once #1983 lands. Also tools-search-directions-mockups.test.ts, in-page-nav-playwright-contract.test.ts and document-section-nav-contract.test.ts (the last two slice ui-smoke.spec.ts between Playwright test titles, so renaming OR reordering an unrelated spec silently rescopes them), and rag-retrieval-parallelism.test.ts, which is left for a session that flags the RAG surface first per AGENTS.md. A REAL COVERAGE HOLE was found while surveying and is NOT yet fixed: audit-navigation-auth-regressions.test.ts around line 285 anchors on '{showUniversalAlsoMatches &&', which occurs TWICE in ClinicalDashboard.tsx (the second around line 3832 sits outside the window), so its not.toContain check does not enforce the named contract across the file. The new helper would reject that anchor outright, which is how it was found. Fix it in the same pass as that file's migration. STOP: do not loosen the demo-data boundary pins in favourites-demo-boundary.test.ts. The exact conditional-spread form '...(demoMode ? prototypeFavouriteItems : [])' with its paired negative is the live-vs-demo privacy contract, and its strictness is the point. Likewise leave the SQL windows ending on '$$;' and header-scroll-hide-contract.test.ts anchoring on the matching '</PhoneHeaderCollapsePortal>' closing tag — those are true structural terminators and are the model the rest should move toward.",
+ "source": "session 2026-08-15; PR #1985; tests/helpers/source-contract.ts"
+ }
+}

tests/document-detail-performance.test.ts

@@ -2,6 +2,8 @@ import { readFileSync } from "node:fs";
 import { resolve } from "node:path";
 import { describe, expect, it } from "vitest";
 
+import { sourceSegment } from "./helpers/source-contract";
+
 function source(path: string) {
  return readFileSync(resolve(process.cwd(), path), "utf8");
 }
@@ -99,8 +101,12 @@ describe("document viewer latency guards", () => {
  const viewer = source("src/components/DocumentViewer.tsx");
  const routeHook = source("src/components/document-viewer/use-document-viewer-route.ts");
  const panelInstances = viewer.match(/<IndexedTextPanel\b/g) ?? [];
- const retryStart = viewer.indexOf("const retryPreview");
- const retryBlock = viewer.slice(retryStart, viewer.indexOf("useEffect", retryStart));
+ // `useEffect` as an end marker is an arbitrary token, not a structure —
+ // DocumentViewer has several. Guarded so a reorder fails loudly instead of
+ // widening this window to the rest of the file.
+ const retryBlock = sourceSegment(viewer, "const retryPreview", "useEffect", {
+ label: "DocumentViewer retryPreview block",
+ });
 
  expect(viewer).toContain('assetScope: "window"');
  expect(viewer).toContain("useInitialResult");

tests/helpers/source-contract.ts

@@ -0,0 +1,129 @@
+/**
+ * Bounded windows for source-text contract tests.
+ *
+ * Several tests in this suite assert on the raw text of a component rather than
+ * its behaviour, scoped to a window between two marker strings. The idiom they
+ * all reached for is:
+ *
+ * source.slice(source.indexOf(start), source.indexOf(end))
+ *
+ * which has a silent failure mode that is worse than the fragility it is
+ * usually blamed for. When `end` no longer matches — someone renamed a symbol,
+ * reflowed a line, changed a class string used as a delimiter — `indexOf`
+ * returns `-1`, and `slice(start, -1)` does not throw. It quietly returns
+ * *everything to the end of the file bar one character*. Every positive
+ * `toContain` in that window then passes for the wrong reason: the assertion
+ * silently stops being scoped and starts matching anywhere in the file.
+ *
+ * The mirror case is just as bad. A missing `start` gives `slice(-1, n)`, an
+ * empty or one-character window, so every `not.toContain` passes vacuously.
+ *
+ * So the danger is not "this test breaks when unrelated code moves" — a broken
+ * test is loud and gets fixed. The danger is a test that keeps passing while
+ * enforcing nothing. This helper converts both cases into an immediate, named
+ * failure.
+ *
+ * It also rejects an *ambiguous* start marker. A window anchored on a string
+ * that occurs more than once silently covers only the first occurrence, so a
+ * negative assertion ("this must not appear here") proves nothing about the
+ * others. That is not hypothetical: `{showUniversalAlsoMatches &&` appears
+ * twice in `ClinicalDashboard.tsx`, and the window anchored on it was checking
+ * one of the two while reading as though it covered the contract.
+ *
+ * Prefer a structural end marker where the language provides one — a matching
+ * closing tag, `$$;` in SQL, a closing brace you can see. `header-scroll-hide-
+ * contract.test.ts` anchoring on `</PhoneHeaderCollapsePortal>` is the model.
+ * An end marker that is merely "whatever is declared next" is adjacency, not
+ * structure, and will widen the day something is inserted between them.
+ */
+
+type SourceSegmentOptions = {
+ /**
+ * Label used in failure messages so a throw names the contract rather than
+ * just the markers. Optional, but a test that scopes a window usually has a
+ * name for what the window IS.
+ */
+ label?: string;
+ /**
+ * Set when the start marker legitimately occurs more than once and the first
+ * occurrence is the intended one. Opt-in, so ambiguity is a decision rather
+ * than an accident.
+ */
+ allowRepeatedStart?: boolean;
+};
+
+function describeMarker(marker: string) {
+ const collapsed = marker.replace(/\s+/g, " ").trim();
+ return collapsed.length > 60 ? `${collapsed.slice(0, 57)}…` : collapsed;
+}
+
+function requireNonEmptyMarker(marker: string, kind: "start" | "end", context: string) {
+ if (marker.length === 0) {
+ throw new Error(`${context}${kind} marker must not be empty`);
+ }
+}
+
+/**
+ * Slice the window between `startMarker` and the first `endMarker` after it.
+ *
+ * Throws — never returns a silently wrong window — when the start is missing,
+ * the end is missing, or the start is ambiguous.
+ */
+export function sourceSegment(
+ contents: string,
+ startMarker: string,
+ endMarker: string,
+ options: SourceSegmentOptions = {},
+): string {
+ const context = options.label ? `${options.label}: ` : "";
+ requireNonEmptyMarker(startMarker, "start", context);
+ requireNonEmptyMarker(endMarker, "end", context);
+ const start = contents.indexOf(startMarker);
+ if (start < 0) {
+ throw new Error(`${context}start marker not found: "${describeMarker(startMarker)}"`);
+ }
+
+ if (!options.allowRepeatedStart) {
+ const second = contents.indexOf(startMarker, start + 1);
+ if (second >= 0) {
+ throw new Error(
+ `${context}start marker "${describeMarker(startMarker)}" occurs more than once, so this window covers only ` +
+ "the first occurrence and proves nothing about the rest of the file. Anchor on something unique, widen the " +
+ "assertion to the whole file, or pass allowRepeatedStart if the first occurrence really is the contract.",
+ );
+ }
+ }
+
+ const end = contents.indexOf(endMarker, start + startMarker.length);
+ if (end < 0) {
+ throw new Error(
+ `${context}end marker not found after the start: "${describeMarker(endMarker)}". Without this guard the window ` +
+ "would have silently become the rest of the file and every assertion in it would pass for the wrong reason.",
+ );
+ }
+
+ return contents.slice(start, end);
+}
+
+/**
+ * Window from `startMarker` to the end of the file. Use only where the contract
+ * genuinely runs to EOF; otherwise give `sourceSegment` a real end marker.
+ */
+export function sourceFrom(contents: string, startMarker: string, options: SourceSegmentOptions = {}): string {
+ const context = options.label ? `${options.label}: ` : "";
+ requireNonEmptyMarker(startMarker, "start", context);
+ const start = contents.indexOf(startMarker);
+ if (start < 0) {
+ throw new Error(`${context}start marker not found: "${describeMarker(startMarker)}"`);
+ }
+ if (!options.allowRepeatedStart) {
+ const second = contents.indexOf(startMarker, start + 1);
+ if (second >= 0) {
+ throw new Error(
+ `${context}start marker "${describeMarker(startMarker)}" occurs more than once; anchor on something unique ` +
+ "or pass allowRepeatedStart.",
+ );
+ }
+ }
+ return contents.slice(start);
+}

tests/search-route-ownership.test.ts

@@ -2,6 +2,8 @@ import { readFileSync } from "node:fs";
 import { resolve } from "node:path";
 import { describe, expect, it } from "vitest";
 
+import { sourceFrom, sourceSegment } from "./helpers/source-contract";
+
 import {
  isAlwaysStandaloneShellPath,
  isDashboardModeHref,
@@ -229,13 +231,18 @@ describe("shared-search route ownership", () => {
 
  it("keeps the mode pill from navigating while the shared home is showing", () => {
  const dashboardSource = readFileSync(resolve(process.cwd(), "src/components/ClinicalDashboard.tsx"), "utf8");
- const selectSearchMode = dashboardSource.slice(dashboardSource.indexOf("function selectSearchMode("));
+ const selectSearchMode = sourceFrom(dashboardSource, "function selectSearchMode(", {
+ label: "selectSearchMode",
+ });
 
  // Isolate the shared-home branch so the negative assertions below cannot be
- // satisfied by unrelated code further down selectSearchMode.
- const branchStart = selectSearchMode.indexOf("if (showSharedHome) {");
- expect(branchStart).toBeGreaterThan(-1);
- const sharedHomeBranch = selectSearchMode.slice(branchStart, selectSearchMode.indexOf("\n }", branchStart));
+ // satisfied by unrelated code further down selectSearchMode. The end marker
+ // is an indentation depth, which is why it goes through the guarded helper:
+ // unguarded, a reformat would turn every negative assertion below into a
+ // vacuous pass rather than a failure.
+ const sharedHomeBranch = sourceSegment(selectSearchMode, "if (showSharedHome) {", "\n }", {
+ label: "selectSearchMode shared-home branch",
+ });
 
  // `/` is the single home page. On it the pill only retargets the composer:
  // rewrite `?mode=` in place, never push a route and never re-render the page.
@@ -255,14 +262,18 @@ describe("shared-search route ownership", () => {
  // Returning home must invalidate the in-flight search before clearing UI —
  // the dashboard stays mounted, so a late applySearchResult would otherwise
  // restore the old answer and rewrite run=1 over the draft home.
- const leaveResultsBranch = selectSearchMode.slice(
- selectSearchMode.indexOf("// Outside the shared home"),
- selectSearchMode.indexOf("function stageAnswerFollowUpDraft"),
+ const leaveResultsBranch = sourceSegment(
+ selectSearchMode,
+ "// Outside the shared home",
+ "function stageAnswerFollowUpDraft",
+ { label: "selectSearchMode leave-results branch" },
  );
  expect(leaveResultsBranch).toMatch(/stopSearch\(\);\s*clearModeResultState\(\);/);
- expect(selectSearchMode.slice(0, selectSearchMode.indexOf("function stageAnswerFollowUpDraft"))).not.toContain(
- "crossModeSearch(mode, carriedQuery)",
- );
+ expect(
+ sourceSegment(selectSearchMode, "function selectSearchMode(", "function stageAnswerFollowUpDraft", {
+ label: "selectSearchMode before stageAnswerFollowUpDraft",
+ }),
+ ).not.toContain("crossModeSearch(mode, carriedQuery)");
  expect(dashboardSource).toContain('if (pathname === "/" && !submittedUrlRunRequested) return;');
  expect(dashboardSource).toContain("if (modeChangeFromUiRef.current && !submittedUrlModeMatchesActive) return;");
  // Ask-this / cross-mode into Answer must not depend solely on auto-run: the

tests/source-contract-helper.test.ts

@@ -0,0 +1,63 @@
+import { describe, expect, it } from "vitest";
+
+import { sourceFrom, sourceSegment } from "./helpers/source-contract";
+
+const file = ["const before = 1;", "function target() {", " return inner;", "}", "const after = 2;"].join("\n");
+
+describe("sourceSegment", () => {
+ it("returns the window between the markers", () => {
+ expect(sourceSegment(file, "function target()", "const after")).toContain("return inner;");
+ expect(sourceSegment(file, "function target()", "const after")).not.toContain("const before");
+ });
+
+ it("throws instead of silently widening to the rest of the file when the end marker is gone", () => {
+ // This is the whole point of the helper. The raw idiom
+ // `slice(indexOf(start), indexOf(missing))` becomes `slice(n, -1)` — the
+ // rest of the file — and every positive assertion in the window then
+ // passes for the wrong reason.
+ expect(() => sourceSegment(file, "function target()", "const renamed")).toThrow(/end marker not found/);
+
+ // Demonstrating the raw idiom's behaviour so the hazard is pinned, not
+ // just described: `slice(n, -1)` reaches the end of the file bar the final
+ // character, so content far outside the intended window is now in scope.
+ const silentlyWrong = file.slice(file.indexOf("function target()"), file.indexOf("const renamed"));
+ expect(silentlyWrong).toContain("const after = 2");
+ });
+
+ it("throws instead of returning a vacuous window when the start marker is gone", () => {
+ expect(() => sourceSegment(file, "function missing()", "const after")).toThrow(/start marker not found/);
+ });
+
+ it("rejects an ambiguous start marker, which would scope the window to the first hit only", () => {
+ const twice = ["render(<Panel />);", "const x = 1;", "render(<Panel />);", "const y = 2;"].join("\n");
+ expect(() => sourceSegment(twice, "render(<Panel />);", "const y")).toThrow(/occurs more than once/);
+ expect(sourceSegment(twice, "render(<Panel />);", "const y", { allowRepeatedStart: true })).toContain("const x");
+ });
+
+ it("rejects overlapping repeated start markers", () => {
+ expect(() => sourceSegment("aaa END", "aa", " END")).toThrow(/occurs more than once/);
+ });
+
+ it("rejects an empty marker instead of returning a misleading window", () => {
+ expect(() => sourceSegment(file, "function target()", "")).toThrow(/end marker must not be empty/);
+ expect(() => sourceSegment(file, "", "const after")).toThrow(/start marker must not be empty/);
+ });
+
+ it("names the contract in the failure when a label is given", () => {
+ expect(() => sourceSegment(file, "function missing()", "const after", { label: "mode menu focus" })).toThrow(
+ /^mode menu focus: start marker not found/,
+ );
+ });
+});
+
+describe("sourceFrom", () => {
+ it("runs to end of file and still guards the start marker", () => {
+ expect(sourceFrom(file, "function target()")).toContain("const after = 2;");
+ expect(() => sourceFrom(file, "function missing()")).toThrow(/start marker not found/);
+ });
+
+ it("rejects overlapping and empty start markers", () => {
+ expect(() => sourceFrom("aaa END", "aa")).toThrow(/occurs more than once/);
+ expect(() => sourceFrom(file, "")).toThrow(/start marker must not be empty/);
+ });
+});

tests/therapy-compass-responsive-contract.test.ts

@@ -2,6 +2,8 @@ import { existsSync, readFileSync } from "node:fs";
 
 import { describe, expect, it } from "vitest";
 
+import { sourceSegment } from "./helpers/source-contract";
+
 const read = (relativePath: string) => readFileSync(new URL(`../${relativePath}`, import.meta.url), "utf8");
 const therapyPath = "src/components/therapy-compass";
 
@@ -146,10 +148,9 @@ describe("Therapy Compass responsive contract", () => {
  });
 
  it("lets result-card evidence and actions use the phone width without forcing the desktop grid", () => {
- const resultCardSource = therapyCardSource.slice(
- therapyCardSource.indexOf("export function ResultCard"),
- therapyCardSource.indexOf("function CardCell"),
- );
+ const resultCardSource = sourceSegment(therapyCardSource, "export function ResultCard", "function CardCell", {
+ label: "therapy ResultCard",
+ });
 
  expect(resultCardSource).toContain("data-therapy-result-card");
  expect(resultCardSource).toContain("md:grid-cols-[minmax(240px,1fr)_minmax(320px,1.35fr)]");