feat(evaluations): accept inline tool definitions in run(tools=...)

[donei003]

Lets a developer define a tool in code instead of creating it in LaunchDarkly first.

_resolve_tools (evaluations/runner.py) currently requires every key in the tools map to resolve via GET projects/<p>/ai-tools/<key>, and that response is the only source for the handler config's tool description and parameters. There is no slot in the public API for a caller-supplied body.

Changes

• New exported InlineTool type carrying description, schema, and the executable (implementation), accepted in the same tools map as today's bare callables. A bare callable still means "library tool, resolve by key" — no change for existing callers, and a map may mix both sources.
• _resolve_tools skips the ai-tools GET for inline entries.
• Validation before any network I/O (_validate_tools, called from run() ahead of every request): key non-blank, schema a JSON object and JSON-serializable with allow_nan=False, implementation callable, no duplicate keys across sources, and an inline key that also names a library tool is rejected naming the key.
• Eval create body sends {key, schema, description, source: "inline"} for inline entries and {key, version, source: "library"} for library entries. tools is still omitted entirely when the map is empty.
• Handler config synthesis is unchanged in shape — config["tools"][key] = {description, parameters} is fed from the inline body instead of the GET response, and an InlineTool is unwrapped to its executable before any handler sees it, so handlers see no difference.

Decision for reviewers

Whether a NativeTool value may be paired with an inline definition. A native tool has no schema of its own, so I've made the combination a validation error rather than inventing a meaning for it.

One other thing worth a look: the brief asked for "no duplicate keys across sources" and "an inline key that also names a library tool is rejected". Because both sources share one tools mapping, a plain dict cannot express the same key twice, so those checks needed a concrete surface. Implemented as: (a) the map is validated as the caller's Mapping before it is copied into a dict, so a Mapping that yields a key twice is caught rather than silently collapsed; and (b) keys are compared case-insensitively only when an inline definition is involved — {"lookup_order": fn, "Lookup_Order": InlineTool(...)} is rejected naming the key, while two library keys differing only by case remain two library lookups, since changing that would alter existing library behavior. Both are tested. Say the word if you'd rather the collision rule were exact-case only, or extended to library/library pairs.

Verification

Run in a clean worktree off origin/main:

• uv run pytest — 1318 passed, 11 skipped (full monorepo suite).
• uv run ruff check . — all checks passed.
• uv run ruff format --check . — 114 files already formatted.
• uv run mypy packages/*/src (the repo's make typecheck, which CI mirrors) — no issues in 50 source files. Note the repo does not type-check test suites under strict, per AGENTS.md, so the new tests are not mypy-covered.

New tests in packages/client/tests/test_evaluations_run.py:

• test_inline_tool_runs_without_reading_the_tool_api — asserts the inline request sequence in full as (method, path) pairs, and asserts explicitly that no recorded URL contains /ai-tools, rather than relying on the strict-sequence transport to trip on a surplus request. Also asserts the create body's inline entry, the synthesized config["tools"], and that the handler receives the bare callable.
• test_inline_tool_description_defaults_to_empty_string
• test_mixed_library_and_inline_tools_each_keep_their_own_source — exactly one tool GET, for the library key only; both wire entries; both config entries.
• test_bad_tool_entry_is_rejected_with_zero_requests — parametrized over blank inline key, blank library key, schema=None, schema as a list, non-serializable schema, NaN, Infinity, non-callable implementation, non-string description, and an entry that is none of the three. Each asserts transport.requests == [].
• test_native_tool_paired_with_an_inline_definition_is_rejected — zero requests.
• test_native_tool_on_its_own_still_resolves_from_the_library — the library path for NativeTool is unchanged.
• test_inline_key_that_also_names_a_library_tool_is_rejected — zero requests.
• test_two_library_keys_differing_only_by_case_are_still_two_lookups — pins the library path against the new collision rule.
• test_duplicate_tool_key_across_sources_is_rejected_with_zero_requests — uses a custom Mapping that yields one key twice.

One existing assertion changed: the happy-path create body in test_complete_run_with_zero_failed_and_error_rows_passes now expects {"key": "lookup_order", "version": 7, "source": "library"}.

CI on this branch is green: Lint & format, Type check, Tests, Build all packages, Install & sync all pass.

Not verified: nothing was exercised against a real gonfalon proxy or ai-evaluator — the source discriminator and the inline body shape are asserted against the recording fake transport only.

Dependencies

Requires the gonfalon proxy/API change deployed. Cross-language spec is ai-sdks-monorepo TESTING.md §8 (spec PR open concurrently).

🤖 Generated with Claude Code

---

Note

Overview
Evaluation runs now take tools as a list of exported Tool objects instead of a key → callable map. You can Tool(...) for inline definitions (schema + description in code, no LaunchDarkly AI library entry) or evals.tools.get(key, implementation=...) to fetch a library tool once, pin its version, and pass it to run()—run() no longer resolves tools via the management API.

init_evaluations(project_key=...) (or LD_PROJECT_KEY) binds the project for datasets, evaluations, and tools.get(); project_key is removed from run(). Evaluation-create payloads distinguish source: "inline" vs source: "library". validate_tools runs before any I/O (keys, schemas, duplicates, NativeTool only on library tools). The management client parameter is renamed api_key (still env LD_API_TOKEN). Runner _resolve_tools / ResolvedTool are removed in favor of the new evaluations/tools.py helpers.

^{Reviewed by Cursor Bugbot for commit a9c40bc. Bugbot is set up for automated code reviews on this repo. Configure here.}

[devin-ai-integration]

Note

Newer findings are available below. Devin Review posted a newer report on this PR, in addition to the findings presented here.

Devin Review found 2 potential issues.

3 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)

[devin-ai-integration]

🔴 Library tool lookup blocks event loop

When async callers invoke ToolsClient.get, its synchronous GET blocks their event loop. Previously run moved tool reads to a worker thread, so slow lookups now stall unrelated requests.

Learn more

The management API client uses a synchronous urllib transport with a 30-second timeout and retry delays LDApiClient. The previous tool-resolution call ran inside asyncio.to_thread in run(). With the new public ToolsClient.get, an application calling it from an async request or async startup blocks the event-loop thread while the GET waits and retries. No other coroutine on that loop can progress until the lookup returns.

Example: An async request calls evals.tools.get("lookup_order", implementation=lookup_order) while the management API takes 30 seconds to respond. Every other request on the same loop waits those 30 seconds, even if it does not use evaluations.

Recommended fix: Offer an awaitable tool lookup that delegates the synchronous request to asyncio.to_thread, or implement an asynchronous management transport. Keep the version pinning and immediate error behavior when callers await the lookup.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[aknight-ld]

[nit] is this decorator redundant given the one beneath?

[jeffdupont]

Reviewed with the 1.0 freeze in mind, against the spec in launchdarkly/ai-sdks-monorepo#24 (my review there covers the spec side). Tests pass at a9c40bc: make test 1417 passed, 11 skipped, exit 0, and make typecheck and make lint are clean. Merged onto current main (fee904a, 7 commits past your base), it merges cleanly and still passes: 1421 passed, typecheck clean.

The behaviour matches #24 everywhere I checked. run() issues no /ai-tools request, tools.get() pins the version and records the project, the create-body shapes are right, the cross-project guard works, and bad lists fail with zero requests. There's no JS counterpart. That's expected, since the GA plan defers evals-from-code in JS past 1.0.

Four things I'd like settled before GA, because each one is a public signature:

1. The root name Tool. It's new in __all__, and in JS the root Tool is already the AI Config tool definition (index.ts:78). So both SDKs would export a root Tool with different meanings, and Python couldn't later add the AI Config Tool (types.py:79) under its JS name. I'd rename it EvalTool. Separately, my monorepo #33 moves the evaluations names to launchdarkly_ai_server.experimental.evaluations, so adding a root name now means one more to move later.
2. Tool is mutable, so "constructed means inline" doesn't hold. Reproduced at a9c40bc: t = Tool(key="search_docs", implementation=f); t.source = "library"; t.version = 99 passes validate_tools([t], "proj") and sends {"key": "search_docs", "version": 99, "source": "library"}. Its project_key is None, and the guard at tools.py:150-153 skips that case. Making the class frozen=True after 1.0 would break anyone who assigns to it, so now is the time. Inline below.
3. tools.get() blocks the event loop. Devin raised this. The docstring now says not to call it inside a running loop, but run() is async, so that's exactly where callers will be, including the README example. Making it async later is a breaking change. I'd make it awaitable now.
4. schema is optional here but required in the spec. Tool(key="a", implementation=f) validates and sends "schema": {}. §8.4 says a missing schema throws. Requiring it later is breaking, while relaxing it later is additive, so I'd drop the default.

Smaller notes:

• This breaks existing callers: run(project_key=...) is gone, init_evaluations(api_token=...) is now api_key= with no alias, and tools went from a map to a list. All of that is fine before 1.0. But the PR title has no !, and squash merges here use the PR title and body as the commit, so release-please won't flag the break in the changelog. feat(evaluations)!: ... would. The title also still says "inline tool definitions", which undersells the change.
• The description still describes the earlier InlineTool-in-a-map design: the reviewer decision, the test names, the 1318-test count. It becomes the squash commit body, so it's worth refreshing.
• packages/client/README.md: the hunk at line 165 replaces the lazy-initialization section and the init_client / get_client / shutdown / inspect_config table instead of adding beside them. That looks accidental. The same README still passes project_key= to run() at lines 64 and 100, which now raises TypeError, and line 81 still says "project_key is supplied per run rather than during initialization".
• Interaction with #127 (eval rows defined in code): both PRs change run()'s signature, and merging the two heads conflicts in __init__.py, api.py, module.py and runner.py (git merge-tree). #127 keeps project_key on run() and names its type InlineDatasetRow, while this PR drops InlineTool in favour of "constructed means inline". I'd agree one naming convention for both before either merges.
• Release order: inline entries and source depend on gonfalon#72707, which is approved but not merged. I haven't checked how the current API treats source on a library entry, so I can't say whether library-only runs keep working if this ships first.
• The lowercase check also runs on tools.get(), so a library tool the API accepted as Search_Docs can't be used from code. More on that on #24.
• dataclasses.replace(library_tool, implementation=g) returns an inline tool carrying the library schema (source, version and project_key reset because they're init=False), and that skips the project guard. It's rare, but worth a line in the docstring.
• On aknight's types.py:60 nit: the extra @dataclass leaves AIConfig.__dataclass_params__.frozen as False. Instances still refuse assignment and stay hashable (I checked), so deleting the line is safe.

[jeffdupont]

Adding Tool here freezes it at 1.0. In JS, the root Tool is the AI Config tool definition (index.ts:78, types.ts:88). Python has the same type unexported at types.py:79. If this ships as is, the same root name means two different things across the SDKs, and Python can't adopt the JS name later. EvalTool would avoid both problems. If monorepo #33 lands, this moves to experimental.evaluations anyway.

[jeffdupont]

init=False keeps these out of the constructor, but a caller can still assign to them. t.source = "library"; t.version = 99 on a constructed tool passes validate_tools and goes out as a library entry (reproduced). I'd make the class @dataclass(frozen=True) and set these three in _library with object.__setattr__. That also stops schema being reassigned after validation, though the dict itself can still be mutated in place.

[jeffdupont]

A library tool always has a project_key when it comes from tools.get(), so None here means the tool was built some other way. Skipping the check in that case is what lets a forged library tool through. I'd treat source == "library" with no project_key as an error, and drop the project_key is not None condition on the module side too, since run() always passes one.

[jeffdupont]

Callers are inside an event loop when they set up a run, because run() is awaited, and this blocks the loop for the 30 s timeout plus up to 3 retries. async def get(...) that does await asyncio.to_thread(self._api.get, path) keeps the same validation-first behaviour and matches what run() already does for every other management call. That's easy to change now and breaking after 1.0.

[jeffdupont]

§8.4 in monorepo #24 says schema is required on an inline tool, with {} legal but absence not. With this default, Tool(key="a", implementation=f) is accepted and sends "schema": {}. _library always passes schema, so making it required (no default) only affects inline construction. description keeps its default.

[jeffdupont]

This hunk replaces the "lazy initialization" section and the init_client / get_client / shutdown / inspect_config table that were here on main. They aren't anywhere else in the file now. I think the new section was meant to go next to them, not in their place. Separately, lines 64 and 100 still pass project_key= to run(), and line 81 says it's supplied per run.