Skip to content

[Brook] Gave StreamRef the proto JSON form inside pydantic models. - #338

Closed
moetemp wants to merge 1 commit into
moe/brook/v3/p15-ref-ownerfrom
moe/brook/v3/p16-ref-pydantic
Closed

moetemp wants to merge 1 commit into
moe/brook/v3/p15-ref-ownerfrom
moe/brook/v3/p16-ref-pydantic

Conversation

@moetemp

@moetemp moetemp commented Oct 21, 2026

Copy link
Copy Markdown
Owner

What changed?

This PR gives StreamRef a pydantic core schema (__get_pydantic_core_schema__). A pydantic model that holds a StreamRef now dumps id, workflowId and firstExecutionRunId, the proto JSON names, as the payload converter already does. It loads both the proto names and snake case. Pydantic is imported only when a model uses the ref, so it stays optional.

Why?

The harness keeps a StreamRef inside a pydantic model, and pydantic wrote it with snake case keys. So the harness had to add its own aliased model.

How did you test it?

pytest tests/contrib/streams/test_pydantic.py. The tests cover:

  • model dump and load, in both key forms;
  • the pydantic_data_converter payload;
  • a Workflow round trip on the memory Store, with pydantic_data_converter and a model that carries the ref.

Full poe lint and the full suite pass at this layer, except test_envconfig, which reads a config file on this host.

  • Unit Tests
  • Staging
  • End to End Tests

The pydantic converter wrote a StreamRef in a model with snake case keys, so the harness needed its own aliased model. A pydantic core schema on StreamRef dumps the proto JSON names and loads both forms, with pydantic imported only when a model uses the ref.
@moetemp

moetemp commented Oct 25, 2026

Copy link
Copy Markdown
Owner Author

Closing as stale. This layer is folded into the v5 tip, #365, and the upstream review uses a fresh series instead. Plan: https://www.notion.so/4048fc567738814990b7c796ac28fe3f

@moetemp moetemp closed this Oct 25, 2026
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.

1 participant