diff --git a/packages/bridge-compiler/src/codegen.ts b/packages/bridge-compiler/src/codegen.ts index 7d32eef0..533475bc 100644 --- a/packages/bridge-compiler/src/codegen.ts +++ b/packages/bridge-compiler/src/codegen.ts @@ -575,7 +575,7 @@ class CodegenContext { // When requestedFields is provided, drop output wires for fields that // weren't requested. Kahn's algorithm will then naturally eliminate // tools that only feed into those dropped wires. - const outputWires = this.requestedFields + const filteredOutputWires = this.requestedFields ? allOutputWires.filter((w) => { // Root wires (path length 0) and element wires are always included if (w.to.path.length === 0) return true; @@ -583,6 +583,7 @@ class CodegenContext { return matchesRequestedFields(fieldPath, this.requestedFields); }) : allOutputWires; + const outputWires = this.reorderOverdefinedOutputWires(filteredOutputWires); // Ensure force-only tools (no wires targeting them from output) are // still included in the tool map for scheduling @@ -2222,6 +2223,172 @@ class CodegenContext { return `{\n${entries.join(",\n")},\n${innerPad}}`; } + private reorderOverdefinedOutputWires(outputWires: Wire[]): Wire[] { + if (outputWires.length < 2) return outputWires; + + const groups = new Map(); + for (const wire of outputWires) { + const pathKey = wire.to.path.join("."); + const group = groups.get(pathKey) ?? []; + group.push(wire); + groups.set(pathKey, group); + } + + const emitted = new Set(); + const reordered: Wire[] = []; + let changed = false; + + for (const wire of outputWires) { + const pathKey = wire.to.path.join("."); + if (emitted.has(pathKey)) continue; + emitted.add(pathKey); + + const group = groups.get(pathKey)!; + if (group.length < 2) { + reordered.push(...group); + continue; + } + + const ranked = group.map((candidate, index) => ({ + wire: candidate, + index, + cost: this.classifyOverdefinitionWire(candidate), + })); + ranked.sort((left, right) => { + if (left.cost !== right.cost) { + changed = true; + return left.cost - right.cost; + } + return left.index - right.index; + }); + reordered.push(...ranked.map((entry) => entry.wire)); + } + + return changed ? reordered : outputWires; + } + + private classifyOverdefinitionWire( + wire: Wire, + visited = new Set(), + ): number { + return this.canResolveWireCheaply(wire, visited) ? 0 : 1; + } + + private canResolveWireCheaply( + wire: Wire, + visited = new Set(), + ): boolean { + if ("value" in wire) return true; + + if ("from" in wire) { + if (!this.refIsZeroCost(wire.from, visited)) return false; + for (const fallback of wire.fallbacks ?? []) { + if (fallback.ref && !this.refIsZeroCost(fallback.ref, visited)) { + return false; + } + } + if ( + wire.catchFallbackRef && + !this.refIsZeroCost(wire.catchFallbackRef, visited) + ) { + return false; + } + return true; + } + + if ("cond" in wire) { + if (!this.refIsZeroCost(wire.cond, visited)) return false; + if (wire.thenRef && !this.refIsZeroCost(wire.thenRef, visited)) + return false; + if (wire.elseRef && !this.refIsZeroCost(wire.elseRef, visited)) + return false; + for (const fallback of wire.fallbacks ?? []) { + if (fallback.ref && !this.refIsZeroCost(fallback.ref, visited)) { + return false; + } + } + if ( + wire.catchFallbackRef && + !this.refIsZeroCost(wire.catchFallbackRef, visited) + ) { + return false; + } + return true; + } + + if ("condAnd" in wire) { + if (!this.refIsZeroCost(wire.condAnd.leftRef, visited)) return false; + if ( + wire.condAnd.rightRef && + !this.refIsZeroCost(wire.condAnd.rightRef, visited) + ) { + return false; + } + for (const fallback of wire.fallbacks ?? []) { + if (fallback.ref && !this.refIsZeroCost(fallback.ref, visited)) { + return false; + } + } + if ( + wire.catchFallbackRef && + !this.refIsZeroCost(wire.catchFallbackRef, visited) + ) { + return false; + } + return true; + } + + if ("condOr" in wire) { + if (!this.refIsZeroCost(wire.condOr.leftRef, visited)) return false; + if ( + wire.condOr.rightRef && + !this.refIsZeroCost(wire.condOr.rightRef, visited) + ) { + return false; + } + for (const fallback of wire.fallbacks ?? []) { + if (fallback.ref && !this.refIsZeroCost(fallback.ref, visited)) { + return false; + } + } + if ( + wire.catchFallbackRef && + !this.refIsZeroCost(wire.catchFallbackRef, visited) + ) { + return false; + } + return true; + } + + return false; + } + + private refIsZeroCost(ref: NodeRef, visited = new Set()): boolean { + if (ref.element) return true; + if ( + ref.module === SELF_MODULE && + ((ref.type === this.bridge.type && ref.field === this.bridge.field) || + (ref.type === "Context" && ref.field === "context") || + (ref.type === "Const" && ref.field === "const")) + ) { + return true; + } + if (ref.module.startsWith("__define_")) return false; + + const key = refTrunkKey(ref); + if (visited.has(key)) return false; + visited.add(key); + + if (ref.module === "__local") { + const incoming = this.bridge.wires.filter( + (wire) => refTrunkKey(wire.to) === key, + ); + return incoming.some((wire) => this.canResolveWireCheaply(wire, visited)); + } + + return false; + } + /** * Build the body of a `.map()` callback from element wires. * diff --git a/packages/bridge-compiler/test/codegen.test.ts b/packages/bridge-compiler/test/codegen.test.ts index 7cbce661..964f567e 100644 --- a/packages/bridge-compiler/test/codegen.test.ts +++ b/packages/bridge-compiler/test/codegen.test.ts @@ -1399,7 +1399,7 @@ bridge Query.test { assert.deepStrictEqual(callLog, ["expensiveApi"], "tool should be called"); }); - test("tool before input — tool always called (primary position)", async () => { + test("tool before input — zero-cost input still wins", async () => { const callLog: string[] = []; const result = await compileAndRun( `version 1.5 @@ -1420,9 +1420,8 @@ bridge Query.test { }, }, ); - // Tool is first (primary) → always called → wins - assert.equal(result.label, "from-api"); - assert.deepStrictEqual(callLog, ["api"]); + assert.equal(result.label, "from-input"); + assert.deepStrictEqual(callLog, []); }); test("two tools — second skipped when first resolves non-null", async () => { @@ -1532,6 +1531,48 @@ bridge Query.test { assert.deepStrictEqual(aotNoCache.data, rtNoCache.data); }); + test("overdefinition parity — zero-cost sources win before tool calls", async () => { + const bridgeText = `version 1.5 +bridge Query.test { + with expensiveApi + with input as i + with output as o + + o.val <- expensiveApi.data + o.val <- i.cached +}`; + const document = parseBridgeFormat(bridgeText); + const callLog: string[] = []; + const tools = { + expensiveApi: () => { + callLog.push("expensiveApi"); + return { data: "expensive" }; + }, + }; + + const rtWithCache = await executeBridge({ + document, + operation: "Query.test", + input: { cached: "hit" }, + tools, + }); + const rtLog = [...callLog]; + callLog.length = 0; + + const aotWithCache = await executeAot({ + document, + operation: "Query.test", + input: { cached: "hit" }, + tools, + }); + const aotLog = [...callLog]; + + assert.deepStrictEqual(rtWithCache.data, { val: "hit" }); + assert.deepStrictEqual(aotWithCache.data, { val: "hit" }); + assert.deepStrictEqual(rtLog, []); + assert.deepStrictEqual(aotLog, []); + }); + test("constant overdefinition parity — first constant remains terminal", async () => { const document: BridgeDocument = { instructions: [ diff --git a/packages/bridge-core/src/ExecutionTree.ts b/packages/bridge-core/src/ExecutionTree.ts index 2f6485a3..430a55c7 100644 --- a/packages/bridge-core/src/ExecutionTree.ts +++ b/packages/bridge-core/src/ExecutionTree.ts @@ -29,6 +29,7 @@ import type { } from "./tree-types.ts"; import { BREAK_SYM, + attachBridgeErrorMetadata, BridgeAbortError, BridgePanicError, wrapBridgeRuntimeError, @@ -881,8 +882,11 @@ export class ExecutionTree implements TreeContext { // ── Cycle detection ───────────────────────────────────────────── if (pullChain.has(key)) { - throw new BridgePanicError( - `Circular dependency detected: "${key}" depends on itself`, + throw attachBridgeErrorMetadata( + new BridgePanicError( + `Circular dependency detected: "${key}" depends on itself`, + ), + { bridgeLoc }, ); } @@ -890,7 +894,7 @@ export class ExecutionTree implements TreeContext { // current element. Otherwise top-level aliases/tools reused inside arrays // are recomputed once per element instead of being memoized at the parent. if (this.parent && !ref.element && !this.isElementScopedTrunk(ref)) { - return this.parent.pullSingle(ref, pullChain); + return this.parent.pullSingle(ref, pullChain, bridgeLoc); } // Walk the full parent chain — shadow trees may be nested multiple levels @@ -1008,6 +1012,174 @@ export class ExecutionTree implements TreeContext { return _resolveWires(this, wires, pullChain); } + classifyOverdefinitionWire(wire: Wire): number { + return this.canResolveWireWithoutScheduling(wire) ? 0 : 1; + } + + private canResolveWireWithoutScheduling( + wire: Wire, + visited = new Set(), + ): boolean { + if ("value" in wire) return true; + + if ("from" in wire) { + if (!this.canResolveRefWithoutScheduling(wire.from, visited)) { + return false; + } + for (const fallback of wire.fallbacks ?? []) { + if ( + fallback.ref && + !this.canResolveRefWithoutScheduling(fallback.ref, visited) + ) { + return false; + } + } + if ( + wire.catchFallbackRef && + !this.canResolveRefWithoutScheduling(wire.catchFallbackRef, visited) + ) { + return false; + } + return true; + } + + if ("cond" in wire) { + if (!this.canResolveRefWithoutScheduling(wire.cond, visited)) + return false; + if ( + wire.thenRef && + !this.canResolveRefWithoutScheduling(wire.thenRef, visited) + ) { + return false; + } + if ( + wire.elseRef && + !this.canResolveRefWithoutScheduling(wire.elseRef, visited) + ) { + return false; + } + for (const fallback of wire.fallbacks ?? []) { + if ( + fallback.ref && + !this.canResolveRefWithoutScheduling(fallback.ref, visited) + ) { + return false; + } + } + if ( + wire.catchFallbackRef && + !this.canResolveRefWithoutScheduling(wire.catchFallbackRef, visited) + ) { + return false; + } + return true; + } + + if ("condAnd" in wire) { + if (!this.canResolveRefWithoutScheduling(wire.condAnd.leftRef, visited)) { + return false; + } + if ( + wire.condAnd.rightRef && + !this.canResolveRefWithoutScheduling(wire.condAnd.rightRef, visited) + ) { + return false; + } + for (const fallback of wire.fallbacks ?? []) { + if ( + fallback.ref && + !this.canResolveRefWithoutScheduling(fallback.ref, visited) + ) { + return false; + } + } + if ( + wire.catchFallbackRef && + !this.canResolveRefWithoutScheduling(wire.catchFallbackRef, visited) + ) { + return false; + } + return true; + } + + if ("condOr" in wire) { + if (!this.canResolveRefWithoutScheduling(wire.condOr.leftRef, visited)) { + return false; + } + if ( + wire.condOr.rightRef && + !this.canResolveRefWithoutScheduling(wire.condOr.rightRef, visited) + ) { + return false; + } + for (const fallback of wire.fallbacks ?? []) { + if ( + fallback.ref && + !this.canResolveRefWithoutScheduling(fallback.ref, visited) + ) { + return false; + } + } + if ( + wire.catchFallbackRef && + !this.canResolveRefWithoutScheduling(wire.catchFallbackRef, visited) + ) { + return false; + } + return true; + } + + return false; + } + + private canResolveRefWithoutScheduling( + ref: NodeRef, + visited = new Set(), + ): boolean { + if (ref.element) return true; + if (this.hasCachedRef(ref)) return true; + + const key = ((ref as any)[TRUNK_KEY_CACHE] ??= trunkKey(ref)); + if (visited.has(key)) return false; + visited.add(key); + + if (ref.module.startsWith("__define_")) return false; + + if (ref.module === "__local") { + const incoming = + this.bridge?.wires.filter((wire) => sameTrunk(wire.to, ref)) ?? []; + for (const wire of incoming) { + if (this.canResolveWireWithoutScheduling(wire, visited)) { + return true; + } + } + return false; + } + + return false; + } + + private hasCachedRef(ref: NodeRef): boolean { + if (this.parent && !ref.element && !this.isElementScopedTrunk(ref)) { + return this.parent.hasCachedRef(ref); + } + + const key: string = ((ref as any)[TRUNK_KEY_CACHE] ??= trunkKey(ref)); + let cursor: ExecutionTree | undefined = this; + if (ref.element && ref.elementDepth && ref.elementDepth > 0) { + let remaining = ref.elementDepth; + while (remaining > 0 && cursor) { + cursor = cursor.parent; + remaining--; + } + } + while (cursor) { + if (cursor.state[key] !== undefined) return true; + cursor = cursor.parent; + } + return false; + } + /** * Resolve an output field by path for use outside of a GraphQL resolver. * diff --git a/packages/bridge-core/src/resolveWires.ts b/packages/bridge-core/src/resolveWires.ts index ab64e567..7750e025 100644 --- a/packages/bridge-core/src/resolveWires.ts +++ b/packages/bridge-core/src/resolveWires.ts @@ -82,7 +82,29 @@ export function resolveWires( ); } } - return resolveWiresAsync(ctx, wires, pullChain); + const orderedWires = orderOverdefinedWires(ctx, wires); + return resolveWiresAsync(ctx, orderedWires, pullChain); +} + +function orderOverdefinedWires(ctx: TreeContext, wires: Wire[]): Wire[] { + if (wires.length < 2 || !ctx.classifyOverdefinitionWire) return wires; + + const ranked = wires.map((wire, index) => ({ + wire, + index, + cost: ctx.classifyOverdefinitionWire!(wire), + })); + + let changed = false; + ranked.sort((left, right) => { + if (left.cost !== right.cost) { + changed = true; + return left.cost - right.cost; + } + return left.index - right.index; + }); + + return changed ? ranked.map((entry) => entry.wire) : wires; } // ── Async resolution loop ─────────────────────────────────────────────────── diff --git a/packages/bridge-core/src/tree-types.ts b/packages/bridge-core/src/tree-types.ts index 66de63cd..d236cde7 100644 --- a/packages/bridge-core/src/tree-types.ts +++ b/packages/bridge-core/src/tree-types.ts @@ -10,6 +10,7 @@ import type { ControlFlowInstruction, NodeRef, SourceLocation, + Wire, } from "./types.ts"; // ── Error classes ─────────────────────────────────────────────────────────── @@ -131,6 +132,8 @@ export interface TreeContext { pullChain?: Set, bridgeLoc?: SourceLocation, ): MaybePromise; + /** Classify an overdefined wire by marginal execution cost (lower = cheaper). */ + classifyOverdefinitionWire?(wire: Wire): number; /** External abort signal — cancels execution when triggered. */ signal?: AbortSignal; } diff --git a/packages/bridge/test/coalesce-cost.test.ts b/packages/bridge/test/coalesce-cost.test.ts index 212fcaff..fd9dcf4f 100644 --- a/packages/bridge/test/coalesce-cost.test.ts +++ b/packages/bridge/test/coalesce-cost.test.ts @@ -9,7 +9,7 @@ import { createGateway } from "./_gateway.ts"; // ═══════════════════════════════════════════════════════════════════════════ // v2.0 Execution Semantics: // • || chains evaluate sequentially (left to right) with short-circuit -// • Overdefinition uses cost-based ordering (cheap → expensive) +// • Overdefinition uses cost-based ordering (zero-cost/already-resolved → expensive) // • Backup tools are NEVER called when a earlier source returns a truthy value // ═══════════════════════════════════════════════════════════════════════════ @@ -278,8 +278,8 @@ o.label <- p.label || b.label || "null-default" catch "error-default" // ── Cost-based resolution: overdefinition ──────────────────────────────── -describe("overdefinition: authored order respected", () => { - test("first wire wins when both are non-null (left-to-right)", async () => { +describe("overdefinition: cost-based prioritization", () => { + test("input beats tool even when tool wire is authored first", async () => { const bridgeText = `version 1.5 bridge Query.lookup { with expensiveApi as api @@ -305,12 +305,11 @@ o.label <- i.hint const result: any = await executor({ document: parse(`{ lookup(q: "x", hint: "cheap") { label } }`), }); - // Authored order: api.label is first → wins - assert.equal(result.data.lookup.label, "expensive"); + assert.equal(result.data.lookup.label, "cheap"); assertDeepStrictEqualIgnoringLoc( callLog, - ["expensiveApi"], - "first wire evaluated first", + [], + "zero-cost input should short-circuit before the API is called", ); }); @@ -345,22 +344,21 @@ o.label <- i.hint assertDeepStrictEqualIgnoringLoc( callLog, ["expensiveApi"], - "API called when input is null", + "API should run only when zero-cost sources are nullish", ); }); - test("overdefinition respects authored order — first wire wins", async () => { - // The expensive tool wire is written FIRST, so it is evaluated first. - // Left-to-right semantics mean the tool result wins. + test("context beats tool even when tool wire is authored first", async () => { const bridgeText = `version 1.5 bridge Query.lookup { with expensiveApi as api + with context as ctx with input as i with output as o api.q <- i.q o.label <- api.label -o.label <- i.hint +o.label <- ctx.defaultLabel }`; const callLog: string[] = []; @@ -371,31 +369,34 @@ o.label <- i.hint }, }; const doc = parseBridge(bridgeText); - const gateway = createGateway(typeDefs, doc, { tools }); + const gateway = createGateway(typeDefs, doc, { + tools, + context: { defaultLabel: "from-context" }, + }); const executor = buildHTTPExecutor({ fetch: gateway.fetch as any }); const result: any = await executor({ - document: parse(`{ lookup(q: "x", hint: "from-input") { label } }`), + document: parse(`{ lookup(q: "x") { label } }`), }); - assert.equal(result.data.lookup.label, "expensive"); + assert.equal(result.data.lookup.label, "from-context"); assertDeepStrictEqualIgnoringLoc( callLog, - ["expensiveApi"], - "first wire wins — authored order matters", + [], + "zero-cost context should short-circuit before the API is called", ); }); - test("authored order: tool before context — tool wins", async () => { + test("resolved alias beats tool even when tool wire is authored first", async () => { const bridgeText = `version 1.5 bridge Query.lookup { with expensiveApi as api - with context as ctx with input as i with output as o +alias i.hint as cached api.q <- i.q o.label <- api.label -o.label <- ctx.defaultLabel +o.label <- cached }`; const callLog: string[] = []; @@ -406,25 +407,21 @@ o.label <- ctx.defaultLabel }, }; const doc = parseBridge(bridgeText); - const gateway = createGateway(typeDefs, doc, { - tools, - context: { defaultLabel: "from-context" }, - }); + const gateway = createGateway(typeDefs, doc, { tools }); const executor = buildHTTPExecutor({ fetch: gateway.fetch as any }); const result: any = await executor({ - document: parse(`{ lookup(q: "x") { label } }`), + document: parse(`{ lookup(q: "x", hint: "cached") { label } }`), }); - // api.label is first wire → evaluated first → wins - assert.equal(result.data.lookup.label, "expensive"); + assert.equal(result.data.lookup.label, "cached"); assertDeepStrictEqualIgnoringLoc( callLog, - ["api"], - "authored order: api first, context second", + [], + "resolved aliases should be treated like zero-cost values", ); }); - test("two tool sources with same cost — file order preserved", async () => { + test("two tool sources with same cost preserve authored order as tie-break", async () => { const bridgeText = `version 1.5 bridge Query.lookup { with svcA as a @@ -456,12 +453,11 @@ o.label <- b.label const result: any = await executor({ document: parse(`{ lookup(q: "x") { label } }`), }); - // Authored order: A is first in the bridge → wins. assert.equal(result.data.lookup.label, "from-A"); assertDeepStrictEqualIgnoringLoc( callLog, ["A"], - "B never called — A is first, short-circuits", + "same-cost tool sources should still use authored order as a tie-break", ); }); }); diff --git a/packages/bridge/test/runtime-error-format.test.ts b/packages/bridge/test/runtime-error-format.test.ts index c87d0656..6572c569 100644 --- a/packages/bridge/test/runtime-error-format.test.ts +++ b/packages/bridge/test/runtime-error-format.test.ts @@ -106,6 +106,25 @@ bridge Query.pricing { o.price <- i.isPro.fail.asd ? i.proPrice : i.basicPrice }`; +const bridgePeekCycleText = `version 1.5 + +tool geo from std.httpCall { + .baseUrl = "https://nominatim.openstreetmap.org" + .path = "/search" + .format = "json" + .limit = "1" +} + +bridge Query.location { + with geo + with input as i + with output as o + + geo.q <- geo[0].city + o.lat <- geo[0].lat + o.lon <- geo[0].lon +}`; + function maxCaretCount(formatted: string): number { return Math.max( 0, @@ -386,4 +405,30 @@ describe("runtime error formatting", () => { }, ); }); + + test("tool input cycles retain the originating wire source location", async () => { + const document = parseBridge(bridgePeekCycleText, { + filename: "playground.bridge", + }); + + await assert.rejects( + () => + executeBridge({ + document, + operation: "Query.location", + input: {}, + }), + (err: unknown) => { + const formatted = formatBridgeError(err); + assert.match( + formatted, + /Bridge Execution Error: Circular dependency detected: "_:Tools:geo:1" depends on itself/, + ); + assert.match(formatted, /playground\.bridge:15:12/); + assert.match(formatted, /geo\.q <- geo\[0\]\.city/); + assert.equal(maxCaretCount(formatted), "geo[0].city".length); + return true; + }, + ); + }); }); diff --git a/packages/bridge/test/shared-parity.test.ts b/packages/bridge/test/shared-parity.test.ts index ac22a563..fbe9cd39 100644 --- a/packages/bridge/test/shared-parity.test.ts +++ b/packages/bridge/test/shared-parity.test.ts @@ -1228,7 +1228,7 @@ runSharedSuite("Shared: alias declarations", aliasCases); const overdefinitionCases: SharedTestCase[] = [ { - name: "first wire wins when both non-null", + name: "zero-cost input beats tool even when tool wire is first", bridgeText: `version 1.5 bridge Query.lookup { with expensiveApi as api @@ -1243,10 +1243,10 @@ bridge Query.lookup { tools: { expensiveApi: async () => ({ label: "from-api" }), }, - expected: { label: "from-api" }, + expected: { label: "cheap" }, }, { - name: "first wire null — falls through to second", + name: "tool runs when zero-cost input is nullish", bridgeText: `version 1.5 bridge Query.lookup { with api @@ -1263,6 +1263,47 @@ bridge Query.lookup { }, expected: { label: "fallback" }, }, + { + name: "zero-cost context beats tool even when tool wire is first", + bridgeText: `version 1.5 +bridge Query.lookup { + with expensiveApi as api + with context as ctx + with input as i + with output as o + api.q <- i.q + o.label <- api.label + o.label <- ctx.defaultLabel +}`, + operation: "Query.lookup", + input: { q: "x" }, + context: { defaultLabel: "from-context" }, + tools: { + expensiveApi: async () => ({ label: "from-api" }), + }, + expected: { label: "from-context" }, + }, + { + name: "same-cost tool sources preserve authored order", + bridgeText: `version 1.5 +bridge Query.lookup { + with svcA as a + with svcB as b + with input as i + with output as o + a.q <- i.q + b.q <- i.q + o.label <- a.label + o.label <- b.label +}`, + operation: "Query.lookup", + input: { q: "x" }, + tools: { + svcA: async () => ({ label: "from-A" }), + svcB: async () => ({ label: "from-B" }), + }, + expected: { label: "from-A" }, + }, ]; runSharedSuite("Shared: overdefinition", overdefinitionCases);