diff --git a/packages/ai/tests/test_reexports.py b/packages/ai/tests/test_reexports.py index de164279..97888ec8 100644 --- a/packages/ai/tests/test_reexports.py +++ b/packages/ai/tests/test_reexports.py @@ -1,6 +1,5 @@ """ -Tests for §5 launchdarkly-ai-python re-export barrel. -Reference: TESTING.md §5 +Tests for the launchdarkly-ai-python re-export barrel. """ import importlib diff --git a/packages/claude-agents/src/launchdarkly_ai_claude_agents/handler.py b/packages/claude-agents/src/launchdarkly_ai_claude_agents/handler.py index 32ee9fd6..67ac64f9 100644 --- a/packages/claude-agents/src/launchdarkly_ai_claude_agents/handler.py +++ b/packages/claude-agents/src/launchdarkly_ai_claude_agents/handler.py @@ -78,9 +78,9 @@ async def build_tool_mcp( """Build an in-process SDK MCP server from LD config tools + handler functions. Imports the SDK lazily rather than off the module-level ``claude_agent_sdk`` import that - ``query``/``ClaudeAgentOptions``/etc. use: ``native_graph.py`` (out of scope for this telemetry - pass) calls this function too, and its own tests mock the SDK by patching - ``importlib.import_module`` rather than this module's names. + ``query``/``ClaudeAgentOptions``/etc. use: ``native_graph.py`` calls this function too, and + its own tests mock the SDK by patching ``importlib.import_module`` rather than this module's + names. """ import importlib @@ -119,9 +119,8 @@ def partition_tools( user_config_tools: LD tool definitions for user-defined tools (sent via MCP) native_tool_names: provider-facing names for query(options.tools=[...]) - Kept at a 3-tuple return for backward compatibility: ``native_graph.py`` (out of scope for this - telemetry pass) unpacks this directly. See :func:`_native_tool_aliases` for the fourth mapping - the span work needs. + Returns a 3-tuple because ``native_graph.py`` unpacks it directly. See + :func:`_native_tool_aliases` for the fourth mapping span recording needs. """ native_tool_map: dict[str, Any] = {} user_config_tools: dict[str, Any] = {} @@ -157,10 +156,10 @@ def _is_coroutine(fn: Any) -> bool: def _build_hooks(native_tool_map: dict[str, Any]) -> dict[str, Any] | None: - """The pre-span-work hook set: tracks a native tool call for telemetry purposes only. + """A span-free hook set: tracks a native tool call for telemetry purposes only. - Kept for ``native_graph.py`` (out of scope for this telemetry pass), which imports this name - directly and does not build ``execute_tool`` spans of its own. See :func:`build_tool_hooks` for + Used by ``native_graph.py``, which imports this name directly and does not build + ``execute_tool`` spans of its own. See :func:`build_tool_hooks` for the span-aware hook set this handler's own ``_call_impl``/``_stream_gen`` use. """ if not native_tool_map: diff --git a/packages/claude-agents/src/launchdarkly_ai_claude_agents/native_graph.py b/packages/claude-agents/src/launchdarkly_ai_claude_agents/native_graph.py index b0ffadb9..b693cd20 100644 --- a/packages/claude-agents/src/launchdarkly_ai_claude_agents/native_graph.py +++ b/packages/claude-agents/src/launchdarkly_ai_claude_agents/native_graph.py @@ -174,7 +174,7 @@ async def _run_query( # Hold an explicit reference so we can call aclose() in the finally block. # Bare `return` inside `async for` abandons the generator — Python's asyncio # finalizer later tries to aclose() it and may raise RuntimeError if the - # generator is suspended inside a real await in the SDK (AIC-2950). + # generator is suspended inside a real await in the SDK. gen = query_fn(prompt=query_prompt, options=options) try: async for message in gen: diff --git a/packages/claude-agents/tests/test_builtins.py b/packages/claude-agents/tests/test_builtins.py index 6ec8599a..2e3d68fd 100644 --- a/packages/claude-agents/tests/test_builtins.py +++ b/packages/claude-agents/tests/test_builtins.py @@ -1,6 +1,5 @@ """ -Tests for §4 Claude Agents built-ins (builtins.py). -Reference: TESTING.md §4 +Tests for Claude Agents built-ins (builtins.py). """ from launchdarkly_ai_claude_agents.builtins import ( diff --git a/packages/claude-agents/tests/test_graph.py b/packages/claude-agents/tests/test_graph.py index be7ff327..387c8ed3 100644 --- a/packages/claude-agents/tests/test_graph.py +++ b/packages/claude-agents/tests/test_graph.py @@ -1,6 +1,5 @@ """ -Tests for §2.1 graph convenience wrapper (claude_graph). -Reference: TESTING.md §2.1 +Tests for the graph convenience wrapper (claude_graph). """ from unittest.mock import MagicMock, patch diff --git a/packages/claude-agents/tests/test_handler.py b/packages/claude-agents/tests/test_handler.py index 5651391e..19fb3630 100644 --- a/packages/claude-agents/tests/test_handler.py +++ b/packages/claude-agents/tests/test_handler.py @@ -1,7 +1,7 @@ """ Tests for launchdarkly-ai-claude-agents handler. -Rewritten against TELEMETRY-CONTRACT.md, replacing the old flat-span assertions. Uses a real +Span assertions follow TELEMETRY-CONTRACT.md. Uses a real ``TracerProvider`` + ``InMemorySpanExporter`` rather than a mocked ``opentelemetry.trace`` module, the same choice ``@launchdarkly/ai-claude-agents``'s ``spans.test.ts`` makes: a mocked tracer cannot see whether parent/child wiring is right, only whether the right methods were called. @@ -84,8 +84,8 @@ def root() -> Any: def _make_config(**kwargs: Any) -> dict[str, Any]: - """Restored from the pre-rewrite file: a couple of the restored non-telemetry tests build a - config inline rather than through ``BASE_CONFIG``/``TOOL_CONFIG``. + """For the non-telemetry tests that build a config inline rather than through + ``BASE_CONFIG``/``TOOL_CONFIG``. """ base = {"model": {"name": "claude-opus-4-5"}, "provider": {"name": "Anthropic"}} base.update(kwargs) @@ -202,7 +202,7 @@ def test_provides_for(self) -> None: def test_multiple_calls_independent(self) -> None: assert create_claude_agents_handler() is not create_claude_agents_handler() - # --- restored from the pre-rewrite file (not telemetry) --- + # --- not telemetry --- def test_attaches_provides_for(self) -> None: h = create_claude_agents_handler() @@ -240,8 +240,7 @@ def test_variable_substitution(self) -> None: _, system = build_prompt(cfg, "hi", {"name": "Ada"}) assert system == "Hello Ada." - # --- restored from the pre-rewrite file (not telemetry); byte-for-byte, only - # ``_make_config`` inlined since the old module-level helper was removed --- + # --- not telemetry --- def test_path_a_instructions(self) -> None: config = _make_config(instructions="You are a helper.") @@ -368,8 +367,7 @@ def test_native_tool_aliases_map_ld_key_to_provider_name(self) -> None: # --------------------------------------------------------------------------- -# §1.3 Tool conversion — restored from the pre-rewrite file (not telemetry). -# ``partition_tools`` kept its 3-tuple return, so these run byte-for-byte. +# Tool conversion (not telemetry). # --------------------------------------------------------------------------- @@ -396,10 +394,9 @@ def test_empty_tools_no_tools_sent(self) -> None: # --------------------------------------------------------------------------- -# §1.4 Tool execution loop (via build_tool_mcp) — restored from the pre-rewrite -# file. ``build_tool_mcp`` kept its lazy ``importlib.import_module`` pattern -# (native_graph.py depends on it), so the old SDK-mocking approach still works -# unmodified for this one. +# Tool execution loop (via build_tool_mcp). ``build_tool_mcp`` resolves the SDK +# lazily through ``importlib.import_module`` (native_graph.py depends on it), so +# this one mocks the SDK module rather than patching ``handler_mod.query``. # --------------------------------------------------------------------------- @@ -415,8 +412,8 @@ def __init__( def _patch_query_for_tool_mcp(messages: list[Any]) -> Any: - """Patches ``claude_agent_sdk`` for the one restored test that exercises - ``build_tool_mcp`` directly, which still resolves the SDK lazily. + """Patches ``claude_agent_sdk`` for the one test that exercises + ``build_tool_mcp`` directly, which resolves the SDK lazily. """ async def _query(**kwargs: Any) -> AsyncIterator[Any]: @@ -1058,11 +1055,10 @@ async def _slow_query(**_kwargs: Any) -> AsyncIterator[Any]: for s in spans(): assert s.end_time is not None - # --- restored from the pre-rewrite file (not telemetry). Adapted from the old - # ``_patch_query``/``_HAS_OTEL`` mocking approach to ``monkeypatch.setattr(handler_mod, - # "query", ...)`` plus real SDK dataclasses, because ``query`` is now a top-level import - # rather than something resolved through ``importlib.import_module`` on every call, and - # ``handler_mod`` no longer has its own ``_HAS_OTEL`` (that flag now lives in ``spans.py``). --- + # --- not telemetry. These use ``monkeypatch.setattr(handler_mod, "query", ...)`` plus real + # SDK dataclasses, because ``query`` is a top-level import rather than something resolved + # through ``importlib.import_module`` on every call, and the ``_HAS_OTEL`` flag lives in + # ``spans.py`` rather than ``handler_mod``. --- async def test_stream_is_defined(self) -> None: h = create_claude_agents_handler() @@ -1481,10 +1477,8 @@ async def _query(**kwargs: Any) -> AsyncIterator[Any]: await create_claude_agents_handler()(cfg, "q") assert "valid JSON" in captured["options"].system_prompt - # --- restored from the pre-rewrite file (not telemetry). Adapted from the old - # ``_patch_query``-plus-mocked-``ClaudeAgentOptions`` approach, since ``options`` is now a - # real ``ClaudeAgentOptions`` instance (attribute access) rather than a dict the old mock's - # ``side_effect=lambda **kw: kw`` produced. --- + # --- not telemetry. ``options`` is a real ``ClaudeAgentOptions`` instance, so these read it + # by attribute access rather than as a dict. --- async def test_absent_output_format_no_change( self, monkeypatch: pytest.MonkeyPatch @@ -1609,7 +1603,7 @@ def _fake_config(**_kwargs: Any) -> Any: result = await claude_agents("cfg-key", "hi", {"key": "u1"}) assert result == {"output": "ok"} - # --- restored from the pre-rewrite file (not telemetry); byte-for-byte --- + # --- not telemetry --- def test_calls_through_to_model_call(self) -> None: from launchdarkly_ai_claude_agents.handler import claude_agents @@ -1669,16 +1663,14 @@ def test_callable_without_extra_kwargs(self) -> None: # --------------------------------------------------------------------------- -# §1.2 Path C — None user_input must not produce None prompt. -# Restored from the pre-rewrite file (not telemetry). Adapted from the old -# ``_patch_query``/mocked-module approach to ``monkeypatch.setattr(handler_mod, "query", ...)``, -# since ``query`` is now resolved once at import time rather than through -# ``importlib.import_module`` on every call. +# None user_input must not produce None prompt (not telemetry). +# Uses ``monkeypatch.setattr(handler_mod, "query", ...)``, since ``query`` is resolved once at +# import time rather than through ``importlib.import_module`` on every call. # --------------------------------------------------------------------------- class TestNoneUserInput: - """TESTING.md §1.2 Path C: When user_input is None, the prompt passed to + """When user_input is None, the prompt passed to the provider must be '' (empty string), not None.""" async def test_none_user_input_instructions_path_prompt_is_empty_string( @@ -1706,8 +1698,7 @@ async def _spy_query(**kwargs: Any) -> AsyncIterator[Any]: # --------------------------------------------------------------------------- -# History parameter (build_prompt) — restored from the pre-rewrite file -# (not telemetry); byte-for-byte, calling build_prompt directly. +# History parameter (build_prompt) — not telemetry; calls build_prompt directly. # --------------------------------------------------------------------------- @@ -1886,13 +1877,13 @@ def test_native_tools_are_still_passed(self) -> None: class TestAbandonedToolSpansAreNotErrors: """An abandoned stream leaves an open tool span UNSET, not ERROR. - The streaming teardown reached close_open_spans, which records an exception and sets ERROR. That - is right for a failure and wrong for abandonment: a consumer stopping early is normal, and the - root and chat spans on the same path are left UNSET with launchdarkly.stream.abandoned. A tool - span whose PostToolUse hook never fired therefore reported an error nobody had. + close_open_spans records an exception and sets ERROR. That is right for a failure and wrong for + abandonment: a consumer stopping early is normal, and the root and chat spans on the same path + are left UNSET with launchdarkly.stream.abandoned. Routed through close_open_spans, a tool span + whose PostToolUse hook never fired would report an error nobody had. - The openai-agents and langchain-agents handlers already used the UNSET path here, so this also - closes a three-way disagreement about what one abandoned run looks like. + The openai-agents and langchain-agents handlers use the same UNSET path, so all three agree on + what one abandoned run looks like. """ async def test_a_tool_span_open_at_abandonment_is_unset_and_marked( @@ -1970,7 +1961,7 @@ async def _drain() -> None: assert "launchdarkly.stream.abandoned" not in root_span.attributes async def test_a_failed_run_still_marks_open_tool_spans_as_errors(self) -> None: - # The distinction the fix rests on: failure keeps ERROR, abandonment does not. + # The distinction abandonment rests on: failure keeps ERROR, abandonment does not. from launchdarkly_ai_claude_agents.handler import build_tool_hooks hooks, close_open_spans, _, _ = build_tool_hooks({}, None, False) @@ -1994,10 +1985,11 @@ async def test_a_failed_run_still_marks_open_tool_spans_as_errors(self) -> None: class TestAnEmptyRunDoesNotClaimItCostNothing: """A stream that ended with nothing reported must leave the root's usage attributes absent. - Both paths wrote the all-zero per-response sum when the stream ended without a ResultMessage and - without absorbing a single assistant turn. Zeros say the run cost nothing, which is a different - claim from not knowing what it cost, and a config-scoped cost query cannot tell the two apart - once the zeros are on the span. The error and abandonment paths already guarded on `reported`. + Neither path writes the all-zero per-response sum when the stream ends without a ResultMessage + and without absorbing a single assistant turn. Zeros say the run cost nothing, which is a + different claim from not knowing what it cost, and a config-scoped cost query cannot tell the + two apart once the zeros are on the span. The error and abandonment paths guard on `reported` + too. """ async def test_the_blocking_path_writes_no_usage_when_nothing_reported( @@ -2047,9 +2039,9 @@ async def _no_result(**_kwargs: Any) -> AsyncIterator[Any]: class TestInputWritesNeverLeakASpan: """Serialising the prompt must not be able to strand the root span. - The input content write ran before the guard that fails the root, so a raise there left it open: - never ended, never exported, so the run disappeared from AI Config Monitoring along with the - feature_flag event it carries. + The input content write sits inside the guard that fails the root. Ahead of it, a raise there + would leave the root open: never ended, never exported, so the run would disappear from AI + Config Monitoring along with the feature_flag event it carries. """ async def test_the_blocking_root_still_ends( @@ -2097,10 +2089,10 @@ async def _q(**_kwargs: Any) -> AsyncIterator[Any]: class TestCancellationEndsEverySpan: """TELEMETRY-CONTRACT.md section 6: a `finally` owns every end. - ``asyncio.CancelledError`` is a ``BaseException``, so `except Exception` never sees it. Before - this, a cancelled run exported nothing at all: the root carries the feature_flag event and - every launchdarkly.* attribute, so the run vanished from AI Config Monitoring rather than - showing as incomplete. + ``asyncio.CancelledError`` is a ``BaseException``, so `except Exception` never sees it. Without + a `finally`, a cancelled run would export nothing at all: the root carries the feature_flag + event and every launchdarkly.* attribute, so the run would vanish from AI Config Monitoring + rather than showing as incomplete. """ async def test_a_cancelled_run_still_exports_its_spans( @@ -2158,9 +2150,9 @@ async def test_a_tool_result_that_will_not_serialise_still_ends_the_span( self, monkeypatch: pytest.MonkeyPatch ) -> None: # A tool result comes from the caller's own function, so it can be anything, including - # something json.dumps refuses. Before this the write ran after the pop and outside any guard, - # so the span was untracked and unended: never exported, and a reader saw a tool that started - # and never returned. + # something json.dumps refuses. If the write ran after the pop and outside any guard, the + # span would be untracked and unended: never exported, and a reader would see a tool that + # started and never returned. class Unserialisable: pass diff --git a/packages/claude-agents/tests/test_native_graph.py b/packages/claude-agents/tests/test_native_graph.py index e01900f4..d126f76a 100644 --- a/packages/claude-agents/tests/test_native_graph.py +++ b/packages/claude-agents/tests/test_native_graph.py @@ -1,6 +1,5 @@ """ -Tests for §2.2 native graph adapter (to_claude_agents) plus Anthropic-specific specs. -Reference: TESTING.md §2.2, §2.x (Anthropic) +Tests for the native graph adapter (to_claude_agents) plus Anthropic-specific specs. """ from __future__ import annotations @@ -129,7 +128,7 @@ async def _query(**kwargs: Any) -> AsyncIterator[Any]: # --------------------------------------------------------------------------- -# §2.2 Generic topology +# Generic topology # --------------------------------------------------------------------------- diff --git a/packages/claude-messages/src/launchdarkly_ai_claude_messages/handler.py b/packages/claude-messages/src/launchdarkly_ai_claude_messages/handler.py index 0de014a3..a60c5027 100644 --- a/packages/claude-messages/src/launchdarkly_ai_claude_messages/handler.py +++ b/packages/claude-messages/src/launchdarkly_ai_claude_messages/handler.py @@ -198,8 +198,8 @@ async def _run_tool_loop( the run's spend with it. """ # Not filtered to the tools that have a registered handler, unlike the TypeScript SDK. That - # difference predates this span work and changes what the model is offered, not what the span - # reports, so it stays as it is: the catalog recorded below is the catalog actually sent. + # difference changes what the model is offered, not what the span reports: the catalog + # recorded below is the catalog actually sent. tools = _build_tools(config.get("tools") or {}) max_tokens = (config.get("model", {}).get("parameters") or {}).get( "max_tokens", 1024 diff --git a/packages/claude-messages/tests/test_handler.py b/packages/claude-messages/tests/test_handler.py index f07336fe..79df5f8e 100644 --- a/packages/claude-messages/tests/test_handler.py +++ b/packages/claude-messages/tests/test_handler.py @@ -1,7 +1,6 @@ """ Tests for launchdarkly-ai-claude-messages handler. -Covers §1.1–1.9 (generic handler tests). -Reference: TESTING.md §1 +Covers the generic handler behaviours. """ from __future__ import annotations @@ -91,7 +90,7 @@ def mock_anthropic(mocker): # --------------------------------------------------------------------------- -# §1.1 Factory function and metadata +# Factory function and metadata # --------------------------------------------------------------------------- @@ -125,7 +124,7 @@ def test_multiple_calls_return_independent_instances( # --------------------------------------------------------------------------- -# §1.2 Prompt construction +# Prompt construction # --------------------------------------------------------------------------- @@ -281,7 +280,7 @@ async def test_path_c_both_instructions_and_messages_messages_wins( # --------------------------------------------------------------------------- -# §1.3 Tool conversion +# Tool conversion # --------------------------------------------------------------------------- @@ -360,7 +359,7 @@ async def test_custom_parameters_passthrough( # --------------------------------------------------------------------------- -# §1.4 Tool execution loop +# Tool execution loop # --------------------------------------------------------------------------- @@ -459,7 +458,7 @@ async def test_multiple_consecutive_tool_calls( # --------------------------------------------------------------------------- -# §1.5 Telemetry +# Telemetry # --------------------------------------------------------------------------- # Span recording # --------------------------------------------------------------------------- @@ -500,8 +499,8 @@ def end(self) -> None: class SpanRecorder: """Stands in for the ``trace`` module inside ``spans.py`` and records every span opened. - Replaces the old single-MagicMock approach, which could not see a span tree at all: every span - was the same object, so a parent and its children were indistinguishable. + A single MagicMock could not see a span tree at all: every span would be the same object, so a + parent and its children would be indistinguishable. """ def __init__(self) -> None: @@ -1011,7 +1010,7 @@ async def test_still_writes_the_legacy_content_events_when_enabled( # --------------------------------------------------------------------------- -# §1.6 Error handling +# Error handling # --------------------------------------------------------------------------- @@ -1125,7 +1124,7 @@ async def test_rethrows_error(self, mock_anthropic: MagicMock) -> None: # --------------------------------------------------------------------------- -# §1.9 Structured output (outputFormat) +# Structured output (outputFormat) # --------------------------------------------------------------------------- @@ -1179,7 +1178,7 @@ async def test_output_format_with_messages_system_appended( # --------------------------------------------------------------------------- -# §1.7 Convenience export +# Convenience export # --------------------------------------------------------------------------- @@ -1227,7 +1226,7 @@ def test_callable_without_extra_kwargs(self) -> None: # --------------------------------------------------------------------------- -# §1.8 Streaming +# Streaming # --------------------------------------------------------------------------- @@ -1378,12 +1377,12 @@ async def _bad_ctx() -> AsyncGenerator[Any, None]: # --------------------------------------------------------------------------- -# §1.2 Path C — None user_input must not produce None content +# None user_input must not produce None content # --------------------------------------------------------------------------- class TestNoneUserInput: - """TESTING.md §1.2 Path C: When user_input is None, the user-role message + """When user_input is None, the user-role message content sent to the provider must be '' not None.""" async def test_none_user_input_instructions_path_no_none_content( @@ -1414,12 +1413,12 @@ async def _capture(**kwargs: Any) -> Any: # --------------------------------------------------------------------------- -# §1.10 MAX_STEPS cap +# MAX_STEPS cap # --------------------------------------------------------------------------- class TestMaxStepsCap: - """TESTING.md §1.10: The tool loop must break with an error after MAX_STEPS (5) iterations.""" + """The tool loop must break with an error after MAX_STEPS (5) iterations.""" def _tool_use_response(self, id: str = "tu1") -> MagicMock: return _anthropic_response( @@ -1507,7 +1506,7 @@ async def _iter() -> AsyncGenerator: # --------------------------------------------------------------------------- -# §1.5 Streaming telemetry (Appendix A.5 — do not patch _HAS_OTEL=False) +# Streaming telemetry (do not patch _HAS_OTEL=False) # --------------------------------------------------------------------------- @@ -1832,9 +1831,9 @@ class TestToolSpanNeverLeaks: """A raise while recording a tool result must not leave its span open. Serialising a tool result can raise, most easily when capture_content is on and the result is - not JSON-serialisable. The success-side content write used to sit outside the try, so that raise - skipped both the finish and the failure path: only the root was marked ERROR, and the tool span - was never ended, so the exporter never saw it. + not JSON-serialisable. The success-side content write therefore sits inside the try: outside it, + that raise would skip both the finish and the failure path, so only the root would be marked + ERROR and the tool span would never end, and the exporter would never see it. """ async def test_an_unserialisable_tool_result_still_ends_the_tool_span( @@ -1871,10 +1870,11 @@ class _Unserialisable: class TestOpenToolSpanIsNeverLeaked: """A BaseException while a tool runs must still close the execute_tool span. - The streaming `finally` closed the model span and the root, but the in-flight tool span was held - only by a local. `except Exception` does not see a `CancelledError` or a `GeneratorExit`, so a - tool cancelled mid-flight left its span open and unexported: the trace showed a closed parent - above a child that never arrived, which reads as a tool still running long after the run ended. + The streaming `finally` closes the model span and the root, and the in-flight tool span too. + `except Exception` does not see a `CancelledError` or a `GeneratorExit`, so if only a local held + that span, a tool cancelled mid-flight would leave it open and unexported: the trace would show + a closed parent above a child that never arrived, which reads as a tool still running long after + the run ended. """ async def test_a_tool_cancelled_mid_flight_still_ends_its_span( @@ -1919,10 +1919,9 @@ async def _cancelled_tool(_: Any) -> Any: tools = rec.named("execute_tool ") assert len(tools) == 1 assert tools[0].ended == 1, "the execute_tool span leaked" - # `cancelled`, not `abandoned`. This test used to assert the latter, which is what the defect - # looked like: nothing here chose to stop reading, a CancelledError ended the run underneath - # the consumer. The consumer-break test above still asserts `abandoned`, which is the case - # that word is for. + # `cancelled`, not `abandoned`: nothing here chose to stop reading, a CancelledError ended + # the run underneath the consumer. The consumer-break test above asserts `abandoned`, + # which is the case that word is for. assert tools[0].attributes["launchdarkly.run.cancelled"] is True assert "launchdarkly.stream.abandoned" not in tools[0].attributes assert rec.root.ended == 1 @@ -1931,10 +1930,10 @@ async def _cancelled_tool(_: Any) -> Any: class TestBlockingChatSpanNeverLeaks: """The blocking path has no `finally`, so anything that raises outside the guard is unrecoverable. - The output content write and the span finish sat outside the try that fails the chat span. A raise - while serialising the completion left the span open and unexported, and dropped the turn from the - run total, so the trace showed an errored root with no model call and a cost lower than the one - Anthropic had already billed. + The output content write and the span finish sit inside the try that fails the chat span. + Outside it, a raise while serialising the completion would leave the span open and unexported, + and drop the turn from the run total, so the trace would show an errored root with no model call + and a cost lower than the one Anthropic had already billed. """ async def test_an_unserialisable_completion_still_ends_the_chat_span( @@ -2089,9 +2088,9 @@ async def _ctx_mgr() -> AsyncGenerator[Any, None]: class TestInputWritesNeverLeakASpan: """Serialising the prompt must not be able to strand a span. - The input content write ran before the guard that fails the span it writes to, so a raise there - left the root open on both paths: never ended, never exported, so the run disappeared from AI - Config Monitoring along with the feature_flag event it carries. + The input content write sits inside the guard that fails the span it writes to. Ahead of it, a + raise there would leave the root open on both paths: never ended, never exported, so the run + would disappear from AI Config Monitoring along with the feature_flag event it carries. """ async def test_the_blocking_root_still_ends( @@ -2192,10 +2191,10 @@ class TestCancellationEndsEverySpan: async def test_a_cancelled_run_still_exports_its_spans( self, mock_anthropic: MagicMock ) -> None: - # asyncio.CancelledError is a BaseException, so `except Exception` never sees it. Before this, - # a cancelled run exported nothing at all: the root carries the feature_flag event and every - # launchdarkly.* attribute, so the run vanished from AI Config Monitoring rather than showing - # as incomplete. + # asyncio.CancelledError is a BaseException, so `except Exception` never sees it. Without a + # `finally`, a cancelled run would export nothing at all: the root carries the feature_flag + # event and every launchdarkly.* attribute, so the run would vanish from AI Config + # Monitoring rather than showing as incomplete. import asyncio async def never_returns(*args: Any, **kwargs: Any) -> Any: diff --git a/packages/client/README.md b/packages/client/README.md index b2607d4e..a85ea7fe 100644 --- a/packages/client/README.md +++ b/packages/client/README.md @@ -151,7 +151,7 @@ All three judge paths build `{{message_history}}` through a single function, `ju Each one is the input, then the tool trajectory, then the output, then the `{score, reasoning}` format block, with empty parts skipped. A judge therefore grades the same conversation wherever it runs, which is what makes a rubric portable between a production sample and a dataset replay. -They did not always agree, and that is why this is a single function now: each path used to join its own history. The offline one carried the row input, the inline one carried the user input, and the deferred one carried **neither** — so a deferred judge graded a response with no request beside it. `JudgeTask` gained `user_input` and `trajectory` to close that. +Every path carries both the input and the trajectory, including the deferred one: `JudgeTask` has `user_input` and `trajectory` fields so a deferred judge never grades a response with no request beside it. For the deferred path those two fields travel on the task, which stays picklable — the trajectory crosses as the rendered string, not the structured record. @@ -159,7 +159,7 @@ A **graph-level** judge (`graph_judge`) gets no trajectory: it grades a final an Two limits keep a trajectory from spending the judge's context window: at most 50 recorded calls per row and 2000 characters per rendered argument bag or result, with anything beyond either reported as a count or marked truncated. Calls past the limit still execute — truncation drops the record, never the work. A `NativeTool` runs inside the provider, so no local wrapper sees it; such a tool is left out of the trajectory and out of the "Tools available" line, since naming a tool whose use cannot be shown would invite a judge to conclude the model ignored it. -A tool result is now judge-prompt input. It stays literal for the same reason the generated output does: the judge config is handed to the handler unrendered and the handler makes exactly one template pass, so a `{{...}}` sequence coming back from a tool is never expanded into the judge prompt. +A tool result is judge-prompt input. It stays literal for the same reason the generated output does: the judge config is handed to the handler unrendered and the handler makes exactly one template pass, so a `{{...}}` sequence coming back from a tool is never expanded into the judge prompt. **Judges are independent AI Configs, so handlers are routed per judge.** A judge may resolve to a different provider or mode than `generation`, and a handler built for one provider cannot execute another's config. `handler` runs a judge when it provides for that judge's provider; pass handlers for any other providers in `judge_handlers`. Selection prefers a handler naming the judge's provider outright over a wildcard multi-provider adapter, and an agent-mode handler can serve a messages-mode judge with its messages collapsed into one instructions block. A plain callable that declares no `provides_for` routes itself, exactly as it already does for the generation config. diff --git a/packages/client/src/launchdarkly_ai_server/content.py b/packages/client/src/launchdarkly_ai_server/content.py index 99eaf1c9..58f83aa3 100644 --- a/packages/client/src/launchdarkly_ai_server/content.py +++ b/packages/client/src/launchdarkly_ai_server/content.py @@ -196,10 +196,10 @@ class ToolDefinitionInput: def to_semconv_finish_reason(raw: str | None) -> str | None: """Maps one provider's finish reason onto semconv's ``gen_ai.response.finish_reasons`` vocabulary. - This SDK used to pass the provider's string through untranslated, on the argument that - translating ``end_turn`` into ``stop`` loses information. Measuring it settled the argument the - other way: a single run emits ``chat`` spans from more than one handler, so a consumer grouping - by finish reason saw ``stop`` and ``end_turn`` as two different outcomes for the same event. + The provider's string is translated rather than passed through, even though translating + ``end_turn`` into ``stop`` looks like it loses information: a single run emits ``chat`` spans + from more than one handler, so a consumer grouping by finish reason would otherwise see + ``stop`` and ``end_turn`` as two different outcomes for the same event. Nothing is lost. The provider's own wording is still on the span, because the raw response is what ``gen_ai.output.messages`` was built from, and an unrecognised reason is passed through diff --git a/packages/client/src/launchdarkly_ai_server/history.py b/packages/client/src/launchdarkly_ai_server/history.py index f5f46948..5d1fa836 100644 --- a/packages/client/src/launchdarkly_ai_server/history.py +++ b/packages/client/src/launchdarkly_ai_server/history.py @@ -1,8 +1,8 @@ """Shared conversation-history composition and multimodal content helpers. Mirrors the TypeScript ``history`` module so every handler composes runtime -``history`` the same way (TESTING.md §1.11) and maps LaunchDarkly-canonical -content blocks to each provider's native shape (Appendix A.7). +``history`` the same way and maps LaunchDarkly-canonical content blocks to +each provider's native shape. History messages are plain dicts: ``{"role": ..., "content": ...}`` where ``content`` is either a string or a list of content-block dicts: diff --git a/packages/client/src/launchdarkly_ai_server/judges.py b/packages/client/src/launchdarkly_ai_server/judges.py index 56d7e032..ee23535c 100644 --- a/packages/client/src/launchdarkly_ai_server/judges.py +++ b/packages/client/src/launchdarkly_ai_server/judges.py @@ -388,8 +388,8 @@ def _matches(h: ProviderHandler) -> bool: ) # user_input and trajectory come off the task rather than being omitted: - # this path used to build a history with neither, so a judge grading the - # same response saw a different conversation than the inline path did. + # without them a judge grading the same response would see a different + # conversation than the inline path shows it. message_history = build_message_history( user_input=task.user_input, trajectory=task.trajectory, diff --git a/packages/client/src/launchdarkly_ai_server/types.py b/packages/client/src/launchdarkly_ai_server/types.py index 761faf5b..9a9a9a22 100644 --- a/packages/client/src/launchdarkly_ai_server/types.py +++ b/packages/client/src/launchdarkly_ai_server/types.py @@ -305,8 +305,8 @@ class JudgeTask: """The input that produced ``actual_output``. Carried so this path builds the same ``message_history`` as the inline one. - Previously absent, which showed a deferred judge a response with no - request beside it. + Without it a deferred judge would see a response with no request beside + it. """ trajectory: str = "" """The invocation's rendered tool-call trajectory. diff --git a/packages/client/src/launchdarkly_ai_server/utils.py b/packages/client/src/launchdarkly_ai_server/utils.py index 2958de6e..09db2895 100644 --- a/packages/client/src/launchdarkly_ai_server/utils.py +++ b/packages/client/src/launchdarkly_ai_server/utils.py @@ -70,7 +70,7 @@ def number_or_zero(value: Any) -> int: total is greater than zero, and that test is false for ``NaN``, so the metric is dropped silently rather than reported low. - Replaces the bare ``int(...)`` this module used to do, which raised on ``None``. + A bare ``int(...)`` is not enough here, because it raises on ``None``. """ if value is None or isinstance(value, bool): return 0 @@ -154,10 +154,10 @@ def parse_usage(usage: dict[str, Any]) -> dict[str, Any]: def to_usage_dict(usage: dict[str, Any]) -> UsageDict: """Builds the public :class:`UsageDict` from a :func:`parse_usage` result. - Shared because ``invoke`` and the judge runner both need it and both used to build the dataclass - by hand from three keys, which silently dropped the cache breakdown the moment ``parse_usage`` - started reporting one. A caller reading ``input_details`` off a blocking call got ``None`` while - the streaming path handed back the nested dict, so the two paths disagreed about the same run. + Shared because ``invoke`` and the judge runner both need it. Building the dataclass by hand + from three keys would silently drop the cache breakdown ``parse_usage`` reports, so a caller + reading ``input_details`` off a blocking call would get ``None`` while the streaming path handed + back the nested dict, and the two paths would disagree about the same run. """ details = usage.get("input_details") return UsageDict( @@ -338,10 +338,9 @@ def set_model_identity_attributes( """Writes the model identity attributes that every LLM span carries. Both spellings of the provider key are emitted on purpose. ``gen_ai.system`` is the pre-1.37 - semconv name and is what handlers shipped before the span hierarchy landed; - ``gen_ai.provider.name`` is the current name. Emitting only the new key would silently break - dashboards written against the old one, and emitting only the old one leaves us off-spec, so - both go out until the next major. + semconv name; ``gen_ai.provider.name`` is the current name. Emitting only the new key would + silently break dashboards written against the old one, and emitting only the old one leaves us + off-spec, so both go out until the next major. ``legacy_system`` exists because the two keys do not always want the same value. The LangChain handlers ship ``gen_ai.system = 'langchain'``, but ``gen_ai.provider.name`` means *who served @@ -406,9 +405,9 @@ def end_unfinished_spans(*spans: Any) -> None: """Ends every span still open, for an exception no ``except Exception`` can catch. ``asyncio.CancelledError`` inherits from ``BaseException``, deliberately, so a timeout or a - ``task.cancel()`` walks straight past every ``except Exception`` a handler writes. The blocking - paths ended their spans only from those clauses, so a cancelled run exported nothing at all: not a - wrong attribute, no span. The root carries the ``feature_flag`` event and every ``launchdarkly.*`` + ``task.cancel()`` walks straight past every ``except Exception`` a handler writes. A path that + ends its spans only from those clauses exports nothing at all for a cancelled run: not a wrong + attribute, no span. The root carries the ``feature_flag`` event and every ``launchdarkly.*`` attribute, so a stranded root means the whole run never reaches AI Config Monitoring. Call this from a ``finally``, not from an ``except``. The point is the paths an ``except`` cannot @@ -545,7 +544,7 @@ def model_stamps_from_meta(meta: Any) -> dict[str, Any]: from a variation's ``_ldMeta`` into a dict that can be merged into ``TrackData``. Keys are omitted (never set to ``None``) when absent; an empty ``modelKey`` is treated as absent and ``modelVersion`` is coerced to - ``int``. Gonfalon's cost attribution reads these two fields from every + ``int``. LaunchDarkly's cost attribution reads these two fields from every ``$ld:ai:*`` event payload. """ if not isinstance(meta, dict): @@ -740,7 +739,7 @@ def set_ld_span_attributes(span: Any, variables: dict[str, Any] | None) -> None: def set_openllmetry_prompt(span: Any, messages: list[dict[str, str]]) -> None: """Set OpenLLMetry-style indexed prompt attributes on a span. - Gonfalon's LLM Summary tab reads ``gen_ai.prompt.N.role`` / ``.content`` + The LLM Summary tab in LaunchDarkly reads ``gen_ai.prompt.N.role`` / ``.content`` (attribute-based, takes precedence over span events). """ for i, msg in enumerate(messages): @@ -755,7 +754,7 @@ def set_openllmetry_completion( ) -> None: """Set OpenLLMetry-style indexed completion attributes and token usage aliases. - Gonfalon reads ``gen_ai.completion.0.role`` / ``.content`` and prefers + LaunchDarkly reads ``gen_ai.completion.0.role`` / ``.content`` and prefers ``gen_ai.usage.prompt_tokens`` / ``completion_tokens``. """ span.set_attribute("gen_ai.completion.0.role", "assistant") diff --git a/packages/client/tests/test_client.py b/packages/client/tests/test_client.py index d7357308..586637b2 100644 --- a/packages/client/tests/test_client.py +++ b/packages/client/tests/test_client.py @@ -1,7 +1,5 @@ """ -Tests for §3.10 config(), §3.12 LD context interpolation, -§3.15 config().stream(). -Reference: TESTING.md §3.10, §3.12, §3.15 +Tests for config(), LD context interpolation, and config().stream(). """ from collections.abc import AsyncGenerator @@ -79,7 +77,7 @@ def mock_ld_client() -> MagicMock: # --------------------------------------------------------------------------- -# §3.10 config() — single handler +# config() — single handler # --------------------------------------------------------------------------- @@ -382,7 +380,7 @@ async def test_parse_failure_returns_raw_string_when_output_format_set( """When outputFormat is set but the handler returns an unparseable string, invoke() returns the raw string rather than raising — agents and streaming handlers cannot guarantee structured output (best-effort, consistent with - TypeScript SDK behavior). See TESTING.md §3.10.""" + TypeScript SDK behavior).""" raw = { "model": {"name": "gpt-4"}, "provider": {"name": "TestProvider"}, @@ -403,7 +401,7 @@ async def test_parse_failure_returns_raw_string_when_output_format_set( # --------------------------------------------------------------------------- -# §3.10 config() — multi-handler routing +# config() — multi-handler routing # --------------------------------------------------------------------------- @@ -565,7 +563,7 @@ async def test_throws_mode_specific_error_when_provider_matches_but_mode_does_no # --------------------------------------------------------------------------- -# §3.12 LD context interpolation +# LD context interpolation # --------------------------------------------------------------------------- @@ -654,7 +652,7 @@ async def capturing_fn( # --------------------------------------------------------------------------- -# §3.15 config().stream() +# config().stream() # --------------------------------------------------------------------------- diff --git a/packages/client/tests/test_content.py b/packages/client/tests/test_content.py index 9a89e6cc..3baa9c0a 100644 --- a/packages/client/tests/test_content.py +++ b/packages/client/tests/test_content.py @@ -141,9 +141,9 @@ def _get_type(self) -> str: class TestToolCallArgumentsAgreeAcrossCarriers: def test_absent_arguments_are_omitted_not_written_as_null(self) -> None: - # to_canonical omits absent arguments. to_text used to write them as null, so the OpenLLMetry - # carrier said the model passed a null argument bag where the canonical one said it passed - # none. The module docstring promises the three carriers cannot disagree. + # to_canonical omits absent arguments, so to_text must too: writing them as null would make + # the OpenLLMetry carrier say the model passed a null argument bag where the canonical one + # says it passed none. The module docstring promises the three carriers cannot disagree. part = SpanMessagePart(type="tool_call", name="f") assert part.to_canonical() == {"type": "tool_call", "name": "f"} assert part.to_text() == json.dumps({"name": "f"}) @@ -162,8 +162,8 @@ class TestContentWritersToleratePeopleWithoutOpenTelemetry: """Handlers hold None for every span when the `otel` extra is absent.""" def test_no_writer_raises_on_a_none_span(self) -> None: - # capture_content=True without the extra installed used to raise AttributeError from inside - # the telemetry path, after the provider had already billed the turn. Telemetry may report + # capture_content=True without the extra installed must not raise AttributeError from inside + # the telemetry path, after the provider has already billed the turn. Telemetry may report # nothing; it may not break the call it is reporting on. messages = [ SpanMessage( diff --git a/packages/client/tests/test_evaluations_run.py b/packages/client/tests/test_evaluations_run.py index 4a4b8571..ef6c1406 100644 --- a/packages/client/tests/test_evaluations_run.py +++ b/packages/client/tests/test_evaluations_run.py @@ -22,7 +22,7 @@ @pytest.fixture(autouse=True) def stub_sdk_client(monkeypatch: pytest.MonkeyPatch) -> MagicMock: - """Give every run a resolvable SDK client, since one is now required.""" + """Give every run a resolvable SDK client, since one is required.""" monkeypatch.setenv("LD_SDK_KEY", "sdk-key") client = MagicMock() client.flush = AsyncMock() @@ -1316,7 +1316,7 @@ async def test_judges_resolve_once_per_run_not_once_per_row( and the next -- rows in a single run scored against different judges. The online path does resolve per invocation (judges.build_judge_tasks), so routing the offline runner through it for convenience is a live way to - reintroduce this. + cause this. """ transport = SequencedTransport( [ @@ -2502,7 +2502,7 @@ async def test_tool_result_placeholders_are_not_expanded_into_the_judge_prompt( monkeypatch: pytest.MonkeyPatch, stub_sdk_client: MagicMock, ) -> None: - """A tool result is now judge-prompt input, so it is an injection surface. + """A tool result is judge-prompt input, so it is an injection surface. It stays literal for the same reason the generated output does: the judge config is handed over unrendered and the handler makes exactly one template diff --git a/packages/client/tests/test_graph.py b/packages/client/tests/test_graph.py index a172e021..297fa109 100644 --- a/packages/client/tests/test_graph.py +++ b/packages/client/tests/test_graph.py @@ -1,6 +1,5 @@ """ -Tests for §3.12 resolve_graph() and graph(). -Reference: TESTING.md §3.12 +Tests for resolve_graph() and graph(). """ from typing import Any @@ -428,7 +427,7 @@ async def test_reverse_traverse_key_is_snake_case( self, mock_ld_client: MagicMock ) -> None: """Python GraphDefinition must expose 'reverse_traverse' (snake_case), not - 'reverseTraverse' (camelCase). See TESTING.md Appendix A.2.""" + 'reverseTraverse' (camelCase).""" gd = await resolve_graph( "graph-key", context=CONTEXT, handlers=[_make_handler()] ) @@ -443,15 +442,14 @@ async def test_disabled_graph_reverse_traverse_key_is_snake_case( self, mock_ld_client: MagicMock ) -> None: """Disabled GraphDefinition stub must also use 'reverse_traverse', not - 'reverseTraverse'. See TESTING.md Appendix A.2.""" + 'reverseTraverse'.""" mock_ld_client.variation = AsyncMock(return_value={"edges": {}}) gd = await resolve_graph("graph-key", context=CONTEXT) assert hasattr(gd, "reverse_traverse") assert not hasattr(gd, "reverseTraverse") async def test_cache_is_bounded(self, mock_ld_client: MagicMock) -> None: - """GraphInstance._cache must not grow beyond MAX_GRAPH_CACHE_SIZE. - See TESTING.md §3.11.""" + """GraphInstance._cache must not grow beyond MAX_GRAPH_CACHE_SIZE.""" from launchdarkly_ai_server.graph import MAX_GRAPH_CACHE_SIZE g = graph("graph-key", handlers=[_make_handler()]) @@ -466,7 +464,7 @@ async def test_equal_content_contexts_share_cache_entry( ) -> None: """Two distinct dict objects with identical JSON content must share one cache entry. The cache key must be json.dumps(context, sort_keys=True), - not id(context). See TESTING.md §3.11.""" + not id(context).""" g = graph("graph-key", handlers=[_make_handler()]) ctx_a = {"kind": "user", "key": "u1"} ctx_b = {"kind": "user", "key": "u1"} # distinct object, same content diff --git a/packages/client/tests/test_graph_stream.py b/packages/client/tests/test_graph_stream.py index 2b1cd47c..ab70b7dc 100644 --- a/packages/client/tests/test_graph_stream.py +++ b/packages/client/tests/test_graph_stream.py @@ -1,6 +1,5 @@ """ -Tests for §3.15a ``graph().stream()``. -Reference: TESTING.md §3.15a, Appendix A.4 / A.13. +Tests for ``graph().stream()``. """ from __future__ import annotations @@ -751,7 +750,7 @@ async def invoke_fn( # --------------------------------------------------------------------------- -# Conversation id + OTel parenting + abandonment (§3.15a / A.4) +# Conversation id + OTel parenting + abandonment # --------------------------------------------------------------------------- diff --git a/packages/client/tests/test_judge_message_history.py b/packages/client/tests/test_judge_message_history.py index c194b321..74d49207 100644 --- a/packages/client/tests/test_judge_message_history.py +++ b/packages/client/tests/test_judge_message_history.py @@ -282,7 +282,7 @@ async def handler( history: Any = None, ) -> dict[str, Any]: # Not awaited: the native stub is sync, unlike the async wrapper a - # real callable gets. Pre-existing asymmetry. + # real callable gets. A known asymmetry. tool_handlers["web_search"]({"q": "x"}) return {"output": "done", "usage": {}} diff --git a/packages/client/tests/test_judges.py b/packages/client/tests/test_judges.py index 5d560948..904f24b3 100644 --- a/packages/client/tests/test_judges.py +++ b/packages/client/tests/test_judges.py @@ -1,6 +1,5 @@ """ -Tests for §3.14 run_judges. -Reference: TESTING.md §3.14 +Tests for run_judges. """ from typing import Any, ClassVar @@ -388,7 +387,7 @@ async def test_returns_empty_dict_when_judges_array_is_empty( class TestScoreGuard: - """`float(score)` used to sit ahead of the evaluation-metric track, so a junk score killed it.""" + """`float(score)` stays behind the evaluation-metric track, so a junk score cannot kill it.""" def test_rejects_non_numeric_scores_without_raising(self) -> None: from launchdarkly_ai_server.judge_scoring import numeric_score @@ -481,7 +480,7 @@ async def judge_fn( class TestRunJudgeTrackData: - """§3.13 run_judge result track_data must not inherit the parent's model stamps.""" + """run_judge result track_data must not inherit the parent's model stamps.""" PARENT: ClassVar[dict[str, Any]] = { "runId": "parent-run", diff --git a/packages/client/tests/test_ld_span_attributes.py b/packages/client/tests/test_ld_span_attributes.py index bd0b470a..edbda39d 100644 --- a/packages/client/tests/test_ld_span_attributes.py +++ b/packages/client/tests/test_ld_span_attributes.py @@ -1,4 +1,4 @@ -"""Context identity on the root feature_flag span. TESTING.md §3.18.""" +"""Context identity on the root feature_flag span.""" from __future__ import annotations diff --git a/packages/client/tests/test_lifecycle.py b/packages/client/tests/test_lifecycle.py index 60bf35f1..43eee720 100644 --- a/packages/client/tests/test_lifecycle.py +++ b/packages/client/tests/test_lifecycle.py @@ -1,6 +1,5 @@ """ -Tests for §3.9 init_client / get_client / shutdown. -Reference: TESTING.md §3.9 +Tests for init_client / get_client / shutdown. """ import os @@ -290,7 +289,7 @@ async def test_completes_teardown_even_if_flush_throws(self) -> None: # --------------------------------------------------------------------------- -# OTel setup details (§3.9) +# OTel setup details # --------------------------------------------------------------------------- diff --git a/packages/client/tests/test_registry.py b/packages/client/tests/test_registry.py index 52a9fedd..2ae4221c 100644 --- a/packages/client/tests/test_registry.py +++ b/packages/client/tests/test_registry.py @@ -1,6 +1,5 @@ """ -Tests for §3.6 Registry and compose, §3.7 resolve_handlers and resolve_tools. -Reference: TESTING.md §3.6–3.7 +Tests for Registry and compose, resolve_handlers and resolve_tools. """ import logging @@ -27,7 +26,7 @@ async def fn(config, user_input, tool_handlers, variables): # type: ignore[over # --------------------------------------------------------------------------- -# §3.6 Registry +# Registry # --------------------------------------------------------------------------- @@ -97,7 +96,7 @@ def test_register_is_additive(self) -> None: # --------------------------------------------------------------------------- -# §3.6 compose +# compose # --------------------------------------------------------------------------- @@ -137,7 +136,7 @@ def test_non_conflicting_entries_are_merged(self) -> None: # --------------------------------------------------------------------------- -# §3.7 resolve_handlers +# resolve_handlers # --------------------------------------------------------------------------- @@ -169,7 +168,7 @@ def test_registry_present_but_empty_no_local(self) -> None: # --------------------------------------------------------------------------- -# §3.7 resolve_tools +# resolve_tools # --------------------------------------------------------------------------- diff --git a/packages/client/tests/test_schema.py b/packages/client/tests/test_schema.py index fad75cf0..e355bbcb 100644 --- a/packages/client/tests/test_schema.py +++ b/packages/client/tests/test_schema.py @@ -1,6 +1,5 @@ """ -Tests for §3.5 parse_ai_config (AiConfig validation). -Reference: TESTING.md §3.5 +Tests for parse_ai_config (AiConfig validation). """ from launchdarkly_ai_server import parse_ai_config diff --git a/packages/client/tests/test_sdk_info.py b/packages/client/tests/test_sdk_info.py index dff75744..43e53e59 100644 --- a/packages/client/tests/test_sdk_info.py +++ b/packages/client/tests/test_sdk_info.py @@ -1,4 +1,4 @@ -"""Tests for TESTING.md §3.9 AI SDK package information events.""" +"""Tests for AI SDK package information events.""" from unittest.mock import MagicMock diff --git a/packages/client/tests/test_span_usage.py b/packages/client/tests/test_span_usage.py index 197f48de..e49df4b2 100644 --- a/packages/client/tests/test_span_usage.py +++ b/packages/client/tests/test_span_usage.py @@ -48,7 +48,7 @@ def test_passes_a_number_through(self) -> None: assert number_or_zero(42) == 42 def test_none_becomes_zero_rather_than_raising(self) -> None: - # The old bare int(...) raised here, taking the whole call down with it. + # A bare int(...) would raise here, taking the whole call down with it. assert number_or_zero(None) == 0 def test_a_non_numeric_string_becomes_zero(self) -> None: @@ -397,9 +397,9 @@ class Unhashable(FakeSpan): class TestToUsageDict: """The public UsageDict must carry everything parse_usage reported. - Both call sites used to build the dataclass by hand from three keys, so the cache breakdown - vanished the moment parse_usage started reporting one: a caller reading input_details off a - blocking call got None while the streaming path handed back the nested dict. + Building the dataclass by hand from three keys would drop the cache breakdown parse_usage + reports: a caller reading input_details off a blocking call would get None while the streaming + path handed back the nested dict. """ def test_carries_the_three_totals(self) -> None: @@ -505,7 +505,8 @@ class TestEndUnfinishedSpans: def test_ends_a_span_a_cancelled_run_left_open(self, tracer_and_exporter) -> None: # asyncio.CancelledError is a BaseException, so `except Exception` never sees it. Without this - # helper a cancelled run exported no span at all, and the root carries the feature_flag event. + # helper a cancelled run would export no span at all, and the root carries the feature_flag + # event. from launchdarkly_ai_server import end_unfinished_spans tracer, exporter = tracer_and_exporter diff --git a/packages/client/tests/test_stream_conversation.py b/packages/client/tests/test_stream_conversation.py index 198f993c..44fffdde 100644 --- a/packages/client/tests/test_stream_conversation.py +++ b/packages/client/tests/test_stream_conversation.py @@ -198,7 +198,7 @@ async def one(tag: str) -> None: class TestStreamParentingWithIdBound: - """The unbound parenting test cannot catch a regression in the wrapper — it never builds one. + """The unbound parenting test cannot catch a fault in the wrapper — it never builds one. A real streaming handler holds a span current across a ``yield``. The wrapper must not disturb that: a span the handler opens after resuming belongs to its own ``chat`` span, exactly as it diff --git a/packages/client/tests/test_tracking.py b/packages/client/tests/test_tracking.py index 8a625d3f..eb9db7dc 100644 --- a/packages/client/tests/test_tracking.py +++ b/packages/client/tests/test_tracking.py @@ -1,6 +1,5 @@ """ -Tests for §3.8 wrap_tool_handlers. -Reference: TESTING.md §3.8 +Tests for wrap_tool_handlers. """ from unittest.mock import MagicMock, patch diff --git a/packages/client/tests/test_trajectory.py b/packages/client/tests/test_trajectory.py index a6c020d6..a6e99a87 100644 --- a/packages/client/tests/test_trajectory.py +++ b/packages/client/tests/test_trajectory.py @@ -16,7 +16,7 @@ def test_wrapped_sync_tool_stays_sync() -> None: """A blanket async wrapper would hand a caller's handler a coroutine - object where it used to get the tool's value. + object where it expects the tool's value. """ def lookup(args: dict[str, Any]) -> str: @@ -246,8 +246,8 @@ def test_render_does_not_escape_non_ascii() -> None: def test_render_leaves_a_non_ascii_string_result_alone() -> None: - """A string result was never JSON-encoded, so it never escaped. Pins that - both paths agree now. + """A string result is never JSON-encoded, so it never escapes. Pins that + both paths agree. """ rendered = render_trajectory( [ToolInvocation(name="lookup", arguments={}, result="café 東京")], diff --git a/packages/client/tests/test_utils.py b/packages/client/tests/test_utils.py index 88f05298..cfb0ef59 100644 --- a/packages/client/tests/test_utils.py +++ b/packages/client/tests/test_utils.py @@ -1,7 +1,6 @@ """ Tests for parse_template, parse_json_with_possible_fences, parse_usage, normalize_mode, create_handler. -Reference: TESTING.md s3.1-3.4, s3.15 """ from typing import Any @@ -270,7 +269,7 @@ async def fn( class TestMakeTrackData: - """§3.10 model stamps — shared node-trackData builder for native graph adapters.""" + """Model stamps — shared node-trackData builder for native graph adapters.""" def _node(self, meta: dict) -> Any: from launchdarkly_ai_server.types import GraphNode @@ -307,7 +306,7 @@ def test_omits_model_key_and_version_when_absent(self) -> None: class TestModelStampsFromMeta: - """§3.10 model stamps — malformed ``modelVersion`` is omitted, never raises.""" + """Model stamps — malformed ``modelVersion`` is omitted, never raises.""" def test_copies_int_version_and_non_empty_key(self) -> None: assert model_stamps_from_meta({"modelKey": "m", "modelVersion": 3}) == { diff --git a/packages/langchain-agents/src/launchdarkly_ai_langchain_agents/messages.py b/packages/langchain-agents/src/launchdarkly_ai_langchain_agents/messages.py index f305df1e..21e3fa86 100644 --- a/packages/langchain-agents/src/launchdarkly_ai_langchain_agents/messages.py +++ b/packages/langchain-agents/src/launchdarkly_ai_langchain_agents/messages.py @@ -3,7 +3,7 @@ Images travel as ``image_url`` content parts with a data or remote URL — the standard multimodal shape every LangChain chat model accepts — rather than the LaunchDarkly-canonical ``{"type": "image", "source": ...}`` block, which no -LangChain provider understands (TESTING.md Appendix A.7). +LangChain provider understands. """ from __future__ import annotations diff --git a/packages/langchain-agents/src/launchdarkly_ai_langchain_agents/native_graph.py b/packages/langchain-agents/src/launchdarkly_ai_langchain_agents/native_graph.py index b0635866..9057c0d0 100644 --- a/packages/langchain-agents/src/launchdarkly_ai_langchain_agents/native_graph.py +++ b/packages/langchain-agents/src/launchdarkly_ai_langchain_agents/native_graph.py @@ -183,7 +183,7 @@ async def invoke( # WorkflowState must reference add_messages from module-level scope. # With `from __future__ import annotations`, LangGraph resolves annotations # via get_type_hints() in the *module* global namespace — a local variable - # would cause NameError at StateGraph(WorkflowState) time (AIC-2948). + # would cause NameError at StateGraph(WorkflowState) time. class WorkflowState(TypedDict): messages: Annotated[list[Any], add_messages] diff --git a/packages/langchain-agents/tests/test_graph.py b/packages/langchain-agents/tests/test_graph.py index ce5e455a..f143cae1 100644 --- a/packages/langchain-agents/tests/test_graph.py +++ b/packages/langchain-agents/tests/test_graph.py @@ -1,6 +1,5 @@ """ -Tests for §2.1 graph convenience wrapper (langchain_graph) and §2.x.2 full coverage. -Reference: TESTING.md §2.1, §2.x.2 +Tests for the graph convenience wrapper (langchain_graph), with full coverage. """ from unittest.mock import MagicMock, patch diff --git a/packages/langchain-agents/tests/test_handler.py b/packages/langchain-agents/tests/test_handler.py index 1861a1ed..8a027ef8 100644 --- a/packages/langchain-agents/tests/test_handler.py +++ b/packages/langchain-agents/tests/test_handler.py @@ -1,7 +1,6 @@ """ Tests for launchdarkly-ai-langchain-agents handler. -Covers §1.1–1.9 (generic) and §2.x.1 span name. -Reference: TESTING.md §1, §2.x (LangChain) +Covers the generic handler behaviours and the LangChain span name. """ from __future__ import annotations @@ -100,14 +99,14 @@ def _side_effect(name: str) -> Any: # --------------------------------------------------------------------------- -# §2.x.0 Agent creation API (Python: create_react_agent from langgraph.prebuilt) +# Agent creation API (Python: create_react_agent from langgraph.prebuilt) # --------------------------------------------------------------------------- class TestAgentCreationAPI: @pytest.mark.asyncio async def test_calls_create_react_agent_not_createAgent(self) -> None: - """§2.x.0 — Python handler must call create_react_agent from langgraph.prebuilt.""" + """Python handler must call create_react_agent from langgraph.prebuilt.""" captured: dict[str, Any] = {"called": False, "args": None, "kwargs": None} mock_agent = AsyncMock() @@ -160,7 +159,7 @@ def _import_side_effect(name: str) -> Any: # --------------------------------------------------------------------------- -# §1.1 Factory +# Factory # --------------------------------------------------------------------------- @@ -197,7 +196,7 @@ async def test_returns_text_from_mixed_thinking_and_text_blocks(self) -> None: # --------------------------------------------------------------------------- -# §1.2 Prompt construction +# Prompt construction # --------------------------------------------------------------------------- @@ -317,7 +316,7 @@ def test_path_c_instructions_takes_priority_over_messages(self) -> None: # --------------------------------------------------------------------------- -# §1.3 Tool conversion +# Tool conversion # --------------------------------------------------------------------------- @@ -394,7 +393,7 @@ def _capture_tool(fn: Any, **kw: Any) -> Any: # --------------------------------------------------------------------------- -# §1.4 Tool execution loop +# Tool execution loop # --------------------------------------------------------------------------- @@ -463,7 +462,7 @@ async def test_no_tools_in_config_handler_never_invoked(self) -> None: # --------------------------------------------------------------------------- -# §1.5 Telemetry +# Telemetry # --------------------------------------------------------------------------- @@ -1147,7 +1146,7 @@ async def test_rethrows_error(self) -> None: # --------------------------------------------------------------------------- -# §1.7 Convenience export +# Convenience export # --------------------------------------------------------------------------- @@ -1208,7 +1207,7 @@ def test_callable_without_extra_kwargs(self) -> None: # --------------------------------------------------------------------------- -# §1.8 Streaming +# Streaming # --------------------------------------------------------------------------- @@ -1296,7 +1295,7 @@ async def _bad_astream(*a: Any, **kw: Any) -> AsyncIterator[Any]: # --------------------------------------------------------------------------- -# §1.5 Streaming telemetry (Appendix A.5 — do not patch _HAS_OTEL=False) +# Streaming telemetry (do not patch _HAS_OTEL=False) # --------------------------------------------------------------------------- @@ -1422,7 +1421,7 @@ async def test_emits_no_content_by_default_on_the_streaming_path(self) -> None: # --------------------------------------------------------------------------- -# §1.9 Output format +# Output format # --------------------------------------------------------------------------- @@ -1474,12 +1473,12 @@ def _capture_agent(*args: Any, **kw: Any) -> Any: # --------------------------------------------------------------------------- -# §1.2 Path C — None user_input must not raise or produce None content +# None user_input must not raise or produce None content # --------------------------------------------------------------------------- class TestNoneUserInput: - """TESTING.md §1.2 Path C: _build_initial_messages must not pass None to + """_build_initial_messages must not pass None to HumanMessage when user_input is None.""" def test_none_user_input_no_none_human_message_content(self) -> None: @@ -1608,15 +1607,15 @@ def test_config_messages_precede_history(self) -> None: class TestAbandonOpenSpans: """An early consumer stop must not look like a provider failure. - The abandonment path used to reuse `close_open_spans`, which records a synthetic exception and + The abandonment path does not reuse `close_open_spans`, which records a synthetic exception and sets ERROR on every span still open. TELEMETRY-CONTRACT.md section 6 says an abandoned span stays - UNSET and carries `launchdarkly.stream.abandoned`, and `openai-agents` already did that. + UNSET and carries `launchdarkly.stream.abandoned`, as it does in `openai-agents`. Tested directly on the callback handler rather than through the streaming path. Reaching the state that matters, a chat or tool span still open at the break, needs a fake model that yields mid-turn, and with the fixtures here LangGraph has already run every callback by the time the - first chunk reaches the consumer. A test driven through `stream` therefore passes whether or not - the fix is present, which is worse than no test. + first chunk reaches the consumer. A test driven through `stream` would therefore pass whatever + the abandonment path did, which is worse than no test. """ def _handler_with_open_spans(self) -> tuple[Any, Any, Any]: @@ -1645,7 +1644,7 @@ def test_marks_open_spans_abandoned_and_leaves_them_unset(self) -> None: assert span.exceptions == [] def test_close_open_spans_still_fails_them_for_a_real_error(self) -> None: - # The failure path keeps its behaviour; only abandonment changed. + # The failure path still records an error; only abandonment leaves the status UNSET. from opentelemetry.trace import StatusCode bundle, chat, tool = self._handler_with_open_spans() @@ -1762,7 +1761,8 @@ class TestModelSpanTrackedBeforeContent: """`_start_model` must insert the span before writing content that can raise. A span created but never inserted is unreachable by close_open_spans, abandon_open_spans and the - end callbacks alike, so it never ends and never exports. `on_tool_start` already had this fix. + end callbacks alike, so it would never end and never export. `on_tool_start` follows the same + order. """ @pytest.mark.asyncio @@ -1791,10 +1791,10 @@ def _get_type(self) -> str: class TestZeroTokensAreStillReported: """A reported 0 is not a missing count. - `extract_llm_usage` read the llm_output fallback with `or`, so a genuine 0 was skipped. With both - counts at zero the bag came back all None, `lang_chain_span_usage` read that as "the provider - said nothing", and the run went unreported: a turn that completed and cost nothing became - indistinguishable from one that never reported. + `extract_llm_usage` must not read the llm_output fallback with `or`, which would skip a genuine + 0. With both counts at zero the bag would come back all None, `lang_chain_span_usage` would read + that as "the provider said nothing", and the run would go unreported: a turn that completed and + cost nothing would be indistinguishable from one that never reported. """ def test_a_zero_prompt_count_survives(self) -> None: @@ -1871,7 +1871,7 @@ async def _agenerate( @pytest.mark.asyncio async def test_usage_metadata_still_wins_when_both_are_present(self) -> None: - # The message-level sum stays authoritative where it has anything to say, so this change + # The message-level sum stays authoritative where it has anything to say, so the fallback # cannot double-count a provider that reports in both places. ctx, rec = _recording() llm = _FakeToolModel( @@ -1928,9 +1928,9 @@ def _explode(*_a: Any, **_k: Any) -> None: class TestInputWritesNeverLeakASpan: """Serialising the prompt must not be able to strand the root span. - The input content write ran before the guard that fails the root, so a raise there left it open: - never ended, never exported, so the run disappeared from AI Config Monitoring along with the - feature_flag event it carries. + The input content write sits inside the guard that fails the root. Ahead of it, a raise there + would leave the root open: never ended, never exported, so the run would disappear from AI + Config Monitoring along with the feature_flag event it carries. """ @pytest.mark.asyncio @@ -1985,10 +1985,10 @@ class TestCancellationEndsEverySpan: @pytest.mark.asyncio async def test_a_cancelled_run_still_exports_its_spans(self) -> None: - # asyncio.CancelledError is a BaseException, so `except Exception` never sees it. Before - # this, a cancelled run exported nothing at all: the root carries the feature_flag event - # and every launchdarkly.* attribute, so the run vanished from AI Config Monitoring rather - # than showing as incomplete. + # asyncio.CancelledError is a BaseException, so `except Exception` never sees it. Without + # a `finally`, a cancelled run would export nothing at all: the root carries the + # feature_flag event and every launchdarkly.* attribute, so the run would vanish from AI + # Config Monitoring rather than showing as incomplete. import asyncio ctx, rec = _recording() @@ -2099,9 +2099,9 @@ async def test_a_provider_that_reports_only_in_llm_output_still_totals_the_root( self, ) -> None: # The streaming walk reads usage_metadata off the astream payloads and sees nothing when a - # provider reports in llm_output.token_usage instead. Without the fallback the root wrote - # zero while its own chat spans held the billed tokens, which is the mismatch the blocking - # path already guards against. + # provider reports in llm_output.token_usage instead. Without the fallback the root would + # write zero while its own chat spans held the billed tokens, which is the mismatch the + # blocking path guards against too. from langchain_core.outputs import ChatGeneration, ChatResult class _LlmOutputOnlyModel(_FakeToolModel): diff --git a/packages/langchain-agents/tests/test_native_graph.py b/packages/langchain-agents/tests/test_native_graph.py index 53457ddc..83d7d5a4 100644 --- a/packages/langchain-agents/tests/test_native_graph.py +++ b/packages/langchain-agents/tests/test_native_graph.py @@ -1,6 +1,5 @@ """ -Tests for §2.2 native graph adapter (to_lang_graph) and LangChain-specific specs. -Reference: TESTING.md §2.2, §2.x.3 +Tests for the native graph adapter (to_lang_graph) and LangChain-specific specs. """ from __future__ import annotations @@ -232,7 +231,7 @@ def _patch_imports(mocks: dict[str, Any]) -> Any: # --------------------------------------------------------------------------- -# §2.2 Generic topology +# Generic topology # --------------------------------------------------------------------------- @@ -276,7 +275,7 @@ async def test_runner_returns_final_output(self) -> None: # --------------------------------------------------------------------------- -# §2.x.3 LangChain-specific specs +# LangChain-specific specs # --------------------------------------------------------------------------- @@ -878,7 +877,7 @@ async def test_config_tools_creates_tool_node(self) -> None: # --------------------------------------------------------------------------- -# §2.x.3 WorkflowState annotations resolve — real StateGraph (no mock) +# WorkflowState annotations resolve — real StateGraph (no mock) # --------------------------------------------------------------------------- diff --git a/packages/langchain-messages/src/launchdarkly_ai_langchain_messages/handler.py b/packages/langchain-messages/src/launchdarkly_ai_langchain_messages/handler.py index b30e8b80..64c20a4a 100644 --- a/packages/langchain-messages/src/launchdarkly_ai_langchain_messages/handler.py +++ b/packages/langchain-messages/src/launchdarkly_ai_langchain_messages/handler.py @@ -48,9 +48,8 @@ def _build_tools(config_tools: dict[str, Any]) -> list[dict[str, Any]]: # Not filtered to the tools that have a registered handler, unlike the TypeScript SDK's - # `buildTools`. That difference predates this span work and changes what the model is offered, - # not what the span reports, so it stays as it is: the catalog recorded below (via - # `to_tool_definitions`) is the catalog actually sent. + # `buildTools`. That difference changes what the model is offered, not what the span reports: + # the catalog recorded below (via `to_tool_definitions`) is the catalog actually sent. return [ { "type": "function", @@ -168,16 +167,14 @@ def _with_get_type(msg: Any) -> Any: """Adapts one LangChain message to the interface the client's ``lang_chain_span_messages`` narrows on. - Works around a version-skew bug in the shared client helper rather than fixing it there: - ``lang_chain_span_messages`` reads a message's role off ``_get_type()``, which older LangChain - releases exposed as the canonical accessor. The ``langchain-core`` release this package - actually depends on replaced it with a plain ``type`` field and dropped the method entirely, so - every real ``SystemMessage``/``HumanMessage``/``AIMessage`` reaching the client helper - unmodified is misclassified as role ``user`` with no error raised: ``getattr(raw, - '_get_type', None)`` returns ``None`` for a missing attribute rather than raising, and the - caller has no way to tell "the method is absent" from "this message really has no type". - Reported in this package's TELEMETRY-CONTRACT.md report rather than patched in - ``packages/client``, which is out of scope for this change. + Works around a version skew in the shared client helper: ``lang_chain_span_messages`` reads a + message's role off ``_get_type()``, which older LangChain releases exposed as the canonical + accessor. The ``langchain-core`` release this package depends on has a plain ``type`` field + instead and no such method, so every real ``SystemMessage``/``HumanMessage``/``AIMessage`` + reaching the client helper unmodified is misclassified as role ``user`` with no error raised: + ``getattr(raw, '_get_type', None)`` returns ``None`` for a missing attribute rather than + raising, and the caller has no way to tell "the method is absent" from "this message really has + no type". """ if callable(getattr(msg, "_get_type", None)): return msg @@ -632,9 +629,9 @@ async def _call_impl( # Behind the flag, because set_output_content_attributes is a no-op without it and # json.dumps is not. With tools and an outputFormat on a non-OpenAI provider the output - # here is the parsed object, so serialising one json.dumps refuses turned a successful - # run into a raised TypeError for a caller who had asked for no content at all. The - # outputFormat-only path above already guards the same work the same way. + # here is the parsed object, so serialising one json.dumps refuses would turn a + # successful run into a raised TypeError for a caller who asked for no content at all. + # The outputFormat-only path above already guards the same work the same way. if capture_content: output_str = output if isinstance(output, str) else json.dumps(output) set_output_content_attributes( diff --git a/packages/langchain-messages/src/launchdarkly_ai_langchain_messages/spans.py b/packages/langchain-messages/src/launchdarkly_ai_langchain_messages/spans.py index 1bbdc09b..c2c95756 100644 --- a/packages/langchain-messages/src/launchdarkly_ai_langchain_messages/spans.py +++ b/packages/langchain-messages/src/launchdarkly_ai_langchain_messages/spans.py @@ -9,10 +9,10 @@ section 1. Two provider keys, two different values, on purpose. ``gen_ai.system`` is the literal string -``langchain`` on every span this package opens, because that is what the handler shipped before -the span hierarchy landed. ``gen_ai.provider.name`` names *who served the model*, and semconv's -enum has no ``langchain`` member, so it follows :func:`serving_provider` instead: the configured -provider name, lower-cased. See TELEMETRY-CONTRACT.md section 9. +``langchain`` on every span this package opens, the value existing consumers of that key expect and +the one the TypeScript LangChain handlers emit. ``gen_ai.provider.name`` names *who served the +model*, and semconv's enum has no ``langchain`` member, so it follows :func:`serving_provider` +instead: the configured provider name, lower-cased. See TELEMETRY-CONTRACT.md section 9. """ from __future__ import annotations diff --git a/packages/langchain-messages/tests/test_handler.py b/packages/langchain-messages/tests/test_handler.py index d00e43fe..65e72a2b 100644 --- a/packages/langchain-messages/tests/test_handler.py +++ b/packages/langchain-messages/tests/test_handler.py @@ -1,6 +1,6 @@ """ Tests for launchdarkly-ai-langchain-messages handler. -Covers §1.1-1.10 (generic handler tests) plus TELEMETRY-CONTRACT.md sections 1-9. +Covers the generic handler behaviours plus TELEMETRY-CONTRACT.md sections 1-9. """ from __future__ import annotations @@ -163,7 +163,7 @@ def _make_tracer_patch(mock_span: MagicMock) -> Any: # --------------------------------------------------------------------------- -# §1.1 Factory function and metadata +# Factory function and metadata # --------------------------------------------------------------------------- @@ -195,7 +195,7 @@ def test_multiple_calls_return_independent_instances(self) -> None: # --------------------------------------------------------------------------- -# §1.2 Prompt construction +# Prompt construction # --------------------------------------------------------------------------- @@ -315,7 +315,7 @@ async def test_returns_text_from_mixed_thinking_and_text_blocks(self) -> None: # --------------------------------------------------------------------------- -# §1.3 Tool conversion +# Tool conversion # --------------------------------------------------------------------------- @@ -368,7 +368,7 @@ async def test_empty_tools_no_tools_sent(self) -> None: # --------------------------------------------------------------------------- -# §1.4 Tool execution loop +# Tool execution loop # --------------------------------------------------------------------------- @@ -577,8 +577,8 @@ async def test_every_span_is_ended(self) -> None: class TestRootSpanAttributes: async def test_gen_ai_system_is_the_literal_langchain(self) -> None: - # TELEMETRY-CONTRACT.md section 9: Python used to set this to the configured provider, - # lower-cased. TypeScript's LangChain handlers keep it the constant `langchain` regardless. + # TELEMETRY-CONTRACT.md section 9: the constant `langchain` regardless of the configured + # provider, matching TypeScript's LangChain handlers. ctx, rec = _recording() from launchdarkly_ai_langchain_messages import create_langchain_messages_handler @@ -1016,7 +1016,7 @@ async def test_rethrows_error(self) -> None: # --------------------------------------------------------------------------- -# §1.9 Structured output — withStructuredOutput +# Structured output — withStructuredOutput # --------------------------------------------------------------------------- @@ -1097,7 +1097,7 @@ async def test_token_usage_from_usage_metadata_when_structured_output(self) -> N # --------------------------------------------------------------------------- -# §1.7 Convenience export +# Convenience export # --------------------------------------------------------------------------- @@ -1144,7 +1144,7 @@ def test_callable_without_extra_kwargs(self) -> None: # --------------------------------------------------------------------------- -# §1.8 Streaming +# Streaming # --------------------------------------------------------------------------- @@ -1379,7 +1379,7 @@ async def test_emits_no_content_by_default_on_the_streaming_path(self) -> None: # --------------------------------------------------------------------------- -# §1.10 MAX_STEPS cap +# MAX_STEPS cap # --------------------------------------------------------------------------- @@ -1736,10 +1736,10 @@ async def aclose(self) -> None: class TestOpenToolSpanIsNeverLeaked: """A BaseException while a tool runs must still close the execute_tool span. - The streaming `finally` closed the model span and the root, but the in-flight tool span was held - only by a local. `except Exception` does not see a `CancelledError` or a `GeneratorExit`, so a - tool cancelled mid-flight left its span open and unexported: the trace showed a closed parent - above a child that never arrived. + The streaming `finally` closes the model span and the root, and the in-flight tool span too. + `except Exception` does not see a `CancelledError` or a `GeneratorExit`, so if only a local held + that span, a tool cancelled mid-flight would leave it open and unexported: the trace would show + a closed parent above a child that never arrived. """ @pytest.mark.asyncio @@ -1778,9 +1778,9 @@ async def _cancelled_tool(_: Any) -> Any: tools = rec.named("execute_tool ") assert len(tools) == 1 assert tools[0].ended == 1, "the execute_tool span leaked" - # `cancelled`, not `abandoned`. This test used to assert the latter, which is what the defect - # looked like: nothing here chose to stop reading, a CancelledError ended the run underneath - # the consumer. The consumer-break test still asserts `abandoned`, which is that word's case. + # `cancelled`, not `abandoned`: nothing here chose to stop reading, a CancelledError ended + # the run underneath the consumer. The consumer-break test asserts `abandoned`, which is + # that word's case. assert tools[0].attributes["launchdarkly.run.cancelled"] is True assert "launchdarkly.stream.abandoned" not in tools[0].attributes assert rec.root.ended == 1 @@ -1789,9 +1789,9 @@ async def _cancelled_tool(_: Any) -> Any: class TestStructuredTurnChatSpanNeverLeaks: """The structured-output turn has no `finally`, so a raise outside its guard is unrecoverable. - The output content write and the span finish sat outside the try that fails the chat span. A raise - while serialising the parsed object left the span open and unexported, and dropped the turn from - the run total even though the provider had already billed it. + The output content write and the span finish sit inside the try that fails the chat span. + Outside it, a raise while serialising the parsed object would leave the span open and + unexported, and drop the turn from the run total even though the provider had already billed it. """ @pytest.mark.asyncio @@ -1977,10 +1977,10 @@ class _Unserialisable: class TestStreamingChatSpanAndTokens: """The streaming path must fail its span and keep its tokens when content serialisation raises. - The content write and the span finish sat outside the try that fails the chat span, and the usage - was accumulated after both. A raise while serialising completion content left the span for the - `finally` to end as abandoned, which reads as a consumer who walked away rather than as the - failure it was, and dropped a turn the provider had already billed. + If the content write and the span finish sat outside the try that fails the chat span, with the + usage accumulated after them, a raise while serialising completion content would leave the span + for the `finally` to end as abandoned, which reads as a consumer who walked away rather than as + the failure it was, and drop a turn the provider had already billed. """ def _exploding_llm(self) -> Any: @@ -2044,10 +2044,10 @@ async def test_the_tokens_already_billed_survive(self) -> None: class TestInputWritesNeverLeakASpan: """Serialising the prompt must not be able to strand a span. - The input content write ran before the guard that fails the span it writes to. A raise there left - the chat span open on the structured path, and on both root paths left the root open: never - ended, never exported, so the run disappeared from AI Config Monitoring along with the - feature_flag event it carries. + The input content write sits inside the guard that fails the span it writes to. Ahead of it, a + raise there would leave the chat span open on the structured path, and on both root paths leave + the root open: never ended, never exported, so the run would disappear from AI Config Monitoring + along with the feature_flag event it carries. """ def _unserialisable_prompt_config(self) -> dict[str, Any]: @@ -2105,13 +2105,13 @@ async def test_the_streaming_root_still_ends(self) -> None: class TestChatSpanInputWritesAreGuarded: - """The per-turn chat span needs the same guard the root got. + """The per-turn chat span needs the same guard the root has. - The input write for each `chat` span ran before the try that fails it. On the blocking path a - raise left the child open and unexported while the root was failed, and there is no `finally` - there to recover it. On the streaming path the raise reached the outer `finally` with - open_model_span still set, so the span was ended as abandoned: a content failure that reads as a - consumer walking away. + The input write for each `chat` span sits inside the try that fails it. Ahead of it, on the + blocking path a raise would leave the child open and unexported while the root was failed, and + there is no `finally` there to recover it. On the streaming path the raise would reach the outer + `finally` with open_model_span still set, so the span would be ended as abandoned: a content + failure that reads as a consumer walking away. """ def _exploding_input(self) -> Any: @@ -2199,10 +2199,10 @@ class TestCancellationEndsEverySpan: @pytest.mark.asyncio async def test_a_cancelled_run_still_exports_its_spans(self) -> None: - # asyncio.CancelledError is a BaseException, so `except Exception` never sees it. Before - # this, a cancelled run exported nothing at all: the root carries the feature_flag event and - # every launchdarkly.* attribute, so the run vanished from AI Config Monitoring rather than - # showing as incomplete. + # asyncio.CancelledError is a BaseException, so `except Exception` never sees it. Without + # a `finally`, a cancelled run would export nothing at all: the root carries the + # feature_flag event and every launchdarkly.* attribute, so the run would vanish from AI + # Config Monitoring rather than showing as incomplete. import asyncio from launchdarkly_ai_langchain_messages import create_langchain_messages_handler diff --git a/packages/openai-agents/src/launchdarkly_ai_openai_agents/handler.py b/packages/openai-agents/src/launchdarkly_ai_openai_agents/handler.py index ac97a9aa..82bb9424 100644 --- a/packages/openai-agents/src/launchdarkly_ai_openai_agents/handler.py +++ b/packages/openai-agents/src/launchdarkly_ai_openai_agents/handler.py @@ -78,9 +78,9 @@ def _build_agent_tools( tool_handlers: dict[str, Any], ) -> list[Any]: # Not filtered to the tools that have a registered handler, unlike the TypeScript SDK, which - # excludes an unregistered tool from the catalog entirely. This difference predates the span - # work and changes what the model is offered, not what a span reports, so it is left as it is; - # `to_tool_definitions` below records the catalog actually sent, whatever it is. + # excludes an unregistered tool from the catalog entirely. This difference changes what the + # model is offered, not what a span reports; `to_tool_definitions` below records the catalog + # actually sent, whatever it is. import importlib agents_mod = importlib.import_module("agents") @@ -207,13 +207,11 @@ def _build_agent_and_prompt( tools = _build_agent_tools(config.get("tools") or {}, tool_handlers) - # `outputFormat` is not wired into `Agent(output_type=...)` here, matching this handler's - # pre-existing behaviour (and unlike the TypeScript handler, which does wire it): the Python - # Agents SDK requires a concrete Python type for `output_type`, not a raw JSON Schema dict (see - # `utils.build_output_type`'s docstring). Fixing that gap is a "what is sent to the model" - # change, not a telemetry one, so it is left alone. `_call_impl` still returns the parsed - # `final_output` object as-is when `outputFormat` is configured, matching the pre-existing - # return-shape contract. + # `outputFormat` is not wired into `Agent(output_type=...)` here (unlike the TypeScript handler, + # which does wire it): the Python Agents SDK requires a concrete Python type for `output_type`, + # not a raw JSON Schema dict (see `utils.build_output_type`'s docstring). `_call_impl` returns + # the parsed `final_output` object as-is when `outputFormat` is configured, matching the + # handler's return-shape contract. agent = Agent( name="assistant", model=config.get("model", {}).get("name", "gpt-4o"), @@ -295,10 +293,11 @@ async def on_llm_end(self, context: Any, agent: Any, response: Any) -> None: def _tool_span_key(context: Any, tool: Any) -> str: """The key an open tool span is filed under, computed the same way by both hooks. - ``on_tool_start`` used to fall back to the tool name when ``tool_call_id`` was absent, while - ``on_tool_end`` read ``str(context.tool_call_id)`` with no fallback. An absent id therefore - filed the span under the tool name and looked for it under the string ``"None"``, so the span - never closed on success and lived until process teardown. + ``on_tool_start`` and ``on_tool_end`` must agree on the fallback for an absent + ``tool_call_id``. If one fell back to the tool name while the other read + ``str(context.tool_call_id)``, the span would be filed under the tool name and looked for + under the string ``"None"``, so it would never close on success and would live until + process teardown. Mirrors the single ``callId`` helper the TypeScript handler shares between its two hooks. """ diff --git a/packages/openai-agents/src/launchdarkly_ai_openai_agents/native_graph.py b/packages/openai-agents/src/launchdarkly_ai_openai_agents/native_graph.py index a8b8a4a1..7d446e7a 100644 --- a/packages/openai-agents/src/launchdarkly_ai_openai_agents/native_graph.py +++ b/packages/openai-agents/src/launchdarkly_ai_openai_agents/native_graph.py @@ -234,7 +234,7 @@ async def on_agent_start(self, context: Any, agent: Any) -> None: if history: # config.instructions takes priority over config.messages, so skip # config conversation turns when instructions are set (parity with the - # single-node handler and TESTING.md §1.11 composition order). + # single-node handler's history composition order). config_messages = ( [] if root.config.get("instructions") diff --git a/packages/openai-agents/tests/test_graph.py b/packages/openai-agents/tests/test_graph.py index 29c6af40..9050e57f 100644 --- a/packages/openai-agents/tests/test_graph.py +++ b/packages/openai-agents/tests/test_graph.py @@ -1,6 +1,5 @@ """ -Tests for §2.1 graph convenience wrapper (openai_graph) and §2.x.4 full coverage. -Reference: TESTING.md §2.1, §2.x.4 +Tests for the graph convenience wrapper (openai_graph), with full coverage. """ from unittest.mock import MagicMock, patch diff --git a/packages/openai-agents/tests/test_handler.py b/packages/openai-agents/tests/test_handler.py index 75e20003..700c2fad 100644 --- a/packages/openai-agents/tests/test_handler.py +++ b/packages/openai-agents/tests/test_handler.py @@ -1,11 +1,11 @@ """ Tests for launchdarkly-ai-openai-agents handler. -TELEMETRY-CONTRACT.md sections 1-9 for the span tree, and TESTING.md §1 for the generic handler -behaviours. Rewritten from the pre-span-work version: this handler drives the ``agents`` SDK's own -``Runner``, which owns the per-turn loop, so a test drives spans by calling the ``RunHooks`` -callbacks (``on_llm_start`` / ``on_llm_end`` / ``on_tool_start`` / ``on_tool_end``) the way the real -``Runner`` would, rather than by mocking a client response directly. +TELEMETRY-CONTRACT.md sections 1-9 for the span tree, plus the generic handler behaviours. This +handler drives the ``agents`` SDK's own ``Runner``, which owns the per-turn loop, so a test drives +spans by calling the ``RunHooks`` callbacks (``on_llm_start`` / ``on_llm_end`` / ``on_tool_start`` / +``on_tool_end``) the way the real ``Runner`` would, rather than by mocking a client response +directly. """ from __future__ import annotations @@ -179,8 +179,8 @@ def _patched_agents(agents_mod: Any) -> Any: def _make_run_result( output: str = "hello", input_tokens: int = 10, output_tokens: int = 5 ) -> Any: - """The pre-span-era flat mock result, kept for the restored non-telemetry tests that predate - the ``RunHooks``-driven span work and never touch spans or usage attribution.""" + """A flat mock result, for the non-telemetry tests that never touch spans or usage + attribution.""" usage = MagicMock() usage.input_tokens = input_tokens usage.output_tokens = output_tokens @@ -201,8 +201,8 @@ async def _empty_async_gen() -> AsyncIterator[Any]: def _mock_agents_module(run_result: Any) -> Any: - """The pre-span-era fully-flat ``agents`` mock: ``Runner.run``/``run_streamed`` never invoke - ``hooks``. Kept for the restored tests that only care about prompt/tool wiring, not spans. + """A fully-flat ``agents`` mock: ``Runner.run``/``run_streamed`` never invoke ``hooks``. For + the tests that only care about prompt/tool wiring, not spans. """ mock = MagicMock() mock.Agent = MagicMock(return_value=MagicMock()) @@ -333,7 +333,7 @@ def _recording() -> Any: # --------------------------------------------------------------------------- -# §1.1 Factory +# Factory # --------------------------------------------------------------------------- @@ -355,7 +355,7 @@ def test_provides_for_values_are_correct(self) -> None: # --------------------------------------------------------------------------- -# §1.2 Prompt construction +# Prompt construction # --------------------------------------------------------------------------- @@ -501,7 +501,7 @@ def test_none_user_input_becomes_empty_string(self) -> None: # --------------------------------------------------------------------------- -# §1.3 / §1.4 Tool conversion and execution +# Tool conversion and execution # --------------------------------------------------------------------------- @@ -596,7 +596,7 @@ def _sync_wrapper(args: Any) -> Any: # --------------------------------------------------------------------------- -# §1.4 Tool execution loop (pre-span-era; restored, not telemetry) +# Tool execution loop (not telemetry) # --------------------------------------------------------------------------- @@ -1099,7 +1099,7 @@ async def test_keeps_malformed_tool_arguments_verbatim(self) -> None: # --------------------------------------------------------------------------- -# §1.6 Error handling (top-level, no partial usage) +# Error handling (top-level, no partial usage) # --------------------------------------------------------------------------- @@ -1147,10 +1147,10 @@ class TestCancellationEndsEverySpan: """TELEMETRY-CONTRACT.md section 6: a `finally` owns every end.""" async def test_a_cancelled_run_still_exports_its_spans(self) -> None: - # asyncio.CancelledError is a BaseException, so `except Exception` never sees it. Before - # this, a cancelled run exported nothing at all: the root carries the feature_flag event - # and every launchdarkly.* attribute, so the run vanished from AI Config Monitoring rather - # than showing as incomplete. + # asyncio.CancelledError is a BaseException, so `except Exception` never sees it. Without + # a `finally`, a cancelled run would export nothing at all: the root carries the + # feature_flag event and every launchdarkly.* attribute, so the run would vanish from AI + # Config Monitoring rather than showing as incomplete. import asyncio async def never_returns( @@ -1202,7 +1202,7 @@ async def never_returns( # --------------------------------------------------------------------------- -# §1.7 Convenience export +# Convenience export # --------------------------------------------------------------------------- @@ -1418,7 +1418,7 @@ async def test_emits_no_content_by_default_on_the_streaming_path(self) -> None: # --------------------------------------------------------------------------- -# §1.9 Output format (build_output_type) +# Output format (build_output_type) # --------------------------------------------------------------------------- @@ -1463,12 +1463,12 @@ async def run(agent: Any, prompt: str, hooks: Any = None, **kw: Any) -> Any: # --------------------------------------------------------------------------- -# §1.2 Path C — None user_input must not produce None prompt +# None user_input must not produce None prompt # --------------------------------------------------------------------------- class TestNoneUserInput: - """TESTING.md §1.2 Path C: When user_input is None, the prompt passed to + """When user_input is None, the prompt passed to Runner.run must be '' (empty string), not None.""" async def test_none_user_input_instructions_path_prompt_is_empty_string( @@ -1481,9 +1481,8 @@ async def test_none_user_input_instructions_path_prompt_is_empty_string( run_result = _make_run_result("ok") agents_mock = _mock_agents_module(run_result) - # `hooks` is accepted (and ignored) here because `_call_impl` now always passes - # `hooks=hooks` to `Runner.run` — a genuine signature change from the pre-span-work - # handler this test predates, per the assignment's adaptation rule. + # `hooks` is accepted (and ignored) here because `_call_impl` always passes + # `hooks=hooks` to `Runner.run`. async def _spy_run(agent: Any, prompt: Any, hooks: Any = None) -> Any: captured_prompts.append(prompt) return run_result @@ -1819,10 +1818,10 @@ async def _raising_run(*_a: Any, **_k: Any) -> Any: class TestCancellationDoesNotDependOnTelemetry: """Stopping the vendor's run is not telemetry, so no span may gate it. - The cancel sat inside a `span is not None` guard. Without the `otel` extra there is no root span - at all, so an early consumer break never cancelled the Runner and its background task kept calling - the model and spending money: the exact failure this teardown exists to prevent, reintroduced by - an install choice that has nothing to do with tracing. + The cancel does not sit inside a `span is not None` guard. Without the `otel` extra there is no + root span at all, so behind such a guard an early consumer break would never cancel the Runner + and its background task would keep calling the model and spending money: the exact failure this + teardown exists to prevent, caused by an install choice that has nothing to do with tracing. """ async def test_an_abandoned_stream_cancels_the_runner_with_telemetry_off( @@ -1872,9 +1871,9 @@ def run_streamed(agent: Any, prompt: Any, hooks: Any = None, **_k: Any) -> Any: class TestInputWritesNeverLeakASpan: """Serialising the prompt must not be able to strand the root span. - The input content write ran before the guard that fails the root, so a raise there left it open: - never ended, never exported, so the run disappeared from AI Config Monitoring along with the - feature_flag event it carries. + The input content write sits inside the guard that fails the root. Ahead of it, a raise there + would leave the root open: never ended, never exported, so the run would disappear from AI + Config Monitoring along with the feature_flag event it carries. """ async def test_the_blocking_root_still_ends(self) -> None: @@ -1929,9 +1928,9 @@ class TestChatSpanIsNeverStranded: """on_llm_end pops the span, so nothing else can end it if the write raises. close_open_spans reaches only spans the hook object still holds. on_llm_end clears - open_model_span first, so a serialisation failure after that point left the chat span open with - nothing tracking it: never ended, never exported. The langchain-agents callback guards the - identical shape for the identical reason. + open_model_span first, so an unguarded serialisation failure after that point would leave the + chat span open with nothing tracking it: never ended, never exported. The langchain-agents + callback guards the identical shape for the identical reason. """ async def test_an_unserialisable_completion_still_ends_the_chat_span(self) -> None: @@ -2033,9 +2032,9 @@ async def test_a_tool_result_that_will_not_serialise_still_ends_the_span( self, ) -> None: # A tool result comes from the caller's own function, so it can be anything, including - # something json.dumps refuses. Before this the write ran after the pop and outside any guard, - # so the span was untracked and unended: never exported, and a reader saw a tool that started - # and never returned. + # something json.dumps refuses. If the write ran after the pop and outside any guard, the + # span would be untracked and unended: never exported, and a reader would see a tool that + # started and never returned. class Unserialisable: pass diff --git a/packages/openai-agents/tests/test_native_graph.py b/packages/openai-agents/tests/test_native_graph.py index 2df4696f..40cc1a98 100644 --- a/packages/openai-agents/tests/test_native_graph.py +++ b/packages/openai-agents/tests/test_native_graph.py @@ -1,6 +1,5 @@ """ -Tests for §2.2 native graph adapter (to_openai_agents) and OpenAI-specific specs. -Reference: TESTING.md §2.2, §2.x.6 +Tests for the native graph adapter (to_openai_agents) and OpenAI-specific specs. """ from __future__ import annotations @@ -150,7 +149,7 @@ async def on_agent_start(self, context: Any, agent: Any) -> None: # --------------------------------------------------------------------------- -# §2.2 Generic topology +# Generic topology # --------------------------------------------------------------------------- @@ -287,7 +286,7 @@ async def test_runner_starts_at_root_and_returns_output(self) -> None: # --------------------------------------------------------------------------- -# §2.x.6 OpenAI-specific specs +# OpenAI-specific specs # --------------------------------------------------------------------------- @@ -614,7 +613,7 @@ async def _run_and_fire_hook(agent: Any, text: str, hooks: Any = None) -> Any: @pytest.mark.asyncio async def test_path_entries_are_unique(self) -> None: - """§2.x.6 — each node key must appear at most once in path. + """Each node key must appear at most once in path. When on_handoff adds the child key, on_agent_start must not add it again. """ # Two-node graph: root -> child diff --git a/packages/openai-agents/tests/test_utils.py b/packages/openai-agents/tests/test_utils.py index 2586ddec..9286718b 100644 --- a/packages/openai-agents/tests/test_utils.py +++ b/packages/openai-agents/tests/test_utils.py @@ -1,6 +1,5 @@ """ -Tests for §2.x.5 build_output_type utility. -Reference: TESTING.md §2.x.5 +Tests for the build_output_type utility. """ from launchdarkly_ai_openai_agents.utils import build_output_type @@ -52,7 +51,7 @@ def test_required_populated_from_all_property_keys(self) -> None: assert "b" in required def test_required_omitted_when_properties_empty(self) -> None: - """§2.x.5 — required must not appear in schema when properties is empty. + """``required`` must not appear in schema when properties is empty. When ``outputFormat`` has a ``properties`` key that maps to ``{}``, the resulting schema must not include ``"required": []``. An empty diff --git a/packages/openai-messages/src/launchdarkly_ai_openai_messages/handler.py b/packages/openai-messages/src/launchdarkly_ai_openai_messages/handler.py index a404fdcb..303dd7ff 100644 --- a/packages/openai-messages/src/launchdarkly_ai_openai_messages/handler.py +++ b/packages/openai-messages/src/launchdarkly_ai_openai_messages/handler.py @@ -49,8 +49,8 @@ def _build_tools(config_tools: dict[str, Any]) -> list[dict[str, Any]]: # Not filtered to the tools that have a registered handler, unlike the TypeScript SDK. That - # difference predates this span work and changes what the model is offered, not what the span - # reports, so it stays as it is: the catalog recorded on the span is the catalog actually sent. + # difference changes what the model is offered, not what the span reports: the catalog recorded + # on the span is the catalog actually sent. return [ { "type": "function", @@ -496,8 +496,8 @@ async def _stream_gen( if previous_response_id: stream_params["previous_response_id"] = previous_response_id # Tools are forwarded on every streaming turn, not only the first, unlike the blocking - # path and unlike the TypeScript SDK. That difference predates this span work and - # changes what the model is offered, not what the span reports, so it stays as it is. + # path and unlike the TypeScript SDK. That difference changes what the model is + # offered, not what the span reports. if tools: stream_params["tools"] = tools diff --git a/packages/openai-messages/tests/test_handler.py b/packages/openai-messages/tests/test_handler.py index 4668976f..6a8c8d7b 100644 --- a/packages/openai-messages/tests/test_handler.py +++ b/packages/openai-messages/tests/test_handler.py @@ -1,7 +1,6 @@ """ Tests for launchdarkly-ai-openai-messages handler. -Covers §1.1-1.9. -Reference: TESTING.md §1, TELEMETRY-CONTRACT.md +Covers the generic handler behaviours and TELEMETRY-CONTRACT.md. """ from __future__ import annotations @@ -93,7 +92,7 @@ def mock_openai(mocker): # --------------------------------------------------------------------------- -# §1.1 Factory function and metadata +# Factory function and metadata # --------------------------------------------------------------------------- @@ -127,7 +126,7 @@ def test_multiple_calls_return_independent_instances( # --------------------------------------------------------------------------- -# §1.2 Prompt construction +# Prompt construction # --------------------------------------------------------------------------- @@ -264,7 +263,7 @@ async def test_path_c_both_instructions_and_messages_messages_wins( # --------------------------------------------------------------------------- -# §1.3 Tool conversion +# Tool conversion # --------------------------------------------------------------------------- @@ -319,7 +318,7 @@ async def test_empty_tools_no_tools_sent(self, mock_openai: MagicMock) -> None: # --------------------------------------------------------------------------- -# §1.4 Tool execution loop +# Tool execution loop # --------------------------------------------------------------------------- @@ -407,7 +406,7 @@ async def test_multiple_consecutive_tool_calls( # --------------------------------------------------------------------------- -# §1.5 Telemetry — span recording +# Telemetry — span recording # --------------------------------------------------------------------------- @@ -1022,7 +1021,7 @@ async def test_still_writes_the_legacy_content_events_when_enabled( # --------------------------------------------------------------------------- -# §1.6 Error handling +# Error handling # --------------------------------------------------------------------------- @@ -1128,7 +1127,7 @@ async def test_rethrows_error(self, mock_openai: MagicMock) -> None: # --------------------------------------------------------------------------- -# §1.9 Structured output (outputFormat) +# Structured output (outputFormat) # --------------------------------------------------------------------------- @@ -1155,7 +1154,7 @@ async def test_output_format_uses_text_format_json_schema( # --------------------------------------------------------------------------- -# §1.7 Convenience export +# Convenience export # --------------------------------------------------------------------------- @@ -1203,7 +1202,7 @@ def test_callable_without_extra_kwargs(self, mock_openai: MagicMock) -> None: # --------------------------------------------------------------------------- -# §1.8 Streaming +# Streaming # --------------------------------------------------------------------------- @@ -1343,11 +1342,11 @@ async def _bad_ctx() -> AsyncGenerator[Any, None]: async def test_tools_forwarded_on_second_streaming_turn( self, mock_openai: MagicMock ) -> None: - """§1.8 - tools must appear in stream_params on every streaming turn. + """Tools must appear in stream_params on every streaming turn. - This is a pre-existing Python-only behaviour that diverges from the TypeScript SDK (which - does not resend tools after the first turn). It changes what the model is offered, not what - the span reports, so this test only pins that the behaviour is unchanged by the span work. + This is a Python-only behaviour that diverges from the TypeScript SDK (which does not + resend tools after the first turn). It changes what the model is offered, not what the span + reports, so this test only pins the behaviour, independently of span recording. """ import launchdarkly_ai_openai_messages.spans as spans_mod from launchdarkly_ai_openai_messages import create_openai_messages_handler @@ -1436,12 +1435,12 @@ async def _stream_ctx(**kwargs: Any) -> AsyncGenerator[Any, None]: # --------------------------------------------------------------------------- -# §1.2 Path C — None user_input must not produce None content +# None user_input must not produce None content # --------------------------------------------------------------------------- class TestNoneUserInput: - """TESTING.md §1.2 Path C: When user_input is None, the user-role message + """When user_input is None, the user-role message content sent to the provider must be '' not None.""" async def test_none_user_input_instructions_path_no_none_content( @@ -1470,12 +1469,12 @@ async def _capture(**kwargs: Any) -> Any: # --------------------------------------------------------------------------- -# §1.10 MAX_STEPS cap +# MAX_STEPS cap # --------------------------------------------------------------------------- class TestMaxStepsCap: - """TESTING.md §1.10: The tool loop must break with an error after MAX_STEPS (5) iterations.""" + """The tool loop must break with an error after MAX_STEPS (5) iterations.""" def _tool_response(self) -> MagicMock: return _make_response( @@ -1559,7 +1558,7 @@ async def _iter() -> AsyncGenerator: # --------------------------------------------------------------------------- -# §1.5 Streaming telemetry (do not patch _HAS_OTEL=False) +# Streaming telemetry (do not patch _HAS_OTEL=False) # --------------------------------------------------------------------------- @@ -1886,9 +1885,9 @@ def test_defaults_to_off(self) -> None: class TestChatSpanNeverLeaks: """A raise while recording conversation content must not leave the chat span open. - The content writes on both sides of the provider call used to sit outside the try that fails the - span. A raise there failed only the root, and the chat span was never ended, so the exporter - never saw the turn. + The content writes on both sides of the provider call sit inside the try that fails the span. + Outside it, a raise there would fail only the root, and the chat span would never end, so the + exporter would never see the turn. """ async def test_an_unserialisable_output_still_ends_the_chat_span( @@ -1948,10 +1947,10 @@ def output(self) -> Any: class TestOpenToolSpanIsNeverLeaked: """A BaseException while a tool runs must still close the execute_tool span. - The streaming `finally` closed the model span and the root, but the in-flight tool span was held - only by a local. `except Exception` does not see a `CancelledError` or a `GeneratorExit`, so a - tool cancelled mid-flight left its span open and unexported: the trace showed a closed parent - above a child that never arrived. + The streaming `finally` closes the model span and the root, and the in-flight tool span too. + `except Exception` does not see a `CancelledError` or a `GeneratorExit`, so if only a local held + that span, a tool cancelled mid-flight would leave it open and unexported: the trace would show + a closed parent above a child that never arrived. """ async def test_a_tool_cancelled_mid_flight_still_ends_its_span( @@ -2004,9 +2003,9 @@ async def _cancelled_tool(_: Any) -> Any: tools = rec.named("execute_tool ") assert len(tools) == 1 assert tools[0].ended == 1, "the execute_tool span leaked" - # `cancelled`, not `abandoned`. This test used to assert the latter, which is what the defect - # looked like: nothing here chose to stop reading, a CancelledError ended the run underneath - # the consumer. The consumer-break test still asserts `abandoned`, which is that word's case. + # `cancelled`, not `abandoned`: nothing here chose to stop reading, a CancelledError ended + # the run underneath the consumer. The consumer-break test asserts `abandoned`, which is + # that word's case. assert tools[0].attributes["launchdarkly.run.cancelled"] is True assert "launchdarkly.stream.abandoned" not in tools[0].attributes assert rec.root.ended == 1 @@ -2015,10 +2014,10 @@ async def _cancelled_tool(_: Any) -> Any: class TestStreamingChatSpanAndTokens: """The streaming path must fail its span and keep its tokens when content serialisation raises. - The content write and the span finish sat after the try that fails the chat span, and the usage - was accumulated last. A raise while serialising the response therefore left the span for `finally` - to end as abandoned, which reads as a consumer who walked away rather than as the failure it was, - and dropped a turn the provider had already billed. + If the content write and the span finish sat outside the try that fails the chat span, with the + usage accumulated after them, a raise while serialising the response would leave the span for + `finally` to end as abandoned, which reads as a consumer who walked away rather than as the + failure it was, and drop a turn the provider had already billed. """ def _exploding_stream(self, mock_openai: MagicMock) -> None: @@ -2097,10 +2096,10 @@ class TestCancellationEndsEverySpan: async def test_a_cancelled_run_still_exports_its_spans( self, mock_openai: MagicMock ) -> None: - # asyncio.CancelledError is a BaseException, so `except Exception` never sees it. Before - # this, a cancelled run exported nothing at all: the root carries the feature_flag event - # and every launchdarkly.* attribute, so the run vanished from AI Config Monitoring rather - # than showing as incomplete. + # asyncio.CancelledError is a BaseException, so `except Exception` never sees it. Without + # a `finally`, a cancelled run would export nothing at all: the root carries the + # feature_flag event and every launchdarkly.* attribute, so the run would vanish from AI + # Config Monitoring rather than showing as incomplete. import asyncio async def never_returns(*args: Any, **kwargs: Any) -> Any: diff --git a/tests/test_cross_handler_parity.py b/tests/test_cross_handler_parity.py index 9ab77678..f7058447 100644 --- a/tests/test_cross_handler_parity.py +++ b/tests/test_cross_handler_parity.py @@ -365,16 +365,16 @@ def test_the_langchain_provider_name_is_the_configured_name( "launchdarkly.graph.key", "launchdarkly.stream.abandoned", # A blocking run that was cancelled. asyncio.CancelledError is a BaseException, so it walks past - # every `except Exception` a handler writes, and without a `finally` the run exported no span at - # all. UNSET plus this marker, never ERROR, for the same reason as the abandoned stream above: - # nothing failed, the caller went away. Python only. TypeScript has no cancellation that skips a - # `catch`, so this key has no counterpart there and its absence is not drift. + # every `except Exception` a handler writes, and without a `finally` the run would export no + # span at all. UNSET plus this marker, never ERROR, for the same reason as the abandoned stream + # above: nothing failed, the caller went away. Python only. TypeScript has no cancellation that + # skips a `catch`, so this key has no counterpart there and its absence is not drift. "launchdarkly.run.cancelled", "feature_flag", "feature_flag.key", "feature_flag.provider.name", "feature_flag.set.id", - # AIC-3230: evaluation-context identity on the root feature_flag event / span. + # Evaluation-context identity on the root feature_flag event / span. # `context.contextKeys` is the f-string prefix; the keys actually emitted are # `context.contextKeys.`. "feature_flag.context.id", @@ -385,7 +385,7 @@ def test_the_langchain_provider_name_is_the_configured_name( "launchdarkly.graph.path", } -#: Functions kept exported for one release that nothing calls any more. +#: Functions that stay exported for one release but that nothing calls. #: #: Their bodies are cut out before the scan below, rather than their keys being listed as expected. #: Listing the keys does not work: `gen_ai.prompt` is written by the live content writer *and* by @@ -612,8 +612,8 @@ class TestStreamingTeardownClosesToolSpans: `except Exception` does not see a `CancelledError` or a `GeneratorExit`, so the tool loop's own handler never runs for those, and the streaming `finally` is the only code left that can end the - span. Four of the six handlers once held the open tool span in a local that `finally` never read, - which exported a closed parent above a child that never arrived. + span. A handler that holds the open tool span in a local that `finally` never reads exports a + closed parent above a child that never arrives. Structural rather than behavioural on purpose: the leak is a property of which variables the teardown reads, and a behavioural test would need a cancellable tool per handler to say the same