From b365307ce322859b7c20f2a413b8e984c4691a00 Mon Sep 17 00:00:00 2001 From: Christie Williams Date: Fri, 2 Oct 2026 12:18:03 -0400 Subject: [PATCH 1/2] docs: describe current behaviour in comments, drop internal references Sweep comments, docstrings and the client README so they describe what the code does today rather than how it got there, and stop pointing at internal docs and systems customers cannot see: - drop TESTING.md / section-number / appendix references, keeping the descriptive section names - replace internal system names with LaunchDarkly-facing ones and drop Jira keys - rewrite development-history narration ("used to", "restored from the pre-rewrite file", "predates this span work", "before this ...") as statements of current behaviour and the reason for it No code, runtime strings or assertion messages change. Co-Authored-By: Claude Opus 5.5 --- packages/ai/tests/test_reexports.py | 3 +- .../launchdarkly_ai_claude_agents/handler.py | 17 ++-- .../native_graph.py | 2 +- packages/claude-agents/tests/test_builtins.py | 3 +- packages/claude-agents/tests/test_graph.py | 3 +- packages/claude-agents/tests/test_handler.py | 98 +++++++++---------- .../claude-agents/tests/test_native_graph.py | 5 +- .../handler.py | 4 +- .../claude-messages/tests/test_handler.py | 79 ++++++++------- packages/client/README.md | 4 +- .../src/launchdarkly_ai_server/content.py | 8 +- .../src/launchdarkly_ai_server/history.py | 4 +- .../src/launchdarkly_ai_server/judges.py | 4 +- .../src/launchdarkly_ai_server/types.py | 4 +- .../src/launchdarkly_ai_server/utils.py | 29 +++--- packages/client/tests/test_client.py | 14 ++- packages/client/tests/test_content.py | 10 +- packages/client/tests/test_evaluations_run.py | 6 +- packages/client/tests/test_graph.py | 12 +-- packages/client/tests/test_graph_stream.py | 5 +- .../tests/test_judge_message_history.py | 2 +- packages/client/tests/test_judges.py | 7 +- .../client/tests/test_ld_span_attributes.py | 2 +- packages/client/tests/test_lifecycle.py | 5 +- packages/client/tests/test_registry.py | 11 +-- packages/client/tests/test_schema.py | 3 +- packages/client/tests/test_sdk_info.py | 2 +- packages/client/tests/test_span_usage.py | 11 ++- .../client/tests/test_stream_conversation.py | 2 +- packages/client/tests/test_tracking.py | 3 +- packages/client/tests/test_trajectory.py | 6 +- packages/client/tests/test_utils.py | 5 +- .../messages.py | 2 +- .../native_graph.py | 2 +- packages/langchain-agents/tests/test_graph.py | 3 +- .../langchain-agents/tests/test_handler.py | 72 +++++++------- .../tests/test_native_graph.py | 9 +- .../handler.py | 29 +++--- .../spans.py | 8 +- .../langchain-messages/tests/test_handler.py | 78 +++++++-------- .../launchdarkly_ai_openai_agents/handler.py | 27 +++-- .../native_graph.py | 2 +- packages/openai-agents/tests/test_graph.py | 3 +- packages/openai-agents/tests/test_handler.py | 75 +++++++------- .../openai-agents/tests/test_native_graph.py | 9 +- packages/openai-agents/tests/test_utils.py | 5 +- .../handler.py | 8 +- .../openai-messages/tests/test_handler.py | 75 +++++++------- tests/test_cross_handler_parity.py | 16 +-- 49 files changed, 380 insertions(+), 416 deletions(-) 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..c23b17f1 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. +All three paths build that history through a single function, so they cannot drift apart. 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 From 88f136a6060ba7a52ec4c6bb24e4a69af3066bc0 Mon Sep 17 00:00:00 2001 From: Christie Williams Date: Fri, 2 Oct 2026 12:19:22 -0400 Subject: [PATCH 2/2] docs: drop a sentence repeated from the paragraph above Co-Authored-By: Claude Opus 5.5 --- packages/client/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/client/README.md b/packages/client/README.md index c23b17f1..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. -All three paths build that history through a single function, so they cannot drift apart. 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. +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.