Repository navigation
feat(AIC-3439)!: let wildcard handlers accept only named providers - #136
Conversation
A LangChain adapter used for one provider no longer has to match every provider, and two scoped wildcards of the same mode can be registered together. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Human message: I think this is a "nice to have" not necessarily "must do it". I'm thinking of situations where, for example, a user might want to use the Google ADK and scope it only to gemini ( You can technically achieve this now by properly including the right subset of handlers, but this makes it more obvious/colocates it with the code if you want to scope them down. tldr; if we think this is confusing or don't want it, we can close it |
jeffdupont
left a comment
There was a problem hiding this comment.
The routing code is close, and it matches js #111 on almost every probe case. This PR also fixes a Python bug. On main, run_judges used the parent handler for any judge whose provider no handler covered, because the no-match branch fell through to judge_handler = handler (main judges.py:110-147). An OpenAI parent would run an Anthropic judge. build_judge_tasks already skipped that case. Two things need doing before this merges: a rebase that fixes two tests, and agreement with #118 on how the evaluations runner picks handlers.
At ee8f6f0c I get 1427 passed and 11 skipped. mypy (51 files), ruff and ruff format --check are clean.
- Two new tests fail once this is merged with today's
main. Git merges it without conflicts. Buttest_scoped_wildcard_does_not_run_a_judge_outside_its_listandtest_two_scoped_judge_handlers_divide_providers(test_evaluations_run.py:2100 and :2133) callinit_evaluations(api_token=...)and passproject_keytorun(). Since this branch was cut,mainhas renamed the argument toapi_key(7a238e5) and movedproject_keyontoinit_evaluations(I found #127 / be1e64a withgit log -S, but didn't read the commit). After I fixed both calls, the merged tree gives 1505 passed and 11 skipped, with mypy (52 files) and ruff clean. The green CI ran on the old base. - #118 conflicts with this, in the code and in what it does. Merging this into #118 (
4cd1e0ee) conflicts inevaluations/runner.pyandpackages/client/README.md. #118 replaces_find_judge_handlerand_covers_providerwith a single_select_handler, and its rules contradict this PR in two ways. First, "a single handler always runs, whatever it declares", so a lone Bedrock-scoped handler would run an Anthropic judge. Second, two wildcard candidates raise "can't disambiguate" instead of the shortest list winning. Whichever PR lands second has to rebuild the scoped order inside_select_handler. I'd land this one first and have #118 callselect_mode_handler. #39 §3.13 already says that a judge outside every scoped list raisesEvaluationsErrornaming the provider. - Bugbot's untyped-parent finding is real, but I'd keep the new behaviour. I reproduced it. With
run_judges(handler=untyped, handlers=[untyped]), the judge ran onmainand is skipped now.graph()picks a lone untyped handler through the non-strict fallback, so that node's judges are now dropped without a log line. Butmain'sbuild_judge_tasksalready skipped the same case (0 tasks in my probe), and JS skips it too, becauseparentCoversis false withoutprovidesFor(judges.ts:25-28). This samecontinueis also what stops an OpenAI parent from running an Anthropic judge. So the change makes Python consistent with itself and with JS. I'd ask for two things: log when a judge is skipped because no handler fits (neither SDK says anything today), and add a changelog line. The evaluations runner still treats an untyped generation handler as covering every judge (runner.py:179-180). That's a separate path and fine as it is, but #39 should say so. - Python replaces a re-registered handler in place, and JS moves it to the end. registry.py:74 keeps the slot, while js registry.ts:29 filters and appends. Ties go to the earlier registration, so the SDKs now choose differently. As a repro, register
['Bedrock', 'Gemini'], then['Bedrock', 'Anthropic'], then['Bedrock', 'Gemini']again. Python then runs the Gemini-list handler for Bedrock, and JS runs the Anthropic-list one.composediffers the same way. We need one rule in #39 §3.6. I'd keep Python's. select_handlernow drops matches that aren'tProviderHandlerinstances (utils.py:601). Onmain, the exact and wildcard steps returned any object withprovides_for. Nowisinstance(chosen, ProviderHandler)throws the match away, andconfig()raisesHandler for provider OpenAI with mode messages not foundeven though a matching handler is registered. I probed this with a duck-typed('*', 'messages')object. If onlyProviderHandleris supported, the error message is misleading. Otherwise, drop the check and use acastfor mypy.- Repeated names count toward list length (utils.py:97).
frozensetmakes['Bedrock', 'Bedrock']the same identity as['Bedrock'], butbest_wildcardranks it bylen(tuple(allow)), so it ties with['Bedrock', 'Anthropic']. Deduping innormalize_providersfixes this. JS has the same issue. - The docstring and the code disagree on whitespace. utils.py:26-28 says surrounding whitespace is rejected rather than trimmed, but
[' Bedrock']is accepted (and never matches). Either reject it or fix the docstring. JS needs the same change.
I tried 23 mutations, and 17 of them fail a test. The 6 that get through are in the inline comments: unscoped beating scoped, the strict coverage check, _provider_matches, the runner's _covers_provider and its shortest-list sort, and accepting a bare string.
Smaller items
ProviderHandler.__init__takesproviderspositionally and doesn't validate or copy it (types.py:169, :175).ProviderHandler(fn, pf, providers=[])builds a handler that matches nothing. Please run it throughnormalize_providers, or make it keyword-only. This signature freezes at 1.0 too.- Exact handlers that have a list keep a separate identity (registry.py:20). The list is ignored for routing on an exact handler, so two
('OpenAI', 'messages')handlers should collide, as #39 says. JS does the same. - The duplicate warning prints a
frozensetrepr:Handler for (('*', 'messages'), frozenset({'Gemini', 'Bedrock'})) already registered. The name order changes from run to run. Sorting the names for the message would fix that. - Public surface.
providersis a keyword-only argument oncreate_handler,create_langchain_messages_handlerandcreate_langchain_agents_handler, and an attribute onProviderHandler, with the same name and meaning as JS. No new names reach__all__(normalize_providers,wildcard_covers,best_wildcard,select_mode_handlerandcovers_provider_namearen't exported), so py #144'sROOT_SURFACEsnapshot isn't affected. - #107 overlap. Merging this into #107 (
c76acd58) conflicts in utils.py, wheremodel_parametersand the new helpers are both additions, and in thecollections.abcimport line of both LangChain handlers (MappingvsSequence). Keep both sides. I don't see a clash in behaviour: #107 is about parameter forwarding, not routing. graph_judgepasseshandlershere (graph.py:1016-1024 and :1238-1250), which is right. JS doesn't, so I've asked for that on js #111.
Parity with js-ai-sdk #111
The two SDKs agree on selection order, ties, case, a None / "" provider, empty and blank lists, both strict error strings (Handler for provider OpenAI not found and ... with mode agent not found), and set identity. They differ in four places:
- re-registration position (4)
- the graph-level judge (JS is wrong)
- copying the caller's list (Python copies, JS doesn't)
['a,b']vs['a', 'b'], which collide in JS and stay separate here
…ped-handlers Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # packages/client/src/launchdarkly_ai_server/types.py # packages/client/src/launchdarkly_ai_server/utils.py # packages/langchain-agents/src/launchdarkly_ai_langchain_agents/graph.py # packages/langchain-agents/src/launchdarkly_ai_langchain_agents/handler.py # packages/langchain-messages/src/launchdarkly_ai_langchain_messages/handler.py
…ct handler identity - Reject provider names with surrounding whitespace; dedupe and copy the list. - A judge with no covering handler is skipped with a warning. - An empty handlers list no longer bypasses the scoped-parent check. - Exact handlers collide on (provider, mode) whatever their list. Co-authored-by: Cursor <cursoragent@cursor.com>
…ped-handlers # Conflicts: # packages/client/src/launchdarkly_ai_server/judges.py
Resolves the conflicts with #152, which routes evaluations through utils.select_handler and removed judge_handlers. - The scoped order now lives in utils.select_mode_handler, which select_handler calls, so config(), graph(), online judges and the evaluations runner share it: exact provider, then the shortest scoped list containing the provider, then unscoped '*'. It is built on main's handler_key, so completion and messages still match. - The runner's own _find_judge_handler / _covers_provider are gone; the scoped runner tests are ported to handlers=[...]. - Registry and the evaluations duplicate check share utils.registration_key, so two scoped wildcards of one mode with different lists are both accepted, and the same list in another order is rejected. - run_judge logs the same warning as run_judges when no handler covers a judge's provider. - normalize_providers moves to types.py so ProviderHandler.__init__ needs no inline import; utils imports it from there. - API reports regenerated for client, langchain-agents and langchain-messages. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Pushed 0d0e893, which merges origin/main and resolves the conflicts with #152:
This needs a human reviewer. |
Both items are addressed in 0d0e893 (merge with main; scoped order moved into select_mode_handler). Dismissing my own stale request; this needs a second approver since I pushed code.
jeffdupont
left a comment
There was a problem hiding this comment.
I merged main and moved the scoped order into select_mode_handler. Since I pushed those changes, I'm dismissing my 10-08 change request. @andrewklatzke, please check the merge; this needs a second approver.
jeffdupont
left a comment
There was a problem hiding this comment.
Approving 0d0e893. The change is additive (keyword-only providers on create_handler and both LangChain factories), the API reports are updated, and it matches launchdarkly/ai-sdks-monorepo#39. Note: the branch now conflicts with main, so it needs a main merge before it can land.
|
My approval was at 0d0e893. Main has moved since, and the branch now conflicts in five files: |
…ped-handlers Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # packages/client/api-report/launchdarkly_ai_server.txt # packages/client/src/launchdarkly_ai_server/evaluations/module.py # packages/client/src/launchdarkly_ai_server/types.py # packages/langchain-agents/src/launchdarkly_ai_langchain_agents/handler.py # packages/langchain-messages/src/launchdarkly_ai_langchain_messages/handler.py
create_handler already carried an inner ProviderHandler's stream_fn and capture_content override forward when the caller omitted them; providers was not, so re-wrapping a scoped wildcard handler silently became unscoped. Bugbot caught this on the latest commit. Co-authored-by: Cursor <cursoragent@cursor.com>
|
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 fa001bd. Configure here.
…ped-handlers Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # packages/client/src/launchdarkly_ai_server/utils.py # packages/langchain-messages/src/launchdarkly_ai_langchain_messages/handler.py
Steps 1 and 2 of [AIC-3672](https://launchdarkly.atlassian.net/browse/AIC-3672). The JS half is AIC-3673. ## What this does - **`release-as: 1.0.0`** on all 14 packages in `release-please-config.json`. Without it, release-please keeps bumping 0.x minors, so the next release PR would ship `launchdarkly-ai-server` 0.6.0 instead of 1.0.0. - **Internal pins.** Every package that depends on `launchdarkly-ai-server` now requires `>=1.0.0,<2`. release-please doesn't update these for Python. #218 (merged) set them to `>=0.6,<2` so either release order resolves; this tightens them to the 1.0 line. - **`uv.lock`** needs no change: uv records workspace dependencies without a version specifier. `uv lock --check` passes. `changelog-sections` is left alone. #235 removes the evaluations Experimental rows when it promotes Evaluations from Code to core. ## Merge this last Merge this after the PRs below. Anything merged before the release PR (#231) merges ships in 1.0.0, and a `!` change merged after 1.0 forces 2.0. - [x] #218 fix(AIC-3598)!: packaging and public types (merged 29 Oct) - [x] #235 feat(evaluations)!: promote Evaluations from Code to core (merged 29 Oct). After it merges, nothing in `launchdarkly_ai_server.experimental` is public. - [x] #237 fix(claude-agents)!: honour max_tokens and disable thinking by default (merged 29 Oct) - [ ] #136 feat(AIC-3439): wildcard handlers accept only named providers (approved; conflicts with main, needs a merge of main) - [x] #234 fix(AIC-3568)!: give the barrel core's `__all__` (merged 29 Oct) - [ ] #246 fix(AIC-3592): make native LangGraph hand-offs complete (spec launchdarkly/ai-sdks-monorepo#118) - [ ] #248 fix(AIC-3573): never send a NativeTool to the model as a function tool (spec launchdarkly/ai-sdks-monorepo#116) - [ ] #240 feat(client): add result variant and value to the feature_flag span event (approved, mergeable) - [ ] #251 fix(AIC-3716): build OpenAI native graph node tools with FunctionTool (review required; touches the same loop as #248) #241 (Shadow Requests) is closed; 1.0 ships with nothing experimental. ## After merging 1. Confirm the regenerated #231 lists 1.0.0 for all 14 packages, then merge it. 2. Install each package from PyPI into an empty venv and import its public entry points (AIC-3672 step 5). 3. Open a PR removing `release-as` straight away, or every later release PR proposes 1.0.0 again. ## Checks - `release-please-config.json` parses as JSON, with `release-as: 1.0.0` on all 14 packages. The first push wrote the lines with literal backslashes; fixed in d720896. - `make test`: 3,711 passed, 18 skipped (before #175 and #236 merged; CI is the current check) (with the parent checkout's `.env` LD URIs blanked; see AIC-3593) - `uv lock --check` passes 🤖 Generated with [Claude Code](https://claude.com/claude-code) [AIC-3672]: https://launchdarkly.atlassian.net/browse/AIC-3672?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Overview** > Prepares the **1.0.0 GA** cut for all 14 Python packages by teaching release-please to release **1.0.0** instead of another **0.x** bump. > > Adds **`release-as: "1.0.0"`** on every entry in `release-please-config.json` (core `launchdarkly-ai-server`, barrel, and all handler packages). > > Tightens **internal PyPI pins** from `launchdarkly-ai-server>=0.6,<2` to **`>=1.0.0,<2`** in each handler `pyproject.toml` and in `packages/ai` (including **`otel`** / **`ld`** extras). Published wheels will require the 1.0 server line once those versions ship. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit ae9f2cc. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: andrewklatzke <aklatzke@launchdarkly.com>
Summary
providerslist, so a LangChain adapter used only for Bedrock acceptsconfig.provider.name == "Bedrock"and no other provider.[provider, mode]still wins. Among scoped wildcards that contain the name, the shortest list wins, then the earlier registration. An unscoped*is the fallback.provides_forplus the provider set, so two scoped wildcards of the same mode can be registered together. Judges, graph selection, and the evaluations runner use the same order.Test plan
uv run pyteston the client, langchain-messages, and langchain-agents suitesruffandmypyon the touched packagesMade with Cursor
Changelog
This PR changes core routing and the evaluations runner. 1.0 is not published, so both lines are breaking and land in that cut:
BEGIN_COMMIT_OVERRIDE
feat(AIC-3439)!: let wildcard handlers accept only named providers
feat(evaluations)!: route generation and judges to a scoped wildcard handler only for its named providers, and accept two scoped wildcards of one mode when their lists differ
END_COMMIT_OVERRIDE