Skip to content

Commit 2cb4233

Browse files
committed
fix(observability-map): enter the walk exactly where execution is guaranteed
The catch-evidence walk entered only a bare block and a do body, so relocating a clause's own statements inside if (true), a single-default switch, an if/else or a try/finally hid the branch evidence while the returns veto still saw the return: a deciding clause read as a swallow on 83 real routes per corpus entry, the generalisation of the switch-break bug fixed in 87e0822. The walk now enters the positions guaranteed to execute whenever the clause runs: a catchless try's tryBlock, the sole clause of a single-default switch, the then-arm of a keyword-exact if (true), and both arms of an if/else with isolated per-arm states merged by intersection (evidence in one arm only earns nothing). definitelyExits folds the literal true keyword so a trailing dead statement after if (true) { exit } is cut. The entry folds are keyword-exact while the liveness folds stay wide, deliberately: entry grants credit, liveness only withholds blindness. Refactors the walk's flags into a threaded state record (verified byte-identical on the real tree before the entries landed); the finished step is also byte-identical including per-clause evidence over all 427 entry points. The six mechanism-B corpus entries measure falls 0 rises 0. New corpus entry dead-classifier-one-arm pins the intersection: widening it to a union raises 80 routes and turns the entry red.
1 parent 467a7fd commit 2cb4233

5 files changed

Lines changed: 328 additions & 39 deletions

File tree

internal-packages/observability-map/src/checks/index.test.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -796,6 +796,51 @@ describe("error-classification", () => {
796796
);
797797
expect(r.status).toBe("pass");
798798
});
799+
800+
// The verdict end of the walk's guaranteed-execution entries, one pair per entered construct.
801+
// The evidence end is `the walk enters exactly the positions guaranteed to execute` in
802+
// scan.test.ts; these hold the wrapped and unwrapped spellings to the same verdict, modeled on
803+
// the switch pair above. Before the entries existed, every wrapper here turned a passing
804+
// deciding clause into a fail with a detail line accusing it of ignoring the error.
805+
const CLAUSE_WRAPPED = (clauseBody: string) => `import { prisma } from "~/db.server";
806+
export async function loader() {
807+
try {
808+
return json(await prisma.thing.findMany());
809+
} catch (e) {
810+
${clauseBody}
811+
}
812+
}`;
813+
814+
const DECIDING_CLAUSE =
815+
"if (e instanceof Error) { return new Response(null, { status: 400 }); }\n" +
816+
"return new Response(null, { status: 500 });";
817+
818+
const GUARANTEED_WRAPPERS: Array<[string, (body: string) => string]> = [
819+
["a catchless try/finally", (body) => `try {\n${body}\n} finally { }`],
820+
["a single-default switch", (body) => `switch (pick()) { default: {\n${body}\n} }`],
821+
["an if (true)", (body) => `if (true) {\n${body}\n}`],
822+
[
823+
"an if/else with the body in both arms",
824+
(body) => `if (pick()) {\n${body}\n} else {\n${body}\n}`,
825+
],
826+
];
827+
828+
for (const [label, wrap] of GUARANTEED_WRAPPERS) {
829+
it(`reads a deciding clause relocated into ${label} with the same verdict`, () => {
830+
const wrapped = run(
831+
"error-classification",
832+
"api.v1.wrapped.ts",
833+
CLAUSE_WRAPPED(wrap(DECIDING_CLAUSE))
834+
);
835+
const bare = run(
836+
"error-classification",
837+
"api.v1.wrapped.ts",
838+
CLAUSE_WRAPPED(DECIDING_CLAUSE)
839+
);
840+
expect(bare.status).toBe("pass");
841+
expect(wrapped).toEqual(bare);
842+
});
843+
}
799844
});
800845

801846
describe("auth-boundary", () => {

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -688,6 +688,17 @@ export const MUTATIONS: Mutation[] = [
688688
"splice if (e instanceof Error) { } into every catch",
689689
(e) => `if (${e} instanceof Error) { }`
690690
),
691+
// The if/else arm walk merges per-arm evidence by INTERSECTION, so a real classifier sitting in
692+
// one arm only earns nothing: one arm running is a condition, not a guarantee. This is the entry
693+
// that goes red the day someone "simplifies" the intersection to a union, at which point every
694+
// clause in the tree earns a branch from a dead arm. Additive: it plants fake signal.
695+
prependToEveryCatch(
696+
"dead-classifier-one-arm",
697+
"preserving",
698+
"splice a dead one-arm classifier if/else into every catch",
699+
(e) =>
700+
`if (false) { if (${e} instanceof Error) { return new Response(null, { status: 400 }); } } else { 0; }`
701+
),
691702

692703
// The additive class. Everything above either takes signal away or moves it about; these put in
693704
// signal that is not real, which is the direction the corpus was blind to.
@@ -905,6 +916,7 @@ export const ADDITIVE_IDS = [
905916
"wrap-body-in-rethrow",
906917
"wrap-body-in-same-arms-throw-ternary",
907918
"empty-instanceof-if",
919+
"dead-classifier-one-arm",
908920
"registered-throw",
909921
"fake-require-guard",
910922
"fake-authenticated-lookup",

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

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -865,6 +865,109 @@ describe("scanFile: catch clause evidence", () => {
865865
});
866866
});
867867

868+
// The walk may enter a construct exactly where the entered statements are guaranteed to execute
869+
// whenever the clause body runs. Before these entries existed, relocating a clause's own
870+
// statements inside `if (true)`, a switch default, an if/else or a try/finally put the branch
871+
// evidence out of reach while the returns veto still saw the return, so a deciding clause read
872+
// as a swallow: 83 real routes regressed per corpus entry. Each identity pair here holds the
873+
// wrapped and unwrapped spellings to the same evidence; `checks/index.test.ts` holds them to the
874+
// same verdict.
875+
describe("the walk enters exactly the positions guaranteed to execute", () => {
876+
const clauseEvidence = (body: string) => {
877+
const ep = scanFile(
878+
"x.ts",
879+
`export async function loader() {
880+
try { return await prisma.thing.findMany(); }
881+
catch (e) {
882+
${body}
883+
}
884+
}`
885+
);
886+
return ep!.catches[0]!;
887+
};
888+
889+
const DECIDING =
890+
"if (e instanceof KnownError) { return new Response(e.code, { status: 400 }); }\n" +
891+
"return new Response(null, { status: 500 });";
892+
const RETHROW = "logger.error(e);\nthrow e;";
893+
894+
const WRAPPERS: Array<[string, (body: string) => string]> = [
895+
["a catchless try/finally", (body) => `try {\n${body}\n} finally { }`],
896+
["a single-default switch", (body) => `switch (pick()) { default: {\n${body}\n} }`],
897+
["an if (true)", (body) => `if (true) {\n${body}\n}`],
898+
[
899+
"an if/else with the body in both arms",
900+
(body) => `if (pick()) {\n${body}\n} else {\n${body}\n}`,
901+
],
902+
];
903+
904+
for (const [label, wrap] of WRAPPERS) {
905+
it(`reads a deciding clause wrapped in ${label} identically`, () => {
906+
expect(clauseEvidence(wrap(DECIDING))).toEqual(clauseEvidence(DECIDING));
907+
});
908+
909+
it(`reads a rethrowing clause wrapped in ${label} identically`, () => {
910+
expect(clauseEvidence(wrap(RETHROW))).toEqual(clauseEvidence(RETHROW));
911+
});
912+
}
913+
914+
// The switch entry is exact: one clause and it is a default. Anything else is not entered and
915+
// keeps its top-level treatment, which is what the identity pair below and the `break and
916+
// continue inside the construct they target` family already pin for the multi-clause shapes.
917+
it("reads a clause wrapped in a single-default switch as the bare clause", () => {
918+
expect(clauseEvidence(`switch (1) { default: {\n${DECIDING}\n} }`)).toEqual(
919+
clauseEvidence(DECIDING)
920+
);
921+
});
922+
923+
// Intersection, not union. One arm running is a condition, not a guarantee, so evidence in a
924+
// single arm earns nothing; `dead-classifier-one-arm` in the mutation corpus is the tree-scale
925+
// twin with the arm provably dead.
926+
it("does not credit a classifier that sits in one arm only", () => {
927+
const evidence = clauseEvidence(
928+
"if (pick()) { if (e instanceof Error) { return new Response(null, { status: 400 }); } } else { 0; }\n" +
929+
"return null;"
930+
);
931+
expect(evidence.branches).toBe(false);
932+
});
933+
934+
it("does not credit a classifier in a dead arm beside an inert arm", () => {
935+
const evidence = clauseEvidence(
936+
"if (false) { if (e instanceof Error) { return new Response(null, { status: 400 }); } } else { 0; }\n" +
937+
"return null;"
938+
);
939+
expect(evidence).toMatchObject({ branches: false, rethrows: false, throws: false });
940+
});
941+
942+
// The else-arm under a literal-true guard can never run, so nothing in it is evidence: no
943+
// rethrow minted, and the deciding statements after the wrapper keep their credit.
944+
it("reads a dead else arm under if true as contributing nothing", () => {
945+
const evidence = clauseEvidence(`if (true) { 0; } else { throw e; }\n${DECIDING}`);
946+
expect(evidence).toMatchObject({ throws: false, branches: true });
947+
});
948+
949+
// A throw in the tryBlock of a CAUGHT try never escapes the clause: the nested catch takes
950+
// it. Crediting it as a rethrow would launder a returnless swallow into not-applicable.
951+
it("does not read the tryBlock of a caught try as this clause's rethrow", () => {
952+
const evidence = clauseEvidence("try { throw e; } catch {}\nlogger.error(e);");
953+
expect(evidence).toMatchObject({ rethrows: false, throws: false });
954+
});
955+
956+
// The walk does not enter a finally block, so the returns veto must still read it off the
957+
// whole statement: this clause's finally return eats the throw, and the error never leaves.
958+
it("reads a try whose finally returns as swallowing, not rethrowing", () => {
959+
const evidence = clauseEvidence("try { throw e; } finally { return null; }");
960+
expect(evidence.rethrows).toBe(false);
961+
});
962+
963+
// The `definitelyExits` fold: `if (true) { X }` definitely exits iff X does, so the trailing
964+
// throw is cut rather than read. Without the fold the throw still walks and mints `throws`.
965+
it("cuts a dead trailing statement after an if true that exits", () => {
966+
const evidence = clauseEvidence("if (true) { return null; }\nthrow e;");
967+
expect(evidence.throws).toBe(false);
968+
});
969+
});
970+
868971
// S1. The other end of the same problem. `reachableStatements` used to cut the statement list
869972
// only on a BARE `return`/`throw`, while the walk descended into blocks and `do` bodies, so a
870973
// `throw e;` written after a nested construct that had already returned was still read as the

0 commit comments

Comments
 (0)