#208

fix(engine): keep an explicit null consolidated output instead of raw

Openrahul05ranjanwants to merge intofrom

Fixes #104.

What changes

At settle, the engine picked the consolidated output with nullish coalescing:

output = consolidated.output ?? raw; // engine/engine.ts:918

Consolidated.output is zJson.optional() — null is valid JSON and a valid consolidated result, and the hook contract (shared/core/schema/hooks/usage-consolidate.ts) says "absent = unchanged". Nullish coalescing treats explicit null as absent, so a hook returning { credits: {...}, output: null } had its result overwritten by the raw response — re-exposing exactly the vendor billing fields the hook stripped.

The settle path now falls back to raw only when consolidated.output is undefined:

output = consolidated.output === undefined ? raw : consolidated.output;

Verification

No deno on this host, so:

  • HEAD-vs-fix semantics of the exact expression, Node 26: with consolidated = { credits: {}, output: null } and raw = { results: [...], vendorTotal: 0 }, HEAD produces raw, the fix produces null; with output omitted (undefined), both produce raw — the "absent = unchanged" case is preserved.
  • Grep over engine/engine.ts: line 918 was the only ?? raw / consolidated-output fallback; the new fromResponse and doc.output.schema steps downstream are untouched and both already accept null (free-form outputs; zJson includes null).
  • Added a Deno.test regression in engine/engine.test.ts next to the empty-claim test, modeled on the same usageUnit harness: explicit { credits: {}, output: null } settles output === null, and an omitted output still passes the raw payload through. I could not run deno test locally; CI is the authoritative run.

Summary by CodeRabbit

  • Bug Fixes
    • Explicit null outputs from consolidation are now preserved, while omitted outputs continue to use the raw payload.
    • Usage is still settled when the consolidation claim is empty.

Activity

2
coderabbitai[bot]4d ago

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 09d54479142e36eff876383beacfbd37b33b5908 and efa08da1e0319bfaac16882026888ead7ac77e20.

Files selected for processing: 2
  • engine/engine.test.ts
  • engine/engine.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.

Walkthrough

LoadedEndpoint.settle now uses the raw payload only when usage.consolidate returns output: undefined. Explicit null remains the settled output. Tests cover both cases and verify derived usage.

Priority: Low

Severity of issue fixed: Low

Merge Risk: Minimal · up to efa08

The change implements the requested output behavior, with no identified issue blocking merge after normal checks.

Pre-merge checks: 4
Check nameStatusExplanation
Description CheckCheck skipped - CodeRabbit’s high-level summary is enabled.
Title checkThe title clearly and concisely describes the main change: preserving an explicit null consolidated output instead of falling back to the raw payload.
Linked Issues checkIssue #104 requires explicit null to remain the consolidated output and only undefined or an omitted output to preserve the raw payload. engine/engine.ts now uses an undefined check. `engine…
Out of Scope Changes checkThe pull request changes only the settle fallback and adds a regression test for issue #104. Both changes directly support the linked issue. No unrelated change is shown.
Full details: Linked Issues check

Explanation

Issue #104 requires explicit null to remain the consolidated output and only undefined or an omitted output to preserve the raw payload. engine/engine.ts now uses an undefined check. engine/engine.test.ts covers both explicit null and omitted output.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.