Repository navigation
Conversation
Rewrite the migration around config().invoke() with handler packages or custom handlers, keeping the original call as the fallback. Remove tracker wiring (the SDK records metrics), switch placeholders to {{name}} without spaces, pass tools as tool_handlers, and update the agent-mode, agent-graph, fallback, audit, and before/after references for launchdarkly-ai-server 0.3.0 and @launchdarkly/ai-node 0.3.0.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Skill eval results
Only suites whose source actually changed since their last recorded score were re-run. Soft-failing while we stabilise the baseline. |
Raise a dedicated exception at the ReAct step limit so the RuntimeError fallback doesn't swallow it, accept sync or async tool functions in the StateGraph example, and drop tools from parameters in the LangChain factory example.
jeffdupont
left a comment
jeffdupont
left a comment
There was a problem hiding this comment.
Its core fallback, except RuntimeError, also catches failures from the SDK's own handlers after they have already run tools. Its install step leaves out launchdarkly-server-sdk, so every request silently takes the fallback.
skills/agentcontrol/migrate/references/phase-1-analysis-checklist.md:143 — launchdarkly-ai-server doesn't bundle the server SDK. On PyPI its only required dependency is opentelemetry-api, and none of the handler packages pull it in either. Without launchdarkly-server-sdk, init_client() raises RuntimeError("LaunchDarkly server SDK not installed...") (python-ai-sdk lifecycle.py:271-280). In this skill that error lands in the except RuntimeError fallback, so a migrated app silently runs the hardcoded path on every request and the Monitoring tab stays empty. Suggest: launchdarkly-ai-server + launchdarkly-server-sdk (required peer). Same fix for the install list at SKILL.md:160. The ai-node row is correct as written.
skills/agentcontrol/migrate/SKILL.md:406 — "Catch RuntimeError specifically; provider errors are different exception types" doesn't hold for the handler packages this skill recommends. They raise RuntimeError themselves after the run has started:
Tool loop exceeded the maximum number of steps (10)in openai-messages, claude-messages and langchain-messagesClaude agent run ended with ...in claude-agents
The SDK re-raises these unchanged. So an agent that hits the step cap after calling a side-effecting tool falls into this fallback, which re-runs the hardcoded agent and calls the tool again. The AgentStepLimitError fix in 71ce24e covers only the custom-loop example.
Smallest fix, on the error path only: inspect_config never raises, so re-raise when the config was actually served:
except RuntimeError:
if (await inspect_config("chat-assistant", context))["enabled"]:
raise
return answer_hardcoded(user_input)The same pattern applies at SKILL.md:24, SKILL.md:234, references/fallback-defaults-pattern.md:9,42, references/before-after-examples.md:75,221 and references/agent-graph-reference.md (the graph except (RuntimeError, ValueError)). For Node.js (fallback-defaults-pattern.md:60, before-after-examples.md:155), "anything that isn't a provider error" also catches Error('Tool loop exceeded...') and tool-handler errors. The same inspectConfig check works there.
skills/agentcontrol/migrate/SKILL.md:267 — On launchdarkly-ai-server 0.4.0, a list works too: handler_key() normalizes list and tuple tags (python-ai-sdk #152). Only 0.3.0 needed a tuple. Every handler package on PyPI requires launchdarkly-ai-server>=0.3.1, so anyone who installs a handler today gets 0.4.0. Suggest dropping this sentence, keeping the tuple in the example, and bumping the compatibility line to 0.4.0+.
skills/agentcontrol/migrate/SKILL.md:421 — This contradicts the caveat at :241. On 0.2.5, openai-messages forwards no parameters and claude-messages forwards only max_tokens, so neither can crash on max_search_results. Only the LangChain handler or a custom handler passes raw params through. python-ai-sdk #107 (merged, unreleased) and js-ai-sdk #118 change every handler to forward a fixed allowlist and drop other keys. The advice to use model.custom stays right, but the reason should be "handlers won't pass it to your code, and custom/LangChain handlers would send it to the provider." Suggest tying the :241 caveat to versions: "openai-messages ≤ 0.2.5 / claude-messages ≤ 0.2.5; fixed by python-ai-sdk #107 in the next release." The same rewording applies at agent-mode-frameworks.md:86, phase-1-analysis-checklist.md:108 and before-after-examples.md:88.
| messages = [ | ||
| {**m, "content": parse_template(m["content"], variables)} for m in ld_config.get("messages", []) | ||
| ] | ||
| response = openai_client.chat.completions.create( |
There was a problem hiding this comment.
Should fix. call_openai is async, but it calls the sync openai_client.chat.completions.create(...). Every request holds the event loop for the whole model call, so concurrent requests in an async server run one at a time. before-after-examples.md:59 already uses AsyncOpenAI with await. Use the same here: response = await async_openai.chat.completions.create(...).
There was a problem hiding this comment.
Fixed in 31e10a3. The example creates an AsyncOpenAI client and awaits the call.
| 3. **Build a `Context` per request.** The context carries targeting inputs — user ID, plan tier, region, whatever the rollout is keyed on. Reuse the same context the app already uses for feature flag evaluation if one exists. | ||
| 4. **Always check `config.enabled`.** Even a successful `completion_config` call can return a disabled config (if the variation is turned off in LaunchDarkly). The disabled path should not call the provider. | ||
| 5. **Do not cache the config object across requests.** Call `completion_config` / `agent_config` inside the request handler so LaunchDarkly can re-evaluate targeting per call. One `LDAIClient` instance, many `completion_config` calls. | ||
| 1. **The request handler's signature and return shape stay the same.** Only where the model, prompt, and parameters come from changes, plus the `try` / `except RuntimeError` around `invoke()`. |
There was a problem hiding this comment.
Should fix. Rule 1 says the request handler's signature stays the same, and SKILL.md:92 says the same. But every example turns a sync answer(user_question) into async def answer(user_id, user_question) (:18 → :71, and SKILL.md:210 → :230). Every existing caller breaks: a sync caller now gets a coroutine back, and every caller has to supply user_id. An agent following the rule won't update the callers. Suggest saying it plainly: "The call site becomes async and needs a context source (user id); update every caller, or keep a sync wrapper with asyncio.run for scripts."
There was a problem hiding this comment.
Fixed in 31e10a3. Core principle 2 and rule 1 now say the call site becomes async and needs a context source, and to update every caller or keep an asyncio.run() wrapper for scripts.
| tracker.track_judge_result(result) | ||
| result = await config(key="chat-assistant", handler=handlers, skip_judges=True).invoke(user_input, context) | ||
| for task in result.judge_tasks or []: | ||
| asyncio.create_task(run_judge_worker(task)) |
There was a problem hiding this comment.
Should fix. The event loop keeps only a weak reference to tasks, so a fire-and-forget create_task(run_judge_worker(task)) can be garbage-collected before it finishes, and that judge score is lost. The asyncio docs carry this warning for create_task. Smallest fix: keep the tasks in a set, as in the docs: t = asyncio.create_task(...); _bg.add(t); t.add_done_callback(_bg.discard). Or point at the online-evals worker instead of inlining this.
There was a problem hiding this comment.
Fixed in 31e10a3. The tasks go into a set with add_done_callback(discard), as in the asyncio docs.
| 4. **Wire feedback tracking if the app has thumbs-up/down UI.** Both SDKs expose `trackFeedback` with a `{kind}` argument. | ||
| 1. **Check the Monitoring tab.** Hit the wrapped endpoint in staging, then open the config in LaunchDarkly → **Monitoring**. Duration, token, and generation counts should appear within 1–2 minutes. Force an error (bad provider key) and confirm the error count increments; the SDK records the error and re-raises. | ||
| 2. **If tokens are missing**, the handler isn't returning usage. Custom handlers must return `{"output": ..., "usage": {"input": n, "output": n}}`. For streaming, enable usage on the final chunk (`stream_options={"include_usage": True}` for OpenAI). | ||
| 3. **Wire feedback if the app has thumbs-up/down UI.** The SDK doesn't record feedback. Store the result's `track_data` with the message and send the event when the user responds: |
There was a problem hiding this comment.
Nice to have. Neither SDK's stream() done event carries track_data today: Python yields response, usage and judge_results (client.py:272-277), and JS yields response, usage and judgeResults. A streaming app with thumbs-up/down has nothing to attribute feedback with. Worth one sentence saying so, since the coverage table sends streaming apps to stream().
There was a problem hiding this comment.
Added in 31e10a3. The feedback step now says stream() doesn't return track_data yet.
…judges Use AsyncOpenAI in the custom handler, say plainly that call sites become async and need a context source, keep references to background judge tasks, note that stream() has no track_data, and set custom knobs with the MCP variation tool instead of curl.
jeffdupont
left a comment
jeffdupont
left a comment
There was a problem hiding this comment.
Follow-up on my 2026-10-26 review. Thanks for 31e10a3: the AsyncOpenAI handler, the async call-site wording and the stream() / track_data note are all fixed.
Still open: the four points in the review body itself had no reply, and none of them changed. Two of them block:
- Install step leaves out the server SDK (
SKILL.md:160,phase-1-analysis-checklist.md:143). Without it every request silently takes the fallback. except RuntimeErroralso catches the SDK's own handler errors (SKILL.md:414,fallback-defaults-pattern.md:9). A step-limit error after a side-effecting tool re-runs the hardcoded agent and calls the tool again.
Two SDK releases landed after my review (0.5.0 on 10-26 for JS and 10-27 for Python, then 1.0.0 on 10-30 for both), so a couple of fixes are now simpler. Details are inline. Two new small issues from 31e10a3 are also inline: the background-task set is function-local, and the MCP tool name doesn't match the rest of the repo.
|
|
||
| Commit the deletion separately from the SDK install if the repo's review process benefits from it — otherwise bundle with sub-step 2. | ||
| 2. **Install the AI SDK.** Detect the package manager from Step 1, then install the core package plus one handler package per provider and mode: | ||
| - Python: `launchdarkly-ai-server` + `launchdarkly-ai-openai-messages`, `-openai-agents`, `-claude-messages`, `-claude-agents`, `-langchain-messages`, or `-langchain-agents` |
There was a problem hiding this comment.
Still open (from my review body). launchdarkly-ai-server still lists only opentelemetry-api as a required dependency. launchdarkly-server-sdk is the optional [ld] extra (python-ai-sdk packages/client/pyproject.toml:31). Without it init_client() raises RuntimeError, this skill's fallback catches it, and every request quietly runs the hardcoded path. The SDK README now documents the fix as one install line: pip install "launchdarkly-ai-server[ld]" (packages/client/README.md:14). Suggest launchdarkly-ai-server[ld] here.
There was a problem hiding this comment.
Fixed in fdf9516: Stage 2 now installs "launchdarkly-ai-server[ld]", noting that the [ld] extra installs launchdarkly-server-sdk.
| | .NET | `LaunchDarkly.ServerSdk` | `LaunchDarkly.ServerSdk.Ai` | https://docs.launchdarkly.com/sdk/ai/dotnet | | ||
| | Language | Core package | Docs | | ||
| |----------|--------------|------| | ||
| | Python | `launchdarkly-ai-server` (bundles `launchdarkly-server-sdk`) | https://launchdarkly.com/docs/sdk/ai/python | |
There was a problem hiding this comment.
Still open. It doesn't bundle it. It's the [ld] extra (see SKILL.md:160). Suggest: `launchdarkly-ai-server[ld]` (the `[ld]` extra installs `launchdarkly-server-sdk`).
There was a problem hiding this comment.
Fixed in fdf9516: updated the Python row in the SDK routing table to "launchdarkly-ai-server[ld]" (the [ld] extra installs launchdarkly-server-sdk).
| ### Fallback and config lifetime (most common failure mode) | ||
|
|
||
| ### Tracker and config lifetime (most common failure mode) | ||
| - **Don't call `invoke()` without a fallback.** It raises `RuntimeError` for a disabled variation, a missing config key, a missing SDK key, or an unreachable LaunchDarkly. Catch `RuntimeError` specifically; provider errors are different exception types and should keep propagating. |
There was a problem hiding this comment.
Still open (from my review body). "Provider errors are different exception types" still doesn't hold on 1.0.0. The handler packages this skill recommends raise RuntimeError themselves after tools have run: Tool loop exceeded the maximum number of steps (openai-messages handler.py:502, claude-messages :492, langchain-messages :1047) and Claude agent run ended with ... (claude-agents handler.py:603). The new agent-mode-frameworks.md:81 rule covers only custom handlers. A step-limit hit after a side-effecting tool lands in this fallback, and the hardcoded agent calls the tool again.
Smallest fix, error path only (inspect_config never raises):
except RuntimeError:
if (await inspect_config("chat-assistant", context))["enabled"]:
raise
return answer_hardcoded(user_input)Same change at SKILL.md:24, :234, fallback-defaults-pattern.md:9,42, before-after-examples.md:75,221, the graph except (RuntimeError, ValueError) in agent-graph-reference.md:100, and the Node.js examples with inspectConfig.
There was a problem hiding this comment.
Fixed in fdf9516: updated all fallback examples across SKILL.md, fallback-defaults-pattern.md, before-after-examples.md, and agent-graph-reference.md to check inspect_config / inspectConfig (or resolve_graph(...).enabled for graphs) and re-raise when enabled is true.
| - no SDK key is set, or LaunchDarkly is unreachable | ||
|
|
||
| ## Pattern 1: Inline fallback (start here) | ||
| The application owns the fallback. Provider errors (`openai.APIError`, `anthropic.APIStatusError`, and so on) are different exception types, so catching `RuntimeError` separates "LaunchDarkly can't serve this config" from "the model call failed." |
There was a problem hiding this comment.
Still open. Same claim as SKILL.md:414. The SDK's own handlers raise RuntimeError (step limit, Claude agent run failure), so this doesn't separate "LaunchDarkly can't serve" from "the run failed". See the inspect_config re-raise there.
There was a problem hiding this comment.
Fixed in fdf9516: updated fallback-defaults-pattern.md to check inspect_config / inspectConfig and re-raise when enabled is true.
| ``` | ||
|
|
||
| See [before-after-examples.md](references/before-after-examples.md) for full Python OpenAI, Node Anthropic, and LangGraph agent-mode paired snippets. | ||
| Custom handlers get the raw prompt, so render placeholders with `parse_template` / `parseTemplate`. The provider name in `create_handler` must match the config's provider exactly (`"OpenAI"`, `"Anthropic"`, `"Bedrock"`, `"Gemini"`), and in Python the first argument must be a tuple. |
There was a problem hiding this comment.
Still open (from my review body). handler_key() accepts a list the same as a tuple (python-ai-sdk utils.py:219-225), since 0.4.0. Suggest dropping "the first argument must be a tuple", keeping the tuple in the example, and raising the compatibility line at :5 from 0.3.0+ to the version this was tested on. 1.0.0 is current for both SDKs.
There was a problem hiding this comment.
Fixed in fdf9516: removed the tuple-only statement and updated compatibility to 1.0.0+.
| **Python — after (agent mode)** — for LangGraph, CrewAI, or any framework that takes a goal/instructions string: | ||
| The context is a plain dict (`{"kind": "user", "key": ..., "name": ...}`), not an `ldclient` `Context`. Reuse the attributes the app already uses for feature-flag targeting. | ||
|
|
||
| > **⚠️ Python handlers don't forward every model parameter.** In `launchdarkly-ai-openai-messages` 0.2.x, the handler sends no variation parameters to OpenAI, so `temperature`, `max_tokens`, and the rest are ignored. `launchdarkly-ai-claude-messages` forwards only `max_tokens`. The Node.js handlers and the Python LangChain handlers forward them all. If the hardcoded call depends on its parameters, use a custom handler (below) or the LangChain handler in Python until this changes, and confirm the behavior against the CHANGELOG. |
There was a problem hiding this comment.
Still open, and now out of date. 0.5.0 (both SDKs, launchdarkly/python-ai-sdk#107, launchdarkly/js-ai-sdk#118) changed every handler to forward a fixed list of keys per handler and drop the rest. So openai-messages now forwards temperature and the like, and "Node.js handlers forward them all" is no longer true either. Suggest: "Before 0.5.0, openai-messages forwarded no parameters and claude-messages only max_tokens. From 0.5.0 every handler forwards its own fixed list and drops other keys." Then before-after-examples.md:88 can drop its 0.2.x note.
There was a problem hiding this comment.
Fixed in fdf9516: updated the parameter-forwarding note to reflect 0.5.0+/1.0.0 behavior and removed the outdated 0.2.x note in before-after-examples.md.
| - Don't attempt a multi-agent graph migration in one pass. Migrate a single agent first; use [agent-graph-reference.md](references/agent-graph-reference.md) as the next-step read. | ||
| - **If the repo contains a `load_chat_model(f"{provider}/{name}")` helper, delete it — don't just avoid using it.** It drops every variation parameter. The LangChain handler packages build the model with the served parameters. | ||
| - **Same rule for hand-rolled `resolve_tools` / `TOOL_REGISTRY` / `ALL_TOOLS` helpers.** Replace them with `tool_handlers`. | ||
| - **Don't put app-scoped knobs in `model.parameters`.** The handlers forward every parameter to the provider SDK, so `max_search_results` crashes the provider call. Use `model.custom`. |
There was a problem hiding this comment.
Still open. On 0.5.0+ the handlers drop unknown keys, so max_search_results no longer crashes the call. The variation just quietly never reaches your code. model.custom is still the right advice. Suggest the reason: "handlers won't pass it to your code, and a custom or older handler would send it to the provider." Same wording at agent-mode-frameworks.md:86 and phase-1-analysis-checklist.md:108.
There was a problem hiding this comment.
Fixed in fdf9516: updated the reason for model.custom here, in agent-mode-frameworks.md, and in phase-1-analysis-checklist.md.
| tracker.track_judge_result(result) | ||
| result = await config(key="chat-assistant", handler=handlers, skip_judges=True).invoke(user_input, context) | ||
| # The event loop holds only weak references to tasks, so keep them until they finish | ||
| _background = set() |
There was a problem hiding this comment.
Partly fixed (new in 31e10a3). _background = set() sits inside the request code, so each request makes a new set and drops it when the function returns. After that nothing holds the tasks, which is the same as before the fix. In the asyncio docs example the set is module-level. Fix: move _background = set() to module scope, above the handler, and keep the three loop lines as they are.
There was a problem hiding this comment.
Fixed in fdf9516: moved _background = set() to module scope above the request handler.
| Code outside a handler can read knobs with `(await inspect_config(key, context))["config"]["model"]["custom"]`, at the cost of a second evaluation per turn. Keep the current values as code defaults so the fallback behaves the same. | ||
|
|
||
| ### CrewAI `Agent` | ||
| Set the knobs with the `update-agentcontrol-config-variation` MCP tool. `custom` replaces the whole object, so send every knob, not only the one you're changing: |
There was a problem hiding this comment.
New in 31e10a3. Every other skill in this repo names this tool update-ai-config-variation (13 references, including configs-update/SKILL.md:23,60), and none of them lists custom as a field it sets. If the hosted MCP tool really is update-agentcontrol-config-variation and takes custom, those other references are stale. If not, an agent following this calls a tool that doesn't exist, and the curl fallback that used to cover that case is gone. Can you confirm the name and the custom field against the live MCP tool list? Same for "custom replaces the whole object".
There was a problem hiding this comment.
Fixed in fdf9516: replaced update-agentcontrol-config-variation with the MCP caveat noting that update-agent-config (resource: "variation") does not expose top-level custom, and restored the REST PATCH snippet.
…cy SDK upgrades to legacy-sdk-migrate Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 5 potential issues.
❌ 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 0dcc6db. Configure here.
… inspect_config fallback, factory renames, and MCP caveat) Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
config().invoke()with handler packages or a custom handler, keeping the original provider call as the fallback wheninvoke()raisesRuntimeError.{{name}}with no spaces;ldctx→ldContext. Tools move totool_handlers.graph(),resolve_graph,to_lang_graph).Jira: DEVREL-1644
Testing
Ran before/after Example 1 against a live config: the served path and the no-SDK-key fallback path both returned. Checked every API name, signature, and package against the installed SDKs; all links return 200.
Notes
launchdarkly-ai-server0.3.0 (Python) and@launchdarkly/ai-node0.3.0 (Node.js).skills.json, so later merges may needpython3 scripts/generate_catalog.pyre-run.Note
Overview
Rolls the AgentControl
migrateskill forward (v0.2.0) from the legacyldai/@launchdarkly/server-sdk-aiflow to the currentlaunchdarkly-ai-server/@launchdarkly/ai-nodemodel where provider calls run throughconfig().invoke()and handler packages (or custom handlers), with metrics recorded automatically instead of manual tracker wiring.The five-stage workflow is reframed: Stage 4 is verify metrics (Monitoring tab, feedback via
track_data) rather than instrumentcreate_tracker()/track_metrics_of; tools move totool_handlers; fallbacks keep the original hardcoded call behindRuntimeError+inspect_config(); placeholders become{{name}}(no spaces). Catalog metadata, README, and reference docs (SKILL.md, agent frameworks, before/after examples, agent graphs, audit checklist) are rewritten accordingly, including routing existing legacy SDK apps tolegacy-sdk-migrate.Reviewed by Cursor Bugbot for commit fdf9516. Bugbot is set up for automated code reviews on this repo. Configure here.