test: fail the build when the six handlers drift apart again - #36
Draft
apucacao wants to merge 3 commits into
Draft
test: fail the build when the six handlers drift apart again#36apucacao wants to merge 3 commits into
apucacao wants to merge 3 commits into
Conversation
Author
|
bugbot run |
apucacao
force-pushed
the
ag/py-telemetry-drift-oracle
branch
from
August 11, 2026 20:43
c48aecb to
59890fa
Compare
Author
|
bugbot run |
apucacao
force-pushed
the
ag/py-telemetry-drift-oracle
branch
from
August 11, 2026 21:01
59890fa to
2234b78
Compare
Author
|
bugbot run |
There was a problem hiding this comment.
✅ 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.
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.
apucacao
force-pushed
the
ag/py-telemetry-drift-oracle
branch
from
August 11, 2026 21:18
2234b78 to
8eee53f
Compare
Author
|
bugbot run |
There was a problem hiding this comment.
✅ 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.
Author
|
bugbot run |
There was a problem hiding this comment.
✅ 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
chatspans 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 thefeature_flagevent appear on the root and nowhere else.Two of them exist because of specific mistakes this work nearly shipped:
gen_ai.systemon the LangChain handlers, andgen_ai.provider.namebeing a passthrough instead of the binary Anthropic-or-OpenAI choice. A passthrough reads as obviously correct and reportsbedrockfor 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_flagevent's attributes are built as a plain dict and never appear inside aset_attributecall.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:
invoke_agentWhere this sits
Needs all six handler PRs (#30 to #35), because it asserts across all of them.
Tests: 844 to 913.
Note
Overview
Adds cross-handler parity tests at the repo root so the six handlers cannot silently disagree on span shape or attribute vocabulary again.
Shape checks call each package's span constructors with a recording tracer and pin shared names (
invoke_agent,chat {model},execute_tool {name}), shared surface functions, LaunchDarkly identity only on the root, and LangChain-specific provider-key rules.A vocabulary lock scans package sources for emitted attribute/event keys and compares them to a committed set aligned with the TypeScript SDK, failing on both unexpected new keys and silently dropped ones. Runtime checks also cover the indexed OpenLLMetry
gen_ai.prompt/gen_ai.completioncarrier.CI now runs bare
uv run pytestinstead ofpackages/*/tests, so the new roottests/directory is included.Reviewed by Cursor Bugbot for commit 8eee53f. Bugbot is set up for automated code reviews on this repo. Configure here.