feat(client)!: put conversation content on spans behind an opt-in flag - #29
feat(client)!: put conversation content on spans behind an opt-in flag#29apucacao wants to merge 1 commit into
Conversation
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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 905f873. Configure here.
Python writes the text of every prompt and every model answer onto its spans today, unconditionally, with no way to turn it off. That text is PII, and it leaves for whatever collector the SDK points at whether or not anyone asked for it. TypeScript treats it as opt-in. This adds the layer that lets Python do the same. Nothing changes behaviour yet. The handlers still call the old writers; they move over one package at a time, and that is where the default takes effect. BREAKING CHANGE: once the handlers move onto this layer, prompt and completion content will be absent from spans unless the caller passes capture_content=True. Anyone reading gen_ai.prompt.0.content today will need to opt in. The gate is an argument rather than ambient state, so it is visible at every call site. Handlers check it a second time before building the argument, which looks redundant and is not: the guard here makes a forgotten call site harmless, and the guard there avoids walking a conversation and serialising JSON that would then be discarded, once per model turn, inside a loop. Three carriers hold the same content, deliberately. The canonical GenAI attributes are what the semantic conventions make normative. The OpenLLMetry indexed attributes are the only ones LaunchDarkly's trace view reads today, so canonical alone renders an empty transcript. The legacy span events are redundant and deprecated, but every published version has emitted them and removing them would silently break anyone who learned to read them. All three are written from the same messages behind the same gate, so they cannot disagree. The system prompt goes in twice, its own canonical attribute and index 0 of the OpenLLMetry carrier, because that shape has no slot for it and dropping it there hides the system prompt from the only view that renders. The two legacy events are asymmetric: the output side writes nothing at all when there are no messages, the input side still adds its event. That matches the TypeScript source rather than being tidy, and there is a test saying so, because implementing the two symmetrically is the obvious thing to do and would diverge. to_semconv_finish_reason maps Anthropic's and OpenAI's own words onto one enum. Passing them through untranslated made a consumer grouping by finish reason see `stop` and `end_turn` as two different outcomes for the same event. An unmapped word passes through verbatim rather than being coerced, which is the signal to add a row; pause_turn is deliberately absent, because no value in the enum means "did not finish". The two LangChain helpers live here rather than in each LangChain package, because both need exactly the same conversion and a copy in each package is how the span code drifted apart the last time. The narrowing is structural, so the client takes no dependency on LangChain.
905f873 to
5e76628
Compare
|
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 5e76628. Configure here.
|
bugbot run |
| "tool_calls": "tool_calls", | ||
| "content_filter": "content_filter", | ||
| "function_call": "tool_calls", | ||
| } |
There was a problem hiding this comment.
Missing Anthropic context-window mapping
Medium Severity
_SEMCONV_FINISH_REASONS omits Anthropic's model_context_window_exceeded, so to_semconv_finish_reason passes it through unchanged. That is another truncation outcome like max_tokens, which already maps to length. Consumers grouping by finish reason will again split one event into two labels, the fragmentation this table is meant to stop.
Reviewed by Cursor Bugbot for commit 5e76628. Configure here.
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
1 issue from previous review remains unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 5e76628. Configure here.
|
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5e76628. Configure here.
| return json.dumps({"name": self.name or "", "arguments": self.arguments}) | ||
| if isinstance(self.result, str): | ||
| return self.result | ||
| return json.dumps(self.result) |
There was a problem hiding this comment.
Null tool result becomes text
Medium Severity
SpanMessagePart.to_text runs json.dumps on a missing tool_call_response result, so OpenLLMetry content and the legacy completion event get the literal text null. Canonical output correctly omits an absent result, so the carriers disagree and the transcript can show a fake tool return.
Reviewed by Cursor Bugbot for commit 5e76628. Configure here.
1 participant
Adds the layer that lets conversation content on spans be opt-in, matching the TypeScript SDK.
Python writes the text of every prompt and every model answer onto its spans today, unconditionally, with no way to turn it off. That text is personal data and it leaves for whatever collector the SDK points at whether or not anyone asked for it.
Additive. The handlers still call the old writers in this PR; they move over one package at a time above, and that is where the default takes effect.
What changed
capture_contentis an argument rather than ambient state, so the gate is visible at every call site.to_semconv_finish_reasonmaps Anthropic's and OpenAI's own words onto one enum. Passing them through untranslated made a consumer grouping by finish reason seestopandend_turnas two outcomes for the same event. An unmapped word passes through verbatim rather than being coerced;pause_turnis deliberately absent, because no value in the enum means "did not finish".Two details that look like bugs and are not, both with tests saying so: the system prompt goes in twice, because the OpenLLMetry shape has no slot for it and dropping it there hides it from the only view that renders; and the two legacy events are asymmetric, because the output side writes nothing for an empty message list while the input side still writes its event, which is what the TypeScript source does.
Breaking change
Declared here, takes effect in the handler PRs above. Once a handler moves onto this layer, prompt and completion content is absent from its spans unless the caller passes
capture_content=True.Where this sits
Builds on the usage layer (#28). All six handler PRs depend on this one.
Tests: 671 to 711.
Note
Overview
Adds an opt-in span content layer so conversation text (PII) is no longer written unconditionally. Handlers will pass
capture=Trueto record prompts, completions, tool catalogs, and tool-call I/O; with the flag off, nothing is written.Content is written to three carriers from the same messages: canonical GenAI attributes (
gen_ai.input.messages, etc.), OpenLLMetry indexed attributes (what LaunchDarkly's trace view reads today), and legacygen_ai.content.*span events for backward compatibility.Also centralizes finish-reason mapping onto the semconv vocabulary (
to_semconv_finish_reason) and shared LangChain helpers (lang_chain_span_messages,lang_chain_finish_reasons) so the two LangChain packages don't drift. This PR is additive; the default-off behavior takes effect as handlers migrate onto this layer.Reviewed by Cursor Bugbot for commit 5e76628. Bugbot is set up for automated code reviews on this repo. Configure here.