Skip to content

test: fail the build when the six handlers drift apart again - #36

Draft
apucacao wants to merge 9 commits into
ag/py-telemetry-langchain-agentsfrom
ag/py-telemetry-drift-oracle
Draft

test: fail the build when the six handlers drift apart again#36
apucacao wants to merge 9 commits into
ag/py-telemetry-langchain-agentsfrom
ag/py-telemetry-drift-oracle

Conversation

@apucacao

@apucacao apucacao commented Aug 11, 2026

Copy link
Copy Markdown

Adds the tests that fail when the six handlers drift apart again.

Every handler package tests its own spans, and each was correct on its own terms while a single run emitted chat spans that disagreed about what a finish reason or a cached token was. Nothing tested that the six agree, which is the property that actually matters and the one that broke.

These live outside the packages, because no package can own an invariant about all six.

Two kinds of check

Shape checks call each package's span constructors directly with a recording tracer, so they need no provider mocks and cannot be fooled by a handler that never reaches its own span code. They pin the three span names, the root's operation attribute, and the rule that the launchdarkly.* identity and the feature_flag event appear on the root and nowhere else.

Two of them exist because of specific mistakes this work nearly shipped: gen_ai.system on the LangChain handlers, and gen_ai.provider.name being a passthrough instead of the binary Anthropic-or-OpenAI choice. A passthrough reads as obviously correct and reports bedrock for a request an OpenAI client made.

A vocabulary lock reads every attribute key, event name and naming template out of the sources and compares it to a committed set of 42, verified to match the TypeScript SDK exactly. It fails when a key is added, and separately when one stops being emitted. The second direction is the one that earns its keep: a key silently disappearing is how a dashboard goes blank with nothing failing. It caught its own regex being wrong while I wrote it, because the feature_flag event's attributes are built as a plain dict and never appear inside a set_attribute call.

Changing an attribute now means editing that list and saying why, which is the right amount of friction for a public contract with whatever reads the traces.

Verified by mutation, not by passing

Four deliberate breakages, each failing exactly one test and nothing else:

  1. renaming invoke_agent
  2. adding an unlisted attribute key
  3. turning the LangChain provider into a passthrough
  4. leaking the LD identity onto a tool span

Where this sits

Needs all six handler PRs (#30 to #35), because it asserts across all of them.

Tests: 844 to 913.


Note

Overview
CI runs bare uv run pytest instead of packages/*/tests, so repo-root tests/ (cross-package invariants) are no longer skipped silently—matching make test.

New tests/test_cross_handler_parity.py adds the oracle for all six handler packages:

  • Shape checks (recording tracer, no provider mocks): shared span API surface, fixed span names (invoke_agent, chat {model}, execute_tool {name}), LaunchDarkly identity and feature_flag only on the root, dual provider keys on the root, and LangChain-specific gen_ai.system / binary serving_provider rules.
  • Vocabulary lock: static scan of package sources vs committed EXPECTED_VOCABULARY (aligned with TypeScript SDK); fails on new keys and on keys that stop being emitted.
  • OpenLLMetry carrier runtime checks for indexed gen_ai.prompt.* / gen_ai.completion.* behind the capture gate.
  • Streaming teardown structural checks: inline tool loops must track open_tool_span in a finally; hook-based handlers must define/call abandon_open_spans from finally and keep abandonment separate from close_open_spans / fail_span.

Changing trace attributes now requires updating the committed vocabulary list with intent in the commit message.

Reviewed by Cursor Bugbot for commit 40ef593. Bugbot is set up for automated code reviews on this repo. Configure here.

@apucacao

Copy link
Copy Markdown
Author

bugbot run

Comment thread tests/test_cross_handler_parity.py
@apucacao
apucacao force-pushed the ag/py-telemetry-drift-oracle branch from c48aecb to 59890fa Compare August 11, 2026 20:43
@apucacao

Copy link
Copy Markdown
Author

bugbot run

Comment thread tests/test_cross_handler_parity.py
Comment thread tests/test_cross_handler_parity.py Outdated
@apucacao
apucacao force-pushed the ag/py-telemetry-drift-oracle branch from 59890fa to 2234b78 Compare August 11, 2026 21:01
@apucacao

Copy link
Copy Markdown
Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 2234b78. Configure here.

@apucacao
apucacao force-pushed the ag/py-telemetry-drift-oracle branch from 2234b78 to 8eee53f Compare August 11, 2026 21:18
@apucacao

Copy link
Copy Markdown
Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 8eee53f. Configure here.

@apucacao

Copy link
Copy Markdown
Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 8eee53f. Configure here.

@apucacao
apucacao force-pushed the ag/py-telemetry-drift-oracle branch from 8eee53f to 84ae935 Compare August 12, 2026 17:23
@apucacao

Copy link
Copy Markdown
Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 84ae935. Configure here.

@apucacao
apucacao force-pushed the ag/py-telemetry-drift-oracle branch from 84ae935 to e841913 Compare August 12, 2026 17:45
@apucacao

Copy link
Copy Markdown
Author

bugbot run

Comment thread tests/test_cross_handler_parity.py
Comment thread tests/test_cross_handler_parity.py Outdated
@apucacao
apucacao force-pushed the ag/py-telemetry-drift-oracle branch from e841913 to 8bee258 Compare August 12, 2026 18:02
@apucacao

Copy link
Copy Markdown
Author

bugbot run

@apucacao
apucacao force-pushed the ag/py-telemetry-drift-oracle branch from 8bee258 to d7ea5e4 Compare August 12, 2026 18:07
@apucacao

Copy link
Copy Markdown
Author

bugbot run

@apucacao

Copy link
Copy Markdown
Author

bugbot run

Comment thread tests/test_cross_handler_parity.py
@apucacao

Copy link
Copy Markdown
Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 5873187. Configure here.

@apucacao
apucacao force-pushed the ag/py-telemetry-drift-oracle branch from 5873187 to 71e4523 Compare August 13, 2026 00:33
@apucacao

Copy link
Copy Markdown
Author

bugbot run

Comment thread tests/test_cross_handler_parity.py
@apucacao
apucacao force-pushed the ag/py-telemetry-drift-oracle branch from 71e4523 to b6fd594 Compare August 13, 2026 00:55
@apucacao

Copy link
Copy Markdown
Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit b6fd594. Configure here.

@apucacao
apucacao force-pushed the ag/py-telemetry-drift-oracle branch from b6fd594 to 6becb8c Compare August 13, 2026 01:09
@apucacao
apucacao force-pushed the ag/py-telemetry-drift-oracle branch from b6fd594 to 6becb8c Compare August 13, 2026 01:09
@apucacao

Copy link
Copy Markdown
Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 6becb8c. Configure here.

Every handler package tests its own spans, and each was correct on its own
terms while a single run emitted `chat` spans that disagreed about what a
finish reason or a cached token was. Nothing tested that the six agree, which
is the property that actually matters and the one that broke.

These tests own that property. They live outside the packages because no
package can own an invariant about all six.

The shape checks call each package's span constructors directly with a
recording tracer, so they need no provider mocks and cannot be fooled by a
handler that never reaches its own span code. They pin the three span names,
the root's operation attribute, and the rule that the launchdarkly.* identity
and the feature_flag event appear on the root and nowhere else.

Two of them exist because of specific mistakes this port nearly shipped. One
pins `gen_ai.system` to the literal `langchain` on the two LangChain handlers,
where Python had been writing the configured provider name. The other pins
`gen_ai.provider.name` to a binary anthropic-or-openai choice, because it names
who served the model and anything that is not Anthropic is served by the
OpenAI client; a passthrough of the configured name reads as correct and
reports `bedrock` for a request an OpenAI client made.

The vocabulary lock reads every attribute key, event name and naming template
out of the sources and compares it to a committed set of 42. It fails when a
key is added, removed or renamed anywhere. That is deliberate: an attribute is
a public contract with whatever reads the traces, so changing one should mean
editing the list and saying why. The set was verified to match the TypeScript
SDK exactly.

The second half of the lock is the one that earns its keep: it fails when a key
stops being emitted, which is how a dashboard goes blank without anything
failing. It caught its own regex being wrong while I wrote it, because the
feature_flag event's attributes are built as a plain dict and never appear
inside a set_attribute call.

Verified by mutation rather than by passing. Renaming invoke_agent, adding an
unlisted key, turning the LangChain provider into a passthrough, and leaking
the LD identity onto a tool span each fail exactly one test and nothing else.
…carrier

Two faults in the lock I added, both pointed out by Bugbot on #36.

It pinned gen_ai.completion.0.role and .content, which appear only inside
set_openllmetry_completion. That helper has no call sites, so those literals
describe nothing the SDK emits. Deleting the dead helper would have failed the
lock for no reason, and the entries also gave cover to a genuine drop of the
live carrier. They now sit in a separate SUPERSEDED_VOCABULARY set that says
what it is and when to delete it.

The live OpenLLMetry keys were never verified at all. They are written as
f{prefix}.{index}.role, so there is no literal for a static scan to find, and
the lock was only ever seeing the two prefix arguments. That carrier is the one
LaunchDarkly's LLM trace view reads today, so dropping it renders an empty
transcript while every canonical attribute is still present and every static
check still passes.

Adds three runtime tests for it, including the capture gate. Checked both
directions by mutation: deleting the dead helper no longer fails anything, and
removing the live writes now fails exactly those tests.
…g the lock

CI ran `pytest packages/*/tests`, a glob that silently skips the repo-root
tests/ directory. The cross-handler oracle added in this layer therefore never
ran in CI at all: 72 tests, including every invariant no single package can own,
invisible to the build the moment they were written. Handler drift would not
have failed anything, which is the one thing the layer exists to do. Now runs
bare pytest, the same command make test uses, which collects both.

The vocabulary lock also let dead code satisfy it. Quarantining the superseded
helpers' keys was not enough, because gen_ai.prompt is written by the live
content writer AND by dead set_openllmetry_prompt, so naming it as expected let
the dead copy keep the lock green after the live write was removed. The scan now
cuts the superseded function bodies out of the source before looking, so only a
live write can satisfy anything, and the quarantine set is gone.

Checked by mutation: removing the live gen_ai.prompt writes now fails the lock
and the runtime carrier tests, where before it failed nothing.

Both found by Bugbot on #36, the CI one at High severity.
Four of the six handlers held an in-flight execute_tool span in a local their
streaming finally never read, so a tool cancelled mid-flight leaked its span. The
six handlers discharge the same duty in two shapes: three dispatch tools inline
and track a local, three go through a vendor hook object and keep the spans
there. This checks both shapes, and checks that the hook-based three still tell
abandonment apart from failure, which is the distinction that decides whether an
abandoned run reads as UNSET or as an error.

Structural rather than behavioural on purpose. The leak is a property of which
variables the teardown reads, and a behavioural version would need a cancellable
tool per handler to say the same thing six times.

Verified against three mutations: dropping the finally cleanup, renaming the
abandonment helper, and leaving the helper defined but uncalled. Each fails the
oracle.
Two of the three checks could not detect the thing they described.

The call-site check searched for abandon\w*( in the handler source, which the
helper's own def line satisfies. Three of these handlers define the helper in the
module that has to call it, so deleting the real call left the assertion green:
an unreachable abandonment helper passed. Definition lines are stripped first now.

The distinctness check compared the offsets of two different literals, which can
never be equal once both exist, so it passed unconditionally. It now reads the
two bodies: abandonment must not call fail_span and must mark the span abandoned,
and failure must still record the exception. A collapsed pair fails it.

Verified against three more mutations: dropping the abandon call in each of the
two handlers that define it locally, and rewriting the abandonment body to fail
its spans. All three now fail; the first two passed before.

Found by Bugbot on #36, reviewing the previous commit's tests.
…found

_function_body kept only the first definition of a name. langchain-agents defines
abandon_open_spans twice: once on the callback handler that holds the spans, and
once on the wrapper the streaming path actually calls. Only the handler was
checked, so a wrapper that closed nothing kept the check green, which is the exact
shape of failure this test exists to catch.

Every definition is checked now. A wrapper satisfies it by handing off to another
abandonment helper rather than by marking spans itself, because delegating is
still doing the job.

Verified against a fourth mutation, emptying the wrapper, which passed before and
fails now. The three earlier mutations still fail.

Found by Bugbot on #36, reviewing the previous commit's tests.
…ilure

The check accepted any body that either called fail_span or delegated to another
close_open_spans. On langchain-agents the wrapper only delegates, and that alone
kept the assertion green while the handler it delegates to stopped failing
anything: the failure path could disappear without the lock noticing.

At least one body must now record the failure itself, and every body must either
record it or hand off to one that does. Same shape as the abandonment check
beside it.

Verified against a fifth mutation, emptying the handler body while the wrapper
still delegates, which passed before and fails now.

Found by Bugbot on #36, reviewing the previous commit's tests.
The check only asked that some abandon call existed somewhere in the handler
module. Moving it into except Exception, or onto a success path, kept the check
green while recreating the leak: GeneratorExit and CancelledError never enter
except Exception, and a success path does not run at all when a consumer walks
away, so an abandon call in either place closes nothing on the one path it exists
for.

Scoped to the body of any finally, rather than the last one. langchain-agents
cleans up in an inner finally and keeps an outer one for the vendor generator, so
pinning this to the last block would fail a handler doing the right thing in the
right place. That is what made the check too loose in the first place.

Verified against a sixth mutation, moving the call from finally into except, which
passed before and fails now. The five earlier mutations still fail.

Found by Bugbot on #36, reviewing the previous commit's tests.
The inline tool-loop check sliced from the last finally: to the end of the file
and asked whether open_tool_span appeared anywhere in that suffix. That is not the
finally body. Every helper defined below the last finally was in scope, so a
function that merely named the tracker satisfied the assertion while the teardown
closed nothing.

Proved by mutation: deleting the real cleanup and adding a helper that names
open_tool_span passes the old check and fails the new one.

_finally_blocks already existed in this file for the hook-based checks, added
because pinning to the last finally was known to be wrong there. The inline check
was left on the old slice, so the same lesson had to be learned twice.

Found by Bugbot on this PR.
@apucacao
apucacao force-pushed the ag/py-telemetry-drift-oracle branch from 6becb8c to 40ef593 Compare August 13, 2026 01:37
@apucacao

Copy link
Copy Markdown
Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 40ef593. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant