Skip to content

fix: pass mode='json' to model_dump() in PyHookRegistry emit path - #77

Merged
Brian Krabach (bkrabach) merged 1 commit into
mainfrom
fix/hook-emit-json-safe-model-dump
May 18, 2026
Merged

fix: pass mode='json' to model_dump() in PyHookRegistry emit path#77
Brian Krabach (bkrabach) merged 1 commit into
mainfrom
fix/hook-emit-json-safe-model-dump

Conversation

@colombod

Copy link
Copy Markdown
Contributor

Summary

  • Symptom: Orchestrator crashes with TypeError: Object of type Decimal is not JSON serializable on the first LLM response, in PyHookRegistry::emit(). The crash fires before any Python hook handler gets control, making it impossible to intercept from the consumer side.
  • Root cause in the hook layer: try_model_dump() in helpers.rs calls bare model_dump()", which returns Python-native types including Decimal(used byUsage.cost_usd). The result is passed directly to json.dumps()with no custom encoder — which cannot handleDecimal`.
  • Why the fix belongs here: The hook emitter is the caller of json.dumps(). It is the correct place to request JSON-safe output. Putting mode="json" here means no consumer or model field ever needs to compensate for the hook system's serialization contract.

What changed

One function, try_model_dump() in bindings/python/src/helpers.rs:

// Before
obj.call_method0("model_dump")

// After
let kwargs = PyDict::new(py);
kwargs.set_item("mode", "json")?;
obj.call_method("model_dump", (), Some(&kwargs))

mode="json" tells Pydantic to emit only JSON-native Python types (str, int, float, list, dict, bool, None) regardless of the field's underlying Python type or whether a @field_serializer is present on the model.

try_model_dump() is a shared helper called by both emit() and emit_and_collect() in hooks.rs — this one-function change covers the entire emit path.

Why this is architecturally correct (not a workaround)

The immediate trigger is amplifier-core 1.5.0 in the Docker worker image (amplifier-cache:python) predating the @field_serializer on Usage.cost_usd added in 1.5.1. But this fix is correct regardless of model version:

  • The hook system should not depend on individual model fields having serializers to be JSON-safe.
  • Any future field of any non-JSON-native type (e.g. UUID, datetime, custom type) in any hook payload would cause the same crash without this fix.
  • This is defence-in-depth at the right layer: the serializer requests what it needs rather than relying on every upstream model to anticipate its requirements.

Test plan

  • Build amplifier-cache:python image after this merges and bump/release amplifier-core
  • Run orchestrator resolver end-to-end — confirm no TypeError: Object of type Decimal is not JSON serializable on LLM response
  • Verify emit() and emit_and_collect() both work correctly (both flow through try_model_dump())
  • Confirm fallback path (non-Pydantic objects) still passes the raw object through unchanged

Rebuilding amplifier-cache:python after this merges will fully resolve the crash for all consumers without any per-consumer workarounds.

Generated with Amplifier

The Rust hook registry calls try_model_dump() on every hook payload then
passes the result directly to json.dumps() with no custom encoder.  Bare
model_dump() returns Python-native types — including Decimal for fields such
as Usage.cost_usd — which json.dumps() cannot handle, producing:

  TypeError: Object of type Decimal is not JSON serializable

Root cause for this crash is the hook emitter itself: it is the caller of
json.dumps(), so it is the right place to request JSON-safe output.
Passing mode="json" tells Pydantic to emit only JSON-native Python types
(str, int, float, list, dict, bool, None) regardless of the field's Python
type or whether a @field_serializer is present on the model.

try_model_dump() is a single shared helper that serves both emit() and
emit_and_collect() in hooks.rs, so this one-function change covers the
entire emit path.

Note: amplifier-core 1.5.1 added a @field_serializer on Usage.cost_usd
which also fixes the immediate symptom, but the Docker worker image is built
from 1.5.0.  This fix is architecturally correct regardless of the model
version — the hook system should not rely on individual field serializers to
be JSON-safe.  Rebuilding amplifier-cache:python after this merges will
resolve the crash for all consumers without any per-consumer workarounds.

Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
@colombod
Diego Colombo (colombod) force-pushed the fix/hook-emit-json-safe-model-dump branch from 0eb30cd to a91b883 Compare May 11, 2026 16:56
@bkrabach
Brian Krabach (bkrabach) merged commit ef97ff4 into main May 18, 2026
6 checks passed
@bkrabach
Brian Krabach (bkrabach) deleted the fix/hook-emit-json-safe-model-dump branch May 18, 2026 01:51
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.

2 participants