Skip to content

fix(engine): keep an explicit null consolidate output instead of restoring the raw payload - #106

Open
BaaRai wants to merge 1 commit into
monid-ai:mainfrom
BaaRai:fix/consolidate-null-output
Open

BaaRai wants to merge 1 commit into
monid-ai:mainfrom
BaaRai:fix/consolidate-null-output

Conversation

@BaaRai

@BaaRai BaaRai commented Oct 8, 2026 •

Copy link
Copy Markdown

Fixes #104

The settle path picked the consolidated output with ??:

output = consolidated.output ?? raw;

Nullish coalescing treats an explicit null as absent, but the usage.consolidate contract only gives absence the meaning of "payload unchanged". null is valid Json (zJson includes it) and a consolidator can return it deliberately when everything it touched was billing. In that case the engine restored the raw vendor payload and re-exposed exactly the fields the hook meant to strip.

The fix falls back to the raw response only when consolidated.output is undefined:

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

Added a regression test next to the other D27 settle tests: a consolidate fn returning { credits: {}, output: null } against a successful response now settles with output: null instead of the raw object. Verified the test fails on the old line and passes with the fix; deno task check, the full deno task test suite (1246 passed) and deno task version:check (no contract-surface change, engine bump not required) are green.

Summary by CodeRabbit

  • Bug Fixes
    • Consolidated results that explicitly return null now preserve that value instead of falling back to the raw response.

…oring the raw payload

The settle path chose the consolidated output with ??, which treats an
explicit null as absent. The usage.consolidate contract only gives
absent (undefined) the meaning of "unchanged"; null is valid Json a
consolidator can return deliberately. Restoring the raw payload on null
re-exposed exactly the vendor billing fields the hook meant to strip.

Fall back to the raw response only when consolidated.output is
undefined, and add a regression test for the explicit null payload.

Fixes monid-ai#104
@BaaRai
BaaRai requested review from a team and ooctoo777 October 8, 2026 08:45
@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: monid-ai/monid/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 017480b3-a2db-4d8d-a470-01e71ad19554
📥 Commits

Reviewing files that changed from the base of the PR and between c57aa3d and 975efbc.

📒 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; 7 remain after this review.


📝 Walkthrough

Walkthrough

LoadedEndpoint.settle now uses the raw response only when usage.consolidate returns output: undefined. An explicit output: null is preserved. A regression test covers this case.

Priority: ⬇️ Low

Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to 975ef

Explicit null output is preserved, while omitted output still falls back to the raw response. No concrete merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: preserving an explicit null consolidated output instead of falling back to the raw payload.
Linked Issues check ✅ Passed Issue #104 requires explicit null output preservation, undefined-only fallback, and a regression test. The PR summary reports the settle change from nullish coalescing to an undefined check and adds t…
Out of Scope Changes check ✅ Passed The reported changes are limited to the settle-path implementation and its regression test. Both changes directly support issue #104. No unrelated change is identified.
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Engine restores raw payload when usage.consolidate returns null

1 participant