Repository navigation
feat(client): add result variant and value to the feature_flag span event - #240
Conversation
Keep the raw variation value that client.variation returns and hand it to handlers as variables["__ldValue"], next to __ld. It stays out of the __ld track data on purpose: that data rides on every $ld:ai:* track event, and the value includes the prompt.
…vent AI Config monitoring only finds traces whose feature_flag event carries feature_flag.result.value, which the LaunchDarkly observability plugins always send. Write the variation key as feature_flag.result.variant (conditionally required by the OpenTelemetry feature-flag conventions) and the evaluated value as feature_flag.result.value, serialized with json.dumps exactly like the ldotel tracing hook. Both are omitted when there is nothing to write, for example on native-graph spans.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
The parity suite fails on any span attribute key it does not know about, and CI caught feature_flag.result.variant. Both result keys are deliberate: they are the OpenTelemetry feature-flag convention's served variant and evaluated value.
The raw variation holds the prompt, so it must not leave the process unless the user opted in to content capture. set_ld_span_attributes takes the handler's resolved capture_content (the existing setting: explicit on the handler, else the configure() default, else off). With capture on and a raw variation present, result.value is json.dumps of that variation, as before. Otherwise it is json.dumps of the variation key when there is one, which also gives graph() roots a value. Otherwise it is omitted. Every handler package passes its own resolved setting to the root span, and the graph() roots pass the process default. Native-graph roots keep the default; they have no variation key and no raw variation, so nothing is written either way. The api report records the new keyword argument.
jeffdupont
left a comment
jeffdupont
left a comment
There was a problem hiding this comment.
Matches the updated #113 spec: variant, gated value, key fallback, and nothing when both are missing. I checked that every start_root_span / _root_span call now passes capture_content. One small thing inline, plus:
- Capture on is only tested at the helper level. The handler tests and
tests/test_cross_handler_parity.pyassert the capture-off case only. Each of the 12 handlers passescapture_content=tostart_root_spanby hand. If one of those arguments is dropped, every test still passes and capture-on users quietly get the key instead of the value. §1.5 asks handler tests to cover capture on too. The parity file already checks handler source text, so one parametrized check that every handler'sstart_root_span(/_root_span(call passescapture_content=would cover it. A behavioral capture-on case in one handler would also do. - Serialization question. gonfalon's flag Monitoring Traces tab groups chart series by the exact
feature_flag.result.valuestring, matched againstJSON.stringify(variation.value)(static/ld/components/feature-monitoring/hooks/useVariationChartConfig.ts:8,45-49).json.dumpswrites{"a": 1}with spaces, so it never matches, and neither does the capture-off key fallback. Do you know which UI readsresult.valuefor AI Configs? If it's that chart,json.dumps(value, separators=(",", ":"))would line up the capture-on case. - The PR description is out of date. Line 15 says "The value isn't gated by content capture, matching the plugins". The code now gates it.
graph() roots never carry a raw variation, so the capture setting cannot change what their feature_flag event writes. Pass the variables alone, as before, and drop the now-unused imports.
The parity suite now checks that every handler passes capture_content= to each root span call, since a missing keyword falls back to off and silently drops the value for capture-on runs. One openai-agents case runs the handler with capture on and asserts the event carries the raw variation.
…es test Content capture now defaults to on, so the test's raw variation became the result value. It asserts the capture-off key fallback, so say so.
jeffdupont
left a comment
jeffdupont
left a comment
There was a problem hiding this comment.
Since my last review: 8702b2c and the new check in tests/test_cross_handler_parity.py cover the capture-on wiring in every handler, 208c1e3 drops the capture argument from the graph root as suggested, and the description now says the value is gated and points to gonfalon#76569 for the serialization question. Thanks.
This matches launchdarkly/ai-sdks-monorepo#113 and the JS PR (#186) on the parts that show up on the span. Both attribute names are right. The variant is left off when the variation key is empty or missing. With capture on, the value is json.dumps of the raw variation. That is what ldotel/tracing.py writes. With capture off, the value is the JSON-encoded key, and it is left off when there is neither. Every handler's root span passes its resolved capture setting. I checked claude-agents, litellm-agents, langchain-messages, typesafe, opencode and vercel, and the AST parity test covers the rest. The change is additive: set_ld_span_attributes gains a keyword-only argument with a default, and no attribute or public name is removed.
One point is worth settling before 1.0. The raw variation is stored on meta as evaluatedValue, and meta is public. JS returns it next to meta and leaves VariationMeta alone. See the inline comment.
A note on parity, not on this PR: Python's capture_content defaults to True (effective_capture_content, SDKOptions in the API report). JS handlers default captureContent to false. So with no configuration, a Python run sends the full AI Config JSON in feature_flag.result.value, and a JS run sends only the key.
meta is public through GraphNode.meta and JudgeTask.judge_meta, so a public evaluatedValue key would expose the full prompt in user logging regardless of capture_content, and renaming it after 1.0 would break callers. Store it as _evaluatedValue, read only by the tracking helpers.
A private _evaluatedValue key still rode on public objects: GraphNode.meta and JudgeTask.judge_meta carried the full prompt into any log or pickle of them, whatever capture_content was set to. extract_variation now returns the raw variation as a separate value, as JS's extractVariation does. execute_and_track and execute_and_stream take it as a keyword-only evaluated_value, config() and inline judges pass it, and the graph runner keeps node values in a private map beside its nodes. A JudgeTask run falls back to the key, as in JS.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ 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 a61c664. Configure here.
….result.value A deferred judge (skip_judges=True, then run_judge) wrote the variation key even with content capture on, while an inline judge wrote the raw variation. JudgeTask now carries it as judge_value, and run_judge passes it on, so both paths write the same value. The task already holds the judge config and the response, so this adds no new kind of content.
…on the graph root The graph root span always wrote the key. It now writes the graph variation as returned by the LD SDK when content capture is on, and the key otherwise. The root has no single handler, so it follows the process default. The helper also skips a null value instead of writing "null".
# Conflicts: # packages/client/api-report/launchdarkly_ai_server.txt
jeffdupont
left a comment
jeffdupont
left a comment
There was a problem hiding this comment.
Since my last review: a61c664 takes the raw variation off meta entirely. extract_variation now returns it as a separate value, so GraphNode.meta and JudgeTask.judge_meta no longer carry the prompt. 36e2e93 carries the judge variation on JudgeTask.judge_value for deferred judges. 404fcc1 gives the graph root the graph variation under the configured default. That replaces my earlier suggestion to drop the argument there, and it is the right call now that the root has a value. Both have tests, with capture on and off for invoke and stream.
This matches the spec in ai-sdks-monorepo #113 and js-ai-sdk #186 on everything that shows up on the span. json.dumps matches what ldotel/tracing.py writes. Every handler passes its resolved capture setting, and the AST check guards that. The API changes are additive.
One request: the description still says graph() roots never carry a raw variation, and the testing section says the root tests check only the key fallback. 404fcc1 changed both, so please update it.
For the spec rather than this PR: set_ld_span_attributes defaults capture_content to False. A custom handler therefore gets only the key unless it passes the flag, while JS writes the value whenever the core passes it. I've asked on #113 for the spec to pick one.
execute_and_track and execute_and_stream merged the caller's variables and only set __ldValue when there was an evaluated value, so a caller-supplied __ldValue could reach the handler and be written as feature_flag.result.value. Both now drop it first, the same as the JS SDK does.
…e default capture_content defaulted to False, so a custom handler that called the helper without the flag sent only the variation key, while the JS SDK writes whatever the core passes, which follows configure(). The argument is now bool | None = None, and None reads the process default at call time. Built-in handlers already pass their resolved setting, so they are unchanged.
…hen it is wrapped again create_handler(q, create_handler(p, fn, None, False)).capture_content was True, because the new ProviderHandler started unset. Wrapping a ProviderHandler again now keeps its streaming implementation and its explicit capture_content override unless new ones are passed, as the JS SDK does. An unset inner handler stays unset and still follows configure().
jeffdupont
left a comment
jeffdupont
left a comment
There was a problem hiding this comment.
558cc6f (strip a caller-supplied __ldValue) and 4173b65 (a re-wrap keeps the stream and the explicit capture_content override, as of when it's wrapped) both look right. 4173b65 matches §3.14 in ai-sdks-monorepo#113 and js-ai-sdk#190. Could you add a line to the description for the create_handler change, since the title doesn't cover it?
One problem with 248ad6d. set_ld_span_attributes now falls back to the process default when capture_content is None, but execute_and_track / execute_and_stream still hand every handler __ldValue regardless of its setting (tracking.py:222, :328). A custom handler's function can't see its own ProviderHandler, so it calls the helper without a flag, and its explicit capture_content=False is skipped. I ran this: create_handler(("Custom","messages"), fn, None, False), where fn calls set_ld_span_attributes(span, variables), with no configure(). At 4e1d24e it writes "v1". At 4173b65 it writes the whole variation, system prompt included, as feature_flag.result.value. §3.9a says an explicit false beats the configured default, and JS (#186) gates this in the core on the handler's effective setting, so a JS custom handler with captureContent: false only gets the key.
Suggested fix, matching JS: only set __ldValue in execute_and_track / execute_and_stream when handler.capture_content is true, and add a test for the case above. The helper's None default can stay as a second gate. I'll ask for the matching edit on #113's new line 398, which currently describes the Python side as "the process default" rather than the handler's setting.
…tent execute_and_track and execute_and_stream handed __ldValue to every handler, and since set_ld_span_attributes now follows the process default when no flag is passed, a custom handler built with capture_content=False whose function called the helper without a flag exported the whole variation. The core now sets __ldValue only when the handler's effective setting is on (a plain function follows the process default), as the JS SDK does. The helper's default stays as a second gate.
jeffdupont
left a comment
jeffdupont
left a comment
There was a problem hiding this comment.
Thanks, f168176 closes the leak. I re-ran my probe: a custom handler with capture_content=False whose function calls set_ld_span_attributes(span, variables) now writes only "v1", and the new test fails on 4173b65 and passes here. The client suite and the built-in handler suites pass. The caller-__ldValue strip and the re-wrap behaviour are unchanged.
One case left, and it's my fault, since I suggested keeping the helper's None default as a second gate. With configure(capture_content=False), a custom handler built with an explicit capture_content=True now gets only the key. The core passes __ldValue because the handler's effective setting is True, but the helper resolves its own None to the process default (utils.py:842-846) and drops it. §3.9a says an explicit true beats a configured false. js-ai-sdk#186's setLdSpanAttributes writes __ldValue whenever the core passed it, so JS writes the full value in this case.
Suggested fix: have set_ld_span_attributes treat capture_content=None as "trust the core" and write __ldValue when it's present. An explicit False still suppresses it, and built-in handlers are unaffected because they pass their resolved flag. Plus a test for explicit True under configure(False). Then I'm happy to approve.
…ontent is None With configure(capture_content=False), a custom handler built with an explicit capture_content=True got only the key: the core passed __ldValue because the handler's effective setting was on, but the helper resolved its own None to the process default again and dropped it. The core already passes __ldValue only to a capture-on handler, so None now writes it whenever it is present, as the JS helper does. An explicit False still suppresses it.
jeffdupont
left a comment
jeffdupont
left a comment
There was a problem hiding this comment.
Approving at 0ccd265. set_ld_span_attributes now trusts the core when capture_content is None and only an explicit False suppresses the value, so a custom handler's explicit True beats configure(capture_content=False).
I re-ran the full matrix at 0ccd265 and f168176:
- explicit False gives the key
- unset with configure(False) gives the key
- explicit True with configure(False) now gives the full value; at f168176 it gave the key
- unset and plain function under the default give the full value
- a plain function with configure(False) gives the key
- a forged __ldValue is stripped
- the streaming path matches invoke
The new parameterized test fails at f168176 and passes here. The client, cross-handler parity and built-in handler suites all pass. This now matches js-ai-sdk#190 and the spec at ai-sdks-monorepo#113 8a86c34 case for case. Thanks for sticking with this one.
Summary
Runs made through the turnkey SDK don't show up on an AI Config's monitoring traces tab. Turnkey writes the
feature_flagspan event withoutfeature_flag.result.value. Since launchdarkly/observability#2553, a trace search for a flag key only scans hours where afeature_flagevent with a value was recorded, so turnkey runs are never found.This adds two attributes to the event
set_ld_span_attributeswrites:feature_flag.result.variant: the variation key. The OpenTelemetry feature-flag conventions make it conditionally required when the provider has variants.feature_flag.result.value: the evaluated value, gated by content capture (below).Content capture
The raw variation holds the prompt, so it only leaves the process when the user opted in. This uses the existing
capture_contentsetting, including theconfigure()default; no new setting.json.dumpsof the variation exactly as the SDK returned it, the same serialization as theldoteltracing hook with value inclusion on.json.dumpsof the key, for example"\"v1\"".Two spans have no single handler. The
graph()root follows the configured process default. A deferred judge (run_judge) follows the judge handler's setting.What changed
extract_variationreturns the raw variation fromclient.variationasvalue, besidemetarather than on it, sincemetais public asGraphNode.metaandJudgeTask.judge_meta. This matches JSextractVariation.execute_and_track/execute_and_streamtake it as a keyword-onlyevaluated_valueand pass it to handlers asvariables["__ldValue"], next to__ld. It is kept out of the__ldtrack data, which rides on every$ld:ai:*track event. A caller-supplied__ldValueinvariablesis dropped, the same way__ldis overwritten.__ldValueis set only when the handler's effective capture setting is on (a plain function follows the process default), so a handler built withcapture_content=Falsenever receives the prompt, as in the JS SDK.config(), inline judges, graph nodes (kept in a private map, not onGraphNode), deferred judges (JudgeTask.judge_value, passed on byrun_judge), and thegraph()root (the graph variation).set_ld_span_attributestakes a keyword-onlycapture_content. Every handler passes the same resolved setting it already uses for its other content. Left unset, it trusts the core and writes__ldValuewhenever it is present, since the core only passes it to a capture-on handler; only an explicitFalsesuppresses it. This matches the JS helper.create_handlerbehavior change, not covered by the title: calling it on aProviderHandlerkeeps that handler's stream and explicitcapture_contentoverride, as of when it is wrapped, unless new ones are passed. Before, the new handler started with no stream and an unsetcapture_content. This matches the JS SDK and §3.14 in launchdarkly/ai-sdks-monorepo#113.Testing
graph()root tests for invoke and stream: the raw graph variation with capture on, the key with capture off, and nothing with capture off and no variation key.build_judge_tasksstores the raw judge variation on the task (it survives a JSON round trip), andrun_judgehands it to the judge handler.extract_variationreturns the raw variation besidemeta, andmetadoes not contain it.__ldValuenever reaches the handler, and a custom handler built withcapture_content=Falsewhose function calls the helper without a flag writes only the key.capture_contenton the helper writes the value whenever the core passed it, and a custom handler with an explicitTrueunderconfigure(capture_content=False)gets the full value.capture_content=to its root span calls.uv run pytest(whole repo),ruff checkandmypypass.The cross-SDK spec change is in launchdarkly/ai-sdks-monorepo#113.
Note
Overview
Turnkey AI runs were missing from AI Config Monitoring traces because the root
feature_flagevent had nofeature_flag.result.value. This PR addsfeature_flag.result.variantandfeature_flag.result.valueon that event (documented in TELEMETRY-CONTRACT.md), aligned with theldoteltracing hook.set_ld_span_attributesnow acceptscapture_content: with capture on andvariables['__ldValue'], the value isjson.dumpsof the raw variation; otherwise a non-empty variation key is written as JSON-encoded key; both attributes are omitted when there is no key (e.g. native graph roots).extract_variationreturns the SDK payload asvaluebesidemeta.execute_and_track/execute_and_streamtakeevaluated_valueand inject SDK-owned__ldValueonly when the handler’s effective capture setting is on; caller-supplied__ldValueis stripped.config(), graph nodes, inline/deferred judges, andgraph()roots all pass the evaluated variation through those paths.Every provider handler passes resolved
capture_contentintostart_root_spanso root spans honor the same gate.create_handlerre-wrapping aProviderHandlernow preserves its stream fn and explicitcapture_contentoverride unless overridden.Cross-handler parity tests require
capture_content=on root span calls; new unit/integration tests cover variant/value rules, graph invoke/stream, judges, and handler re-wrap behavior.Reviewed by Cursor Bugbot for commit 0ccd265. Bugbot is set up for automated code reviews on this repo. Configure here.
Serialization
feature_flag.result.valueis written withjson.dumpsdefaults, the same as theldoteltracing hook, so the Python SDK and the Python plugin produce identical values. JavaScript'sJSON.stringifyomits the spaces; launchdarkly/gonfalon#76569 makes the flag Monitoring tabs match variations by parsed JSON value, so both forms resolve to the same variation.