Skip to content

[Brook G2] Converted transfer types nested in JSON values. - #368

Closed
moetemp wants to merge 1 commit into
moe/brook/review/basefrom
moe/brook/review/g2-nested-transfer-types
Closed

moetemp wants to merge 1 commit into
moe/brook/review/basefrom
moe/brook/review/g2-nested-transfer-types

Conversation

@moetemp

@moetemp moetemp commented Oct 25, 2026 •

Copy link
Copy Markdown
Owner

What was changed

This PR makes the JSON converter convert transfer type values that are nested in a JSON value, such as a list item or a dataclass field. The extra lookup costs about 3% on a decode of many small dataclasses.

  • A nested proto transfer type is written as its proto JSON.
  • The decode uses the converter of the type hint's class, and passes a generic hint such as NestedValue[int] whole.
  • A dataclass with its own transfer type stays as its fields inside a JSON value, so payloads written since 1.31.0 still decode.

Why?

Today the converter runs only at the top level of a payload, so a nested transfer type value fails to encode. Stream refs (#373) are the first users, since an Activity takes one input object and a ref travels in one of its fields. Big-payload handles may be next. Stream handles and big-payload handles has the discussion.

Part of the Streams review series.

Checklist

  1. How was this tested:
    • test_transfer_type_values_nested_in_json, test_proto_transfer_type_nested_in_json_is_its_proto_json and test_transfer_type_dataclass_in_a_top_level_list_keeps_its_fields in tests/test_converter.py.

The JSON converter used a `transfer_type_convertible` converter only at
the top level of a payload, so such a value in a list or a dataclass
field failed to encode. A nested dataclass is still written as fields.
The lookup runs after the primitive checks, so it doesn't slow them.
@moetemp

moetemp commented Nov 10, 2026

Copy link
Copy Markdown
Owner Author

Closed in favor of #381, the combined branch, which has this change.

@moetemp moetemp closed this Nov 10, 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