From 2e27140463e6b1ee4530f25508a5d2a6d51d4f19 Mon Sep 17 00:00:00 2001 From: "Gareth.Y" <91742523+linjiaye929@users.noreply.github.com> Date: Sun, 11 Oct 2026 23:32:02 -0700 Subject: [PATCH 1/5] fix: preserve nested attachments in dataclass fields --- py/src/braintrust/bt_json.py | 17 +++++-- py/src/braintrust/test_bt_json.py | 75 +++++++++++++++++++++++++++++++ py/src/braintrust/test_logger.py | 22 +++++++++ 3 files changed, 110 insertions(+), 4 deletions(-) diff --git a/py/src/braintrust/bt_json.py b/py/src/braintrust/bt_json.py index e6035d6bd..36125d8f9 100644 --- a/py/src/braintrust/bt_json.py +++ b/py/src/braintrust/bt_json.py @@ -59,10 +59,7 @@ def _to_bt_safe(v: Any) -> Any: return v.reference if dataclasses.is_dataclass(v) and not isinstance(v, type): - # Use manual field iteration instead of dataclasses.asdict() because - # asdict() deep-copies values, which breaks objects like Attachment - # that contain non-copyable items (thread locks, file handles, etc.) - return {f.name: _to_bt_safe(getattr(v, f.name)) for f in dataclasses.fields(v)} + return bt_safe_deep_copy(v) # Pydantic model classes (not instances) with model_json_schema if isinstance(v, type) and hasattr(v, "model_json_schema") and callable(cast(Any, v).model_json_schema): @@ -216,6 +213,18 @@ def _deep_copy_object(v: Any, depth: int = 0) -> Any: visited_discard(obj_id) try: + if dataclasses.is_dataclass(v) and not isinstance(v, type): + obj_id = id(v) + if obj_id in visited: + return "" + visited_add(obj_id) + try: + # Share the container traversal so nested attachments retain + # their identity and dataclass fields honor cycle/depth limits. + # dataclasses.asdict() would try to deepcopy attachment locks. + return {f.name: _deep_copy_object(getattr(v, f.name), depth + 1) for f in dataclasses.fields(v)} + finally: + visited_discard(obj_id) return _to_bt_safe(v) except Exception: return f"" diff --git a/py/src/braintrust/test_bt_json.py b/py/src/braintrust/test_bt_json.py index a1d4b7118..e3babc85b 100644 --- a/py/src/braintrust/test_bt_json.py +++ b/py/src/braintrust/test_bt_json.py @@ -6,11 +6,86 @@ from typing import Any from unittest import TestCase +import pytest from braintrust.bt_json import bt_dumps, bt_safe_deep_copy from braintrust.logger import Attachment, ExternalAttachment from braintrust.test_helpers import with_memory_logger # noqa: F401 +@pytest.mark.parametrize("shape", ["direct", "list", "dict", "tuple", "dataclass", "dataclass-in-list"]) +def test_dataclass_nested_attachments(shape): + from dataclasses import dataclass + + @dataclass + class Payload: + value: Any + + attachment = Attachment(data=b"data", filename="file.txt", content_type="text/plain") + original, expected = { + "direct": (attachment, attachment), + "list": ([attachment], [attachment]), + "dict": ({"file": attachment}, {"file": attachment}), + "tuple": ((attachment,), [attachment]), + "dataclass": (Payload(attachment), {"value": attachment}), + "dataclass-in-list": ([Payload(attachment)], [{"value": attachment}]), + }[shape] + + assert bt_safe_deep_copy(Payload(original)) == {"value": expected} + + +def test_dataclass_container_cycles_and_shared_values(): + from dataclasses import dataclass + + @dataclass + class Payload: + value: Any + + attachment = Attachment(data=b"data", filename="file.txt", content_type="text/plain") + payload = Payload([attachment]) + payload.value.append(payload) + + assert bt_safe_deep_copy([payload, payload]) == [ + {"value": [attachment, ""]}, + {"value": [attachment, ""]}, + ] + + +def test_dataclass_container_depth_limit(): + from dataclasses import dataclass + + @dataclass + class Payload: + value: Any + + assert bt_safe_deep_copy(Payload([Payload("leaf")]), max_depth=2) == {"value": [""]} + + +def test_dataclass_json_container_fields(): + from dataclasses import dataclass + + @dataclass + class Payload: + value: Any + + payload = Payload({"items": (1, None, "text"), "nested": {"enabled": True}}) + assert json.loads(bt_dumps(payload)) == {"value": {"items": [1, None, "text"], "nested": {"enabled": True}}} + + +def test_dataclass_unreadable_field_is_non_sanitizable(): + from dataclasses import dataclass + + @dataclass + class Payload: + value: int + + def __getattribute__(self, name): + if name == "value": + raise RuntimeError("Cannot read field") + return super().__getattribute__(name) + + assert bt_safe_deep_copy(Payload(1)) == "" + + class TestBTJson(TestCase): def testdeep_copy_event_basic(self): original = { diff --git a/py/src/braintrust/test_logger.py b/py/src/braintrust/test_logger.py index 19dc3add3..4bc0acf8f 100644 --- a/py/src/braintrust/test_logger.py +++ b/py/src/braintrust/test_logger.py @@ -3426,6 +3426,28 @@ def test_attachment_upload_tracked_on_flush(with_memory_logger, with_simulate_lo assert with_memory_logger.upload_attempts[0] is attachment +@pytest.mark.parametrize("field", ["input", "output"]) +def test_dataclass_nested_attachments_reach_flush(field, with_memory_logger, with_simulate_login): + from dataclasses import dataclass + + @dataclass + class Payload: + files: list[BaseAttachment] + + attachment = Attachment(data=b"data", filename="file.txt", content_type="text/plain") + external = ExternalAttachment(url="s3://bucket/key", filename="file.pdf", content_type="application/pdf") + payload = Payload([attachment, external]) + test_logger = init_test_logger(__name__) + with test_logger.start_span(name="dataclass-attachments") as span: + span.log(**{field: payload}) + test_logger.flush() + + # The memory logger exercises extraction without making upload requests. + assert with_memory_logger.upload_attempts == [attachment, external] + assert with_memory_logger.pop()[0][field] == {"files": [attachment.reference, external.reference]} + assert payload.files == [attachment, external] + + def test_same_attachment_logged_twice_tracked_twice(with_memory_logger, with_simulate_login): """Test that same attachment logged twice appears twice in upload attempts.""" attachment = Attachment(data=b"data", filename="file.txt", content_type="text/plain") From 30c6cbebf7a18933b6c3ee3a47ff8db7d3bb7afc Mon Sep 17 00:00:00 2001 From: "Gareth.Y" <91742523+linjiaye929@users.noreply.github.com> Date: Mon, 12 Oct 2026 04:35:08 -0700 Subject: [PATCH 2/5] fix: preserve dataclass attachment subclasses --- py/src/braintrust/bt_json.py | 6 ++++ py/src/braintrust/test_bt_json.py | 60 +++++++++++++++++++++++++++++++ py/src/braintrust/test_logger.py | 46 ++++++++++++++++++++++++ 3 files changed, 112 insertions(+) diff --git a/py/src/braintrust/bt_json.py b/py/src/braintrust/bt_json.py index 36125d8f9..0d551b9f0 100644 --- a/py/src/braintrust/bt_json.py +++ b/py/src/braintrust/bt_json.py @@ -214,6 +214,12 @@ def _deep_copy_object(v: Any, depth: int = 0) -> Any: try: if dataclasses.is_dataclass(v) and not isinstance(v, type): + # Attachment subclasses may also be dataclasses. Keep their + # specialized handling instead of copying their fields. + from braintrust.logger import BaseAttachment, ReadonlyAttachment + + if isinstance(v, (BaseAttachment, ReadonlyAttachment)): + return _to_bt_safe(v) obj_id = id(v) if obj_id in visited: return "" diff --git a/py/src/braintrust/test_bt_json.py b/py/src/braintrust/test_bt_json.py index e3babc85b..c35c86ae7 100644 --- a/py/src/braintrust/test_bt_json.py +++ b/py/src/braintrust/test_bt_json.py @@ -33,6 +33,66 @@ class Payload: assert bt_safe_deep_copy(Payload(original)) == {"value": expected} +@pytest.mark.parametrize("kind", ["base", "readonly"]) +@pytest.mark.parametrize("shape", ["direct", "list", "dict", "dataclass", "dataclass-in-list"]) +def test_dataclass_attachment_types_keep_special_handling(kind, shape): + from dataclasses import dataclass + + from braintrust.logger import BaseAttachment, ReadonlyAttachment + + @dataclass + class CustomAttachment(BaseAttachment): + key: str + + @property + def reference(self): + return { + "type": "braintrust_attachment", + "key": self.key, + "filename": "file.txt", + "content_type": "text/plain", + } + + @property + def data(self): + return b"data" + + def upload(self): + raise AssertionError("Serialization must not upload attachments") + + def debug_info(self): + return {"key": self.key} + + @dataclass + class CustomReadonlyAttachment(ReadonlyAttachment): + reference: dict[str, Any] + + def __post_init__(self): + super().__init__(self.reference) + + @dataclass + class Payload: + value: Any + + attachment = CustomAttachment("test-key") + if kind == "readonly": + attachment = CustomReadonlyAttachment(attachment.reference) + expected = attachment if kind == "base" else attachment.reference + original, expected = { + "direct": (attachment, expected), + "list": ([attachment], [expected]), + "dict": ({"file": attachment}, {"file": expected}), + "dataclass": (Payload(attachment), {"value": expected}), + "dataclass-in-list": (Payload([attachment]), {"value": [expected]}), + }[shape] + + copied = bt_safe_deep_copy(original) + assert copied == expected + if shape == "direct" and kind == "base": + assert copied is attachment + assert json.loads(bt_dumps(copied)) == json.loads(bt_dumps(expected)) + + def test_dataclass_container_cycles_and_shared_values(): from dataclasses import dataclass diff --git a/py/src/braintrust/test_logger.py b/py/src/braintrust/test_logger.py index 4bc0acf8f..1697c5c51 100644 --- a/py/src/braintrust/test_logger.py +++ b/py/src/braintrust/test_logger.py @@ -3448,6 +3448,52 @@ class Payload: assert payload.files == [attachment, external] +@pytest.mark.parametrize("field", ["input", "output"]) +@pytest.mark.parametrize("nested", [False, True]) +def test_dataclass_base_attachment_reaches_flush(field, nested, with_memory_logger, with_simulate_login): + from dataclasses import dataclass + + @dataclass + class CustomAttachment(BaseAttachment): + key: str + + @property + def reference(self): + return { + "type": "braintrust_attachment", + "key": self.key, + "filename": "file.txt", + "content_type": "text/plain", + } + + @property + def data(self): + return b"data" + + def upload(self): + raise AssertionError("The memory logger must not perform real uploads") + + def debug_info(self): + return {"key": self.key} + + @dataclass + class Payload: + files: list[BaseAttachment] + + attachment = CustomAttachment("test-key") + value = Payload([attachment]) if nested else attachment + test_logger = init_test_logger(__name__) + with test_logger.start_span(name="custom-dataclass-attachment") as span: + span.log(**{field: value}) + test_logger.flush() + + # The memory logger verifies extraction and scheduling, without real uploads. + assert len(with_memory_logger.upload_attempts) == 1 + assert with_memory_logger.upload_attempts[0] is attachment + expected = {"files": [attachment.reference]} if nested else attachment.reference + assert with_memory_logger.pop()[0][field] == expected + + def test_same_attachment_logged_twice_tracked_twice(with_memory_logger, with_simulate_login): """Test that same attachment logged twice appears twice in upload attempts.""" attachment = Attachment(data=b"data", filename="file.txt", content_type="text/plain") From 840147abe7ebcccc501fbb34134b089a494d1dd6 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 14 Oct 2026 20:31:21 +0000 Subject: [PATCH 3/5] fix(bt_json): keep SDK object placeholders for dataclass subclasses Route dataclasses in bt_safe_deep_copy through _to_bt_safe so the Span, Experiment, Dataset, Logger, and attachment checks run before fields are copied. The walker passes its own field copier, so nested values keep sharing its cycle and depth tracking instead of starting a new copy. Copy dataclass fields one at a time so a single unreadable field no longer drops the readable fields and their attachments. Share one cycle guard across the walker's branches, cache the lazy braintrust.logger import, and move the dataclass attachment fixture into test_helpers. Co-Authored-By: Claude Opus 5.5 --- py/src/braintrust/bt_json.py | 121 ++++++++++++------------------ py/src/braintrust/test_bt_json.py | 72 ++++++++++-------- py/src/braintrust/test_helpers.py | 29 ++++++- py/src/braintrust/test_logger.py | 26 +------ 4 files changed, 120 insertions(+), 128 deletions(-) diff --git a/py/src/braintrust/bt_json.py b/py/src/braintrust/bt_json.py index 0d551b9f0..263641457 100644 --- a/py/src/braintrust/bt_json.py +++ b/py/src/braintrust/bt_json.py @@ -3,6 +3,7 @@ import math import warnings from collections.abc import Callable, Mapping +from functools import cache, partial from typing import Any, NamedTuple, cast, overload @@ -16,9 +17,20 @@ _HAS_ORJSON = False -def _to_bt_safe(v: Any) -> Any: +@cache +def _logger_types() -> tuple[type, ...]: + # Imported lazily to avoid circular imports, and cached because `_to_bt_safe` runs per logged value. + from braintrust.logger import BaseAttachment, Dataset, Experiment, Logger, ReadonlyAttachment, Span + + return Span, Experiment, Dataset, Logger, BaseAttachment, ReadonlyAttachment + + +def _to_bt_safe(v: Any, copy_dataclass_field: Callable[[Any], Any] | None = None) -> Any: """ Converts the object to a Braintrust-safe representation (i.e. Attachment objects are safe (specially handled by background logger)). + + `copy_dataclass_field` converts dataclass field values. `bt_safe_deep_copy` passes its own walker so + fields share its cycle and depth tracking; without it, the dataclass starts a new `bt_safe_deep_copy`. """ # Fast path: check primitives via type identity before hitting # isinstance checks against abstract classes or Pydantic model_dump. @@ -37,8 +49,7 @@ def _to_bt_safe(v: Any) -> Any: if isinstance(v, (int, str)): return v - # avoid circular imports - from braintrust.logger import BaseAttachment, Dataset, Experiment, Logger, ReadonlyAttachment, Span + Span, Experiment, Dataset, Logger, BaseAttachment, ReadonlyAttachment = _logger_types() if isinstance(v, Span): return "" @@ -59,7 +70,9 @@ def _to_bt_safe(v: Any) -> Any: return v.reference if dataclasses.is_dataclass(v) and not isinstance(v, type): - return bt_safe_deep_copy(v) + if copy_dataclass_field is None: + return bt_safe_deep_copy(v) + return _dataclass_to_dict(v, copy_dataclass_field) # Pydantic model classes (not instances) with model_json_schema if isinstance(v, type) and hasattr(v, "model_json_schema") and callable(cast(Any, v).model_json_schema): @@ -97,6 +110,20 @@ def _to_bt_safe(v: Any) -> Any: return bt_loads(bt_dumps(v, encoder=_str_encoder)) +def _dataclass_to_dict(v: Any, copy_field: Callable[[Any], Any]) -> dict[str, Any]: + # Use manual field iteration instead of dataclasses.asdict() because + # asdict() deep-copies values, which breaks objects like Attachment + # that contain non-copyable items (thread locks, file handles, etc.) + result = {} + for f in dataclasses.fields(v): + try: + result[f.name] = copy_field(getattr(v, f.name)) + except Exception: + # Keep the other fields (and any attachments in them) when one can't be read. + result[f.name] = f"" + return result + + @overload def bt_safe_deep_copy( obj: Mapping[str, Any], @@ -152,43 +179,14 @@ def _deep_copy_object(v: Any, depth: int = 0) -> Any: if depth >= max_depth: return "" - # Fast path for dict (the most common container in log data). - # Uses type identity instead of isinstance(v, Mapping) which is slow. - if tv is dict: - obj_id = id(v) - if obj_id in visited: - return "" - visited_add(obj_id) - try: - result = {} - for k in v: - if type(k) is str: - key_str = k - else: - try: - key_str = str(k) - except Exception: - key_str = f"" - result[key_str] = _deep_copy_object(v[k], depth + 1) - return result - finally: - visited_discard(obj_id) - elif tv is list or tv is tuple: - obj_id = id(v) - if obj_id in visited: - return "" - visited_add(obj_id) - try: - return [_deep_copy_object(x, depth + 1) for x in v] - finally: - visited_discard(obj_id) - # Slow path for non-builtin Mapping/set types. - elif isinstance(v, Mapping): - obj_id = id(v) - if obj_id in visited: - return "" - visited_add(obj_id) - try: + obj_id = id(v) + if obj_id in visited: + return "" + visited_add(obj_id) + try: + # Fast paths for builtin containers (the most common values in log data). + # Type identity checks avoid the slow isinstance(v, Mapping) for them. + if tv is dict or (tv is not list and tv is not tuple and isinstance(v, Mapping)): result = {} for k in v: if type(k) is str: @@ -200,40 +198,17 @@ def _deep_copy_object(v: Any, depth: int = 0) -> Any: key_str = f"" result[key_str] = _deep_copy_object(v[k], depth + 1) return result - finally: - visited_discard(obj_id) - elif isinstance(v, set): - obj_id = id(v) - if obj_id in visited: - return "" - visited_add(obj_id) - try: + if tv is list or tv is tuple or isinstance(v, set): return [_deep_copy_object(x, depth + 1) for x in v] - finally: - visited_discard(obj_id) - try: - if dataclasses.is_dataclass(v) and not isinstance(v, type): - # Attachment subclasses may also be dataclasses. Keep their - # specialized handling instead of copying their fields. - from braintrust.logger import BaseAttachment, ReadonlyAttachment - - if isinstance(v, (BaseAttachment, ReadonlyAttachment)): - return _to_bt_safe(v) - obj_id = id(v) - if obj_id in visited: - return "" - visited_add(obj_id) - try: - # Share the container traversal so nested attachments retain - # their identity and dataclass fields honor cycle/depth limits. - # dataclasses.asdict() would try to deepcopy attachment locks. - return {f.name: _deep_copy_object(getattr(v, f.name), depth + 1) for f in dataclasses.fields(v)} - finally: - visited_discard(obj_id) - return _to_bt_safe(v) - except Exception: - return f"" + try: + # Dataclass fields go back through this walker so nested attachments keep + # their identity and share its cycle/depth tracking. + return _to_bt_safe(v, partial(_deep_copy_object, depth=depth + 1)) + except Exception: + return f"" + finally: + visited_discard(obj_id) return _deep_copy_object(obj) diff --git a/py/src/braintrust/test_bt_json.py b/py/src/braintrust/test_bt_json.py index c35c86ae7..2eb844bca 100644 --- a/py/src/braintrust/test_bt_json.py +++ b/py/src/braintrust/test_bt_json.py @@ -9,7 +9,7 @@ import pytest from braintrust.bt_json import bt_dumps, bt_safe_deep_copy from braintrust.logger import Attachment, ExternalAttachment -from braintrust.test_helpers import with_memory_logger # noqa: F401 +from braintrust.test_helpers import DataclassAttachment, with_memory_logger # noqa: F401 @pytest.mark.parametrize("shape", ["direct", "list", "dict", "tuple", "dataclass", "dataclass-in-list"]) @@ -38,30 +38,7 @@ class Payload: def test_dataclass_attachment_types_keep_special_handling(kind, shape): from dataclasses import dataclass - from braintrust.logger import BaseAttachment, ReadonlyAttachment - - @dataclass - class CustomAttachment(BaseAttachment): - key: str - - @property - def reference(self): - return { - "type": "braintrust_attachment", - "key": self.key, - "filename": "file.txt", - "content_type": "text/plain", - } - - @property - def data(self): - return b"data" - - def upload(self): - raise AssertionError("Serialization must not upload attachments") - - def debug_info(self): - return {"key": self.key} + from braintrust.logger import ReadonlyAttachment @dataclass class CustomReadonlyAttachment(ReadonlyAttachment): @@ -74,7 +51,7 @@ def __post_init__(self): class Payload: value: Any - attachment = CustomAttachment("test-key") + attachment = DataclassAttachment("test-key") if kind == "readonly": attachment = CustomReadonlyAttachment(attachment.reference) expected = attachment if kind == "base" else attachment.reference @@ -131,19 +108,54 @@ class Payload: assert json.loads(bt_dumps(payload)) == {"value": {"items": [1, None, "text"], "nested": {"enabled": True}}} -def test_dataclass_unreadable_field_is_non_sanitizable(): - from dataclasses import dataclass +def test_dataclass_unreadable_field_keeps_readable_fields(): + from dataclasses import dataclass, field @dataclass class Payload: - value: int + files: list[Any] + value: int = 0 + unset: int = field(init=False) def __getattribute__(self, name): if name == "value": raise RuntimeError("Cannot read field") return super().__getattribute__(name) - assert bt_safe_deep_copy(Payload(1)) == "" + attachment = Attachment(data=b"data", filename="file.txt", content_type="text/plain") + + assert bt_safe_deep_copy(Payload([attachment])) == { + "files": [attachment], + "value": "", + "unset": "", + } + + +@pytest.mark.parametrize("nested", [False, True]) +@pytest.mark.parametrize( + "base_name,placeholder", + [("_NoopSpan", ""), ("Experiment", ""), ("Dataset", ""), ("Logger", "")], +) +def test_dataclass_logger_object_subclass_keeps_placeholder(base_name, placeholder, nested): + from dataclasses import dataclass, make_dataclass + + from braintrust import logger + + # Field values of these SDK objects must never be copied into logged events. + DataclassObject = make_dataclass( + "DataclassObject", [("secret", str, "token")], bases=(getattr(logger, base_name),) + ) + + @dataclass + class Payload: + value: Any + + obj = DataclassObject() + original = Payload([obj]) if nested else obj + expected = {"value": [placeholder]} if nested else placeholder + + assert bt_safe_deep_copy(original) == expected + assert json.loads(bt_dumps(original)) == expected class TestBTJson(TestCase): diff --git a/py/src/braintrust/test_helpers.py b/py/src/braintrust/test_helpers.py index 8b0086b77..f592677a8 100644 --- a/py/src/braintrust/test_helpers.py +++ b/py/src/braintrust/test_helpers.py @@ -1,9 +1,10 @@ +import dataclasses import os from contextlib import contextmanager import pytest from braintrust import logger -from braintrust.logger import ObjectMetadata, OrgProjectMetadata, ProjectExperimentMetadata +from braintrust.logger import BaseAttachment, ObjectMetadata, OrgProjectMetadata, ProjectExperimentMetadata from braintrust.util import LazyValue @@ -129,6 +130,32 @@ def find_span_by_name(spans, name): raise AssertionError(f"Expected span named {name!r}. Available spans: {available_names}") +@dataclasses.dataclass +class DataclassAttachment(BaseAttachment): + """A `BaseAttachment` subclass that is also a dataclass, for serialization tests.""" + + key: str + + @property + def reference(self): + return { + "type": "braintrust_attachment", + "key": self.key, + "filename": "file.txt", + "content_type": "text/plain", + } + + @property + def data(self): + return b"data" + + def upload(self): + raise AssertionError("Tests must not upload attachments") + + def debug_info(self): + return {"key": self.key} + + def init_test_exp(experiment_name: str, project_name: str = None): """ Initialize an experiment for testing with fake project and experiment metadata. diff --git a/py/src/braintrust/test_logger.py b/py/src/braintrust/test_logger.py index 1697c5c51..d7455d769 100644 --- a/py/src/braintrust/test_logger.py +++ b/py/src/braintrust/test_logger.py @@ -48,6 +48,7 @@ from braintrust.prompt_cache.parameters_cache import ParametersCache from braintrust.prompt_cache.prompt_cache import PromptCache from braintrust.test_helpers import ( + DataclassAttachment, assert_dict_matches, assert_logged_out, init_test_exp, @@ -3453,34 +3454,11 @@ class Payload: def test_dataclass_base_attachment_reaches_flush(field, nested, with_memory_logger, with_simulate_login): from dataclasses import dataclass - @dataclass - class CustomAttachment(BaseAttachment): - key: str - - @property - def reference(self): - return { - "type": "braintrust_attachment", - "key": self.key, - "filename": "file.txt", - "content_type": "text/plain", - } - - @property - def data(self): - return b"data" - - def upload(self): - raise AssertionError("The memory logger must not perform real uploads") - - def debug_info(self): - return {"key": self.key} - @dataclass class Payload: files: list[BaseAttachment] - attachment = CustomAttachment("test-key") + attachment = DataclassAttachment("test-key") value = Payload([attachment]) if nested else attachment test_logger = init_test_logger(__name__) with test_logger.start_span(name="custom-dataclass-attachment") as span: From a715ce384bf75991dcce809103ade3a58e79730d Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 14 Oct 2026 20:36:33 +0000 Subject: [PATCH 4/5] test(bt_json): consolidate dataclass serialization tests Fold the per-shape and per-type parametrizations into one case per behavior: nested attachments, SDK object special handling, shared cycle/depth tracking, and unreadable fields. Merge the two logger flush tests and drop the input/output parametrization, which exercised the same path. Remove the JSON-container test and the older dataclass attachment test, both covered by the remaining tests. Co-Authored-By: Claude Opus 5.5 --- py/src/braintrust/test_bt_json.py | 199 +++++++++--------------------- py/src/braintrust/test_logger.py | 35 ++---- 2 files changed, 67 insertions(+), 167 deletions(-) diff --git a/py/src/braintrust/test_bt_json.py b/py/src/braintrust/test_bt_json.py index 2eb844bca..0890c7929 100644 --- a/py/src/braintrust/test_bt_json.py +++ b/py/src/braintrust/test_bt_json.py @@ -1,6 +1,7 @@ # pyright: reportUnknownVariableType=false # pyright: reportUnknownArgumentType=false # pyright: reportPrivateUsage=false +import dataclasses import json import warnings from typing import Any @@ -8,114 +9,91 @@ import pytest from braintrust.bt_json import bt_dumps, bt_safe_deep_copy -from braintrust.logger import Attachment, ExternalAttachment +from braintrust.logger import Attachment, ExternalAttachment, ReadonlyAttachment from braintrust.test_helpers import DataclassAttachment, with_memory_logger # noqa: F401 -@pytest.mark.parametrize("shape", ["direct", "list", "dict", "tuple", "dataclass", "dataclass-in-list"]) -def test_dataclass_nested_attachments(shape): - from dataclasses import dataclass +@dataclasses.dataclass +class _Payload: + value: Any - @dataclass - class Payload: - value: Any - attachment = Attachment(data=b"data", filename="file.txt", content_type="text/plain") - original, expected = { - "direct": (attachment, attachment), - "list": ([attachment], [attachment]), - "dict": ({"file": attachment}, {"file": attachment}), - "tuple": ((attachment,), [attachment]), - "dataclass": (Payload(attachment), {"value": attachment}), - "dataclass-in-list": ([Payload(attachment)], [{"value": attachment}]), - }[shape] +@dataclasses.dataclass +class _DataclassReadonlyAttachment(ReadonlyAttachment): + reference: dict[str, Any] + + def __post_init__(self): + super().__init__(self.reference) - assert bt_safe_deep_copy(Payload(original)) == {"value": expected} +def test_dataclass_nested_attachments(): + attachment = Attachment(data=b"data", filename="file.txt", content_type="text/plain") + payload = _Payload( + { + "direct": attachment, + "list": [attachment], + "tuple": (attachment,), + "dataclass": _Payload(attachment), + "dataclasses": [_Payload(attachment)], + } + ) -@pytest.mark.parametrize("kind", ["base", "readonly"]) -@pytest.mark.parametrize("shape", ["direct", "list", "dict", "dataclass", "dataclass-in-list"]) -def test_dataclass_attachment_types_keep_special_handling(kind, shape): - from dataclasses import dataclass + assert bt_safe_deep_copy(payload) == { + "value": { + "direct": attachment, + "list": [attachment], + "tuple": [attachment], + "dataclass": {"value": attachment}, + "dataclasses": [{"value": attachment}], + } + } - from braintrust.logger import ReadonlyAttachment - @dataclass - class CustomReadonlyAttachment(ReadonlyAttachment): - reference: dict[str, Any] +@pytest.mark.parametrize("kind", ["span", "experiment", "dataset", "logger", "attachment", "readonly-attachment"]) +def test_dataclass_sdk_objects_keep_special_handling(kind): + from braintrust import logger - def __post_init__(self): - super().__init__(self.reference) + if kind == "attachment": + obj = expected = DataclassAttachment("test-key") + elif kind == "readonly-attachment": + obj = _DataclassReadonlyAttachment(DataclassAttachment("test-key").reference) + expected = obj.reference + else: + base = { + "span": logger._NoopSpan, + "experiment": logger.Experiment, + "dataset": logger.Dataset, + "logger": logger.Logger, + } + # The `secret` field must never be copied into logged events. + obj = dataclasses.make_dataclass("DataclassObject", [("secret", str, "token")], bases=(base[kind],))() + expected = f"<{kind}>" + + assert bt_safe_deep_copy({"direct": obj, "nested": _Payload([obj])}) == { + "direct": expected, + "nested": {"value": [expected]}, + } - @dataclass - class Payload: - value: Any - - attachment = DataclassAttachment("test-key") - if kind == "readonly": - attachment = CustomReadonlyAttachment(attachment.reference) - expected = attachment if kind == "base" else attachment.reference - original, expected = { - "direct": (attachment, expected), - "list": ([attachment], [expected]), - "dict": ({"file": attachment}, {"file": expected}), - "dataclass": (Payload(attachment), {"value": expected}), - "dataclass-in-list": (Payload([attachment]), {"value": [expected]}), - }[shape] - - copied = bt_safe_deep_copy(original) - assert copied == expected - if shape == "direct" and kind == "base": - assert copied is attachment - assert json.loads(bt_dumps(copied)) == json.loads(bt_dumps(expected)) - - -def test_dataclass_container_cycles_and_shared_values(): - from dataclasses import dataclass - - @dataclass - class Payload: - value: Any +def test_dataclass_cycles_and_depth_share_walker_state(): attachment = Attachment(data=b"data", filename="file.txt", content_type="text/plain") - payload = Payload([attachment]) + payload = _Payload([attachment]) payload.value.append(payload) + # Shared (non-circular) references are copied each time; only true cycles are replaced. assert bt_safe_deep_copy([payload, payload]) == [ {"value": [attachment, ""]}, {"value": [attachment, ""]}, ] - - -def test_dataclass_container_depth_limit(): - from dataclasses import dataclass - - @dataclass - class Payload: - value: Any - - assert bt_safe_deep_copy(Payload([Payload("leaf")]), max_depth=2) == {"value": [""]} - - -def test_dataclass_json_container_fields(): - from dataclasses import dataclass - - @dataclass - class Payload: - value: Any - - payload = Payload({"items": (1, None, "text"), "nested": {"enabled": True}}) - assert json.loads(bt_dumps(payload)) == {"value": {"items": [1, None, "text"], "nested": {"enabled": True}}} + assert bt_safe_deep_copy(_Payload([_Payload("leaf")]), max_depth=2) == {"value": [""]} def test_dataclass_unreadable_field_keeps_readable_fields(): - from dataclasses import dataclass, field - - @dataclass + @dataclasses.dataclass class Payload: files: list[Any] value: int = 0 - unset: int = field(init=False) + unset: int = dataclasses.field(init=False) def __getattribute__(self, name): if name == "value": @@ -131,33 +109,6 @@ def __getattribute__(self, name): } -@pytest.mark.parametrize("nested", [False, True]) -@pytest.mark.parametrize( - "base_name,placeholder", - [("_NoopSpan", ""), ("Experiment", ""), ("Dataset", ""), ("Logger", "")], -) -def test_dataclass_logger_object_subclass_keeps_placeholder(base_name, placeholder, nested): - from dataclasses import dataclass, make_dataclass - - from braintrust import logger - - # Field values of these SDK objects must never be copied into logged events. - DataclassObject = make_dataclass( - "DataclassObject", [("secret", str, "token")], bases=(getattr(logger, base_name),) - ) - - @dataclass - class Payload: - value: Any - - obj = DataclassObject() - original = Payload([obj]) if nested else obj - expected = {"value": [placeholder]} if nested else placeholder - - assert bt_safe_deep_copy(original) == expected - assert json.loads(bt_dumps(original)) == expected - - class TestBTJson(TestCase): def testdeep_copy_event_basic(self): original = { @@ -733,36 +684,6 @@ class ModelWithAttachment(BaseModel): except ImportError: self.skipTest("Pydantic not available") - def test_bt_safe_deep_copy_dataclass_with_attachments(self): - """Test that dataclasses with attachments are handled correctly.""" - from dataclasses import dataclass - - attachment = Attachment(data=b"data", filename="file.txt", content_type="text/plain") - - @dataclass - class DataWithAttachment: - name: str - value: int - file: Attachment - - data = DataWithAttachment(name="test", value=42, file=attachment) - - result = bt_safe_deep_copy({"data": data}) - - # Dataclasses with Attachment fields are now properly converted by - # recursively applying _to_bt_safe to each field instead of using - # dataclasses.asdict() which would try to deepcopy the Attachment. - self.assertIsInstance(result["data"], dict) - self.assertEqual(result["data"]["name"], "test") - self.assertEqual(result["data"]["value"], 42) - self.assertIs(result["data"]["file"], attachment) - - # Attachments in regular dicts also work fine - dict_with_attachment = {"name": "test", "value": 42, "file": attachment} - result2 = bt_safe_deep_copy({"data": dict_with_attachment}) - self.assertIsInstance(result2["data"], dict) - self.assertIs(result2["data"]["file"], attachment) - def test_bt_safe_deep_copy_circular_in_pydantic_deferred_to_bt_dumps(self): """Test that circular references inside Pydantic model results bypass bt_safe_deep_copy detection. diff --git a/py/src/braintrust/test_logger.py b/py/src/braintrust/test_logger.py index d7455d769..48e4d4163 100644 --- a/py/src/braintrust/test_logger.py +++ b/py/src/braintrust/test_logger.py @@ -3427,8 +3427,7 @@ def test_attachment_upload_tracked_on_flush(with_memory_logger, with_simulate_lo assert with_memory_logger.upload_attempts[0] is attachment -@pytest.mark.parametrize("field", ["input", "output"]) -def test_dataclass_nested_attachments_reach_flush(field, with_memory_logger, with_simulate_login): +def test_dataclass_attachments_reach_flush(with_memory_logger, with_simulate_login): from dataclasses import dataclass @dataclass @@ -3437,41 +3436,21 @@ class Payload: attachment = Attachment(data=b"data", filename="file.txt", content_type="text/plain") external = ExternalAttachment(url="s3://bucket/key", filename="file.pdf", content_type="application/pdf") + dataclass_attachment = DataclassAttachment("test-key") payload = Payload([attachment, external]) test_logger = init_test_logger(__name__) with test_logger.start_span(name="dataclass-attachments") as span: - span.log(**{field: payload}) + span.log(input=payload, output=dataclass_attachment) test_logger.flush() # The memory logger exercises extraction without making upload requests. - assert with_memory_logger.upload_attempts == [attachment, external] - assert with_memory_logger.pop()[0][field] == {"files": [attachment.reference, external.reference]} + assert with_memory_logger.upload_attempts == [attachment, external, dataclass_attachment] + logged = with_memory_logger.pop()[0] + assert logged["input"] == {"files": [attachment.reference, external.reference]} + assert logged["output"] == dataclass_attachment.reference assert payload.files == [attachment, external] -@pytest.mark.parametrize("field", ["input", "output"]) -@pytest.mark.parametrize("nested", [False, True]) -def test_dataclass_base_attachment_reaches_flush(field, nested, with_memory_logger, with_simulate_login): - from dataclasses import dataclass - - @dataclass - class Payload: - files: list[BaseAttachment] - - attachment = DataclassAttachment("test-key") - value = Payload([attachment]) if nested else attachment - test_logger = init_test_logger(__name__) - with test_logger.start_span(name="custom-dataclass-attachment") as span: - span.log(**{field: value}) - test_logger.flush() - - # The memory logger verifies extraction and scheduling, without real uploads. - assert len(with_memory_logger.upload_attempts) == 1 - assert with_memory_logger.upload_attempts[0] is attachment - expected = {"files": [attachment.reference]} if nested else attachment.reference - assert with_memory_logger.pop()[0][field] == expected - - def test_same_attachment_logged_twice_tracked_twice(with_memory_logger, with_simulate_login): """Test that same attachment logged twice appears twice in upload attempts.""" attachment = Attachment(data=b"data", filename="file.txt", content_type="text/plain") From f42392f52268798fec4405fccebcf5ed73989d6c Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 14 Oct 2026 20:54:08 +0000 Subject: [PATCH 5/5] refactor(bt_json): pass walker depth instead of a per-leaf partial Pass the deep-copy walker and the field depth to `_to_bt_safe` as plain arguments so non-dataclass leaves no longer allocate a `functools.partial`. Return the cached logger types as a NamedTuple and use it in `_to_json_safe` instead of a second lazy import. Split the string-dispatched special-type test into one for SDK object placeholders and one for attachment subclasses. Co-Authored-By: Claude Opus 5.5 --- py/src/braintrust/bt_json.py | 35 +++++++++++++---------- py/src/braintrust/test_bt_json.py | 46 ++++++++++++++++++------------- 2 files changed, 48 insertions(+), 33 deletions(-) diff --git a/py/src/braintrust/bt_json.py b/py/src/braintrust/bt_json.py index 263641457..120e8e4e1 100644 --- a/py/src/braintrust/bt_json.py +++ b/py/src/braintrust/bt_json.py @@ -3,7 +3,7 @@ import math import warnings from collections.abc import Callable, Mapping -from functools import cache, partial +from functools import cache from typing import Any, NamedTuple, cast, overload @@ -17,20 +17,30 @@ _HAS_ORJSON = False +class _LoggerTypes(NamedTuple): + Span: type + Experiment: type + Dataset: type + Logger: type + BaseAttachment: type + ReadonlyAttachment: type + + @cache -def _logger_types() -> tuple[type, ...]: +def _logger_types() -> _LoggerTypes: # Imported lazily to avoid circular imports, and cached because `_to_bt_safe` runs per logged value. from braintrust.logger import BaseAttachment, Dataset, Experiment, Logger, ReadonlyAttachment, Span - return Span, Experiment, Dataset, Logger, BaseAttachment, ReadonlyAttachment + return _LoggerTypes(Span, Experiment, Dataset, Logger, BaseAttachment, ReadonlyAttachment) -def _to_bt_safe(v: Any, copy_dataclass_field: Callable[[Any], Any] | None = None) -> Any: +def _to_bt_safe(v: Any, copy_dataclass_field: Callable[[Any, int], Any] | None = None, depth: int = 0) -> Any: """ Converts the object to a Braintrust-safe representation (i.e. Attachment objects are safe (specially handled by background logger)). - `copy_dataclass_field` converts dataclass field values. `bt_safe_deep_copy` passes its own walker so - fields share its cycle and depth tracking; without it, the dataclass starts a new `bt_safe_deep_copy`. + `copy_dataclass_field(value, depth)` converts dataclass field values. `bt_safe_deep_copy` passes its own + walker and the fields' depth so they share its cycle and depth tracking; without it, the dataclass + starts a new `bt_safe_deep_copy`. """ # Fast path: check primitives via type identity before hitting # isinstance checks against abstract classes or Pydantic model_dump. @@ -72,7 +82,7 @@ def _to_bt_safe(v: Any, copy_dataclass_field: Callable[[Any], Any] | None = None if dataclasses.is_dataclass(v) and not isinstance(v, type): if copy_dataclass_field is None: return bt_safe_deep_copy(v) - return _dataclass_to_dict(v, copy_dataclass_field) + return _dataclass_to_dict(v, copy_dataclass_field, depth) # Pydantic model classes (not instances) with model_json_schema if isinstance(v, type) and hasattr(v, "model_json_schema") and callable(cast(Any, v).model_json_schema): @@ -110,14 +120,14 @@ def _to_bt_safe(v: Any, copy_dataclass_field: Callable[[Any], Any] | None = None return bt_loads(bt_dumps(v, encoder=_str_encoder)) -def _dataclass_to_dict(v: Any, copy_field: Callable[[Any], Any]) -> dict[str, Any]: +def _dataclass_to_dict(v: Any, copy_field: Callable[[Any, int], Any], depth: int) -> dict[str, Any]: # Use manual field iteration instead of dataclasses.asdict() because # asdict() deep-copies values, which breaks objects like Attachment # that contain non-copyable items (thread locks, file handles, etc.) result = {} for f in dataclasses.fields(v): try: - result[f.name] = copy_field(getattr(v, f.name)) + result[f.name] = copy_field(getattr(v, f.name), depth) except Exception: # Keep the other fields (and any attachments in them) when one can't be read. result[f.name] = f"" @@ -204,7 +214,7 @@ def _deep_copy_object(v: Any, depth: int = 0) -> Any: try: # Dataclass fields go back through this walker so nested attachments keep # their identity and share its cycle/depth tracking. - return _to_bt_safe(v, partial(_deep_copy_object, depth=depth + 1)) + return _to_bt_safe(v, _deep_copy_object, depth + 1) except Exception: return f"" finally: @@ -224,15 +234,12 @@ def _to_json_safe(obj: Any) -> Any: """ Handler for non-JSON-serializable objects. Returns a string representation of the object. """ - # avoid circular imports - from braintrust.logger import BaseAttachment - try: v = _to_bt_safe(obj) # JSON-safe representation of Attachment objects are their reference. # If we get this object at this point, we have to assume someone has already uploaded the attachment! - if isinstance(v, BaseAttachment): + if isinstance(v, _logger_types().BaseAttachment): v = v.reference return v diff --git a/py/src/braintrust/test_bt_json.py b/py/src/braintrust/test_bt_json.py index 0890c7929..b8a6821f0 100644 --- a/py/src/braintrust/test_bt_json.py +++ b/py/src/braintrust/test_bt_json.py @@ -49,29 +49,37 @@ def test_dataclass_nested_attachments(): } -@pytest.mark.parametrize("kind", ["span", "experiment", "dataset", "logger", "attachment", "readonly-attachment"]) -def test_dataclass_sdk_objects_keep_special_handling(kind): +@pytest.mark.parametrize( + "base_name,placeholder", + [("_NoopSpan", ""), ("Experiment", ""), ("Dataset", ""), ("Logger", "")], +) +def test_dataclass_sdk_object_subclasses_keep_placeholder(base_name, placeholder): from braintrust import logger - if kind == "attachment": - obj = expected = DataclassAttachment("test-key") - elif kind == "readonly-attachment": - obj = _DataclassReadonlyAttachment(DataclassAttachment("test-key").reference) - expected = obj.reference - else: - base = { - "span": logger._NoopSpan, - "experiment": logger.Experiment, - "dataset": logger.Dataset, - "logger": logger.Logger, - } - # The `secret` field must never be copied into logged events. - obj = dataclasses.make_dataclass("DataclassObject", [("secret", str, "token")], bases=(base[kind],))() - expected = f"<{kind}>" + # The `secret` field must never be copied into logged events. + obj = dataclasses.make_dataclass( + "DataclassObject", [("secret", str, "token")], bases=(getattr(logger, base_name),) + )() assert bt_safe_deep_copy({"direct": obj, "nested": _Payload([obj])}) == { - "direct": expected, - "nested": {"value": [expected]}, + "direct": placeholder, + "nested": {"value": [placeholder]}, + } + + +def test_dataclass_attachment_subclasses_keep_special_handling(): + attachment = DataclassAttachment("test-key") + reference = { + "type": "braintrust_attachment", + "key": "readonly-key", + "filename": "a.txt", + "content_type": "text/plain", + } + readonly = _DataclassReadonlyAttachment(reference) + + assert bt_safe_deep_copy({"direct": [attachment, readonly], "nested": _Payload([attachment, readonly])}) == { + "direct": [attachment, reference], + "nested": {"value": [attachment, reference]}, }