fix(observability-map): stop crediting an if or switch that never looks at the error

catchClauseEvidence counted any if/switch inside a catch as branching, whether
or not its condition read the caught error. catch (_e) { if (organization)
... } and catch (error: any) { if (wantsJson) ... } both counted, crediting a
route that always takes the same path regardless of what was thrown.

The if condition (or switch discriminant) must now reference the caught
error binding; a bindingless catch { ... } cannot qualify at all. Ternary
handling is unchanged. Measured on the real tree: 5 of 242 catch clauses lose
branches, 4 routes flip from pass to fail on error-classification, and the
global score moves from 18 to 17. Hand-read all 4: each was a false positive,
branching on an unrelated local rather than the error.

Author: 1stvamp

internal-packages/observability-map/src/scan.ts

@@ -153,11 +153,28 @@ function guardsParse(tryBlock: ts.Block): boolean {
   return found;
 }
 
+/** Whether some node in the tree rooted at `node` matches `predicate`. */
+function someNode(node: ts.Node, predicate: (n: ts.Node) => boolean): boolean {
+  if (predicate(node)) return true;
+  return ts.forEachChild(node, (child) => someNode(child, predicate)) === true;
+}
+
 function containsInstanceOf(node: ts.Node): boolean {
-  if (ts.isBinaryExpression(node) && node.operatorToken.kind === ts.SyntaxKind.InstanceOfKeyword) {
-    return true;
-  }
-  return ts.forEachChild(node, containsInstanceOf) === true;
+  return someNode(
+    node,
+    (n) => ts.isBinaryExpression(n) && n.operatorToken.kind === ts.SyntaxKind.InstanceOfKeyword
+  );
+}
+
+/** Whether `node` reads the given catch binding anywhere, e.g. `e` in `e instanceof X` or `error.code`. */
+function referencesBinding(node: ts.Node, bindingName: string): boolean {
+  return someNode(node, (n) => ts.isIdentifier(n) && n.text === bindingName);
+}
+
+/** The catch binding's name, or null for a bindingless `catch { ... }` or a destructured one. */
+function catchBindingName(clause: ts.CatchClause): string | null {
+  const decl = clause.variableDeclaration;
+  return decl && ts.isIdentifier(decl.name) ? decl.name.text : null;
 }
 
 /**
@@ -177,10 +194,17 @@ function selectsAnErrorPath(node: ts.ConditionalExpression): boolean {
 function catchClauseEvidence(clause: ts.CatchClause): { rethrows: boolean; branches: boolean } {
   let rethrows = false;
   let branches = false;
+  const bindingName = catchBindingName(clause);
 
   const visit = (node: ts.Node) => {
     if (ts.isThrowStatement(node)) rethrows = true;
-    if (ts.isIfStatement(node) || ts.isSwitchStatement(node)) branches = true;
+    if (
+      bindingName !== null &&
+      ((ts.isIfStatement(node) && referencesBinding(node.expression, bindingName)) ||
+        (ts.isSwitchStatement(node) && referencesBinding(node.expression, bindingName)))
+    ) {
+      branches = true;
+    }
     if (ts.isConditionalExpression(node) && selectsAnErrorPath(node)) branches = true;
     ts.forEachChild(node, visit);
   };

internal-packages/observability-map/test/scan.test.ts

@@ -1289,3 +1289,101 @@ describe("scanFile: branches ignores the error-stringifying ternary", () => {
     expect(ep!.catches[0]!.branches).toBe(true);
   });
 });
+
+describe("scanFile: branches requires the if/switch condition to examine the error", () => {
+  it("does not set branches for an `if` on an unrelated variable", () => {
+    const ep = scanFile(
+      "retry-count.ts",
+      `
+      export async function action({ request }) {
+        let attempt = 0;
+        try {
+          return json(await load(request));
+        } catch (e) {
+          if (attempt > 3) return json({}, { status: 503 });
+          return json({}, { status: 500 });
+        }
+      }
+      `
+    );
+    expect(ep!.catches[0]!.branches).toBe(false);
+  });
+
+  it("sets branches for an `if` whose condition references the caught error", () => {
+    const ep = scanFile(
+      "branch-if-e.ts",
+      `
+      export async function loader({ request }) {
+        try {
+          return json(await load(request));
+        } catch (e) {
+          if (e instanceof ApiError) return json({}, { status: e.status });
+          return json({}, { status: 500 });
+        }
+      }
+      `
+    );
+    expect(ep!.catches[0]!.branches).toBe(true);
+  });
+
+  it("sets branches for a `switch` on a property of the caught error", () => {
+    const ep = scanFile(
+      "branch-switch-error-code.ts",
+      `
+      export async function loader({ request }) {
+        try {
+          return json(await load(request));
+        } catch (error) {
+          switch (error.code) {
+            case "P2025":
+              return json({}, { status: 404 });
+            default:
+              return json({}, { status: 500 });
+          }
+        }
+      }
+      `
+    );
+    expect(ep!.catches[0]!.branches).toBe(true);
+  });
+
+  it("does not set branches for a `switch` on an unrelated discriminant", () => {
+    const ep = scanFile(
+      "branch-switch-unrelated.ts",
+      `
+      export async function action({ request }) {
+        const mode = "strict";
+        try {
+          return json(await load(request));
+        } catch (error) {
+          switch (mode) {
+            case "strict":
+              return json({}, { status: 400 });
+            default:
+              return json({}, { status: 500 });
+          }
+        }
+      }
+      `
+    );
+    expect(ep!.catches[0]!.branches).toBe(false);
+  });
+
+  it("cannot set branches for a bindingless catch, even with an `if` inside it", () => {
+    const ep = scanFile(
+      "bindingless-if.ts",
+      `
+      export async function loader({ request }) {
+        let attempt = 0;
+        try {
+          return json(await load(request));
+        } catch {
+          if (attempt > 3) return json({}, { status: 503 });
+          return json({}, { status: 500 });
+        }
+      }
+      `
+    );
+    expect(ep!.catches[0]!.branches).toBe(false);
+  });
+});