Skip to content

feat(evaluations): pass the tool trajectory to scorers - #143

Open
donei003 wants to merge 2 commits into
mainfrom
feat/scorer-tool-calls
Open

donei003 wants to merge 2 commits into
mainfrom
feat/scorer-tool-calls

Conversation

@donei003

@donei003 donei003 commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Summary

A deterministic scorer can now grade the tool trajectory. Before, a scorer received only (row, output).

A scorer declares a required third positional parameter and receives a ScorerContext. A defaulted third parameter does not count.

  • tool_calls: the tool calls in the order they started. The list is empty when no tool ran.
  • tool_calls_omitted: the number of calls made after the recording limit.

A scorer with two parameters works as before. The SDK reads the signature once, in Scorer.__post_init__. A function that requires more than three positional arguments, or any keyword-only argument, raises ValueError at construction.

Future inputs are new fields on ScorerContext. The scorer signature does not change again.

The spec change is in launchdarkly/ai-sdks-monorepo, TESTING.md §8.8.4, branch claude/tool-trajectory-scorers-284fbe.

Notes

  • arguments and result on each call are bounded text, the same values a judge reads.
  • ScorerContext and ToolInvocation are exported from launchdarkly_ai_server.experimental.evaluations.

Test plan

  • uv run pytest packages/client
  • uv run mypy packages/*/src
  • uv run ruff check .

🤖 Generated with Claude Code


Note

Overview
Deterministic evaluation scorers can now grade the handler’s tool trajectory, not only (row, output). Opt in by adding a required third positional argument that receives a new frozen ScorerContext (tool_calls in start order, plus tool_calls_omitted when recording hits the cap). Two-argument scorers are unchanged.

At Scorer construction, inspect.signature sets whether the fn gets context; invalid signatures (more than three positional params, required keyword-only args) raise ValueError. The runner builds ScorerContext from each generated row’s existing trajectory fields when invoking scorers.

ScorerContext and ToolInvocation are exported from launchdarkly_ai_server.experimental.evaluations (API report updated). Tests cover sync/async scorers, empty trajectories, signature detection, and full run() with mixed scorers.

Reviewed by Cursor Bugbot for commit d402d86. Bugbot is set up for automated code reviews on this repo. Configure here.

…ntext

A scorer can declare a third parameter and receive a ScorerContext.
The context holds the tool calls in start order. Two-parameter scorers
work as before.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
devin-ai-integration[bot]

This comment was marked as resolved.

@jeffdupont jeffdupont left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passing the tool trajectory through one ScorerContext is a good shape: future inputs become new fields, and two-argument scorers keep working. Two things to fix before merge. I checked both on 4456f28 and with a trial merge of current main.

1. Must fix: an existing scorer with a defaulted third parameter now gets the context. _accepts_context counts every positional parameter, including ones with defaults (criteria.py:176). So a scorer that already exists, like def contains(row, output, case_sensitive=False), now receives a ScorerContext as case_sensitive. The context object is truthy, so the scorer takes the wrong branch. I ran that scorer through _run_scorer_for_result: on this branch it scores 0.0 for a correct answer (case_sensitive was a ScorerContext). main calls it with two arguments and it scores 1.0. That breaks the promise in the spec that no existing scorer breaks, and it fails silently, with a wrong score and no error. Smallest fix: pass the context only when the third positional parameter has no default, return len(required) >= 3, and flip case c in test_scorer_detects_the_context_parameter_once_at_construction. The spec line in monorepo #43 needs the same wording.

2. Must fix: the rebase needs four more changes. It conflicts in runner.py (imports) and tests/test_evaluations_run.py. After resolving those:

  • main moved this feature behind the experimental entry point (#157). ScorerContext has to be exported from launchdarkly_ai_server.experimental.evaluations, and added to the documented set in tests/test_evaluations.py (test_experimental_surface_is_exactly_the_documented_set). Without it the test module fails at import: ImportError: cannot import name 'ScorerContext' from 'launchdarkly_ai_server.experimental.evaluations'.
  • api-surface/python/launchdarkly_ai_server.experimental.evaluations.txt in the monorepo needs + ScorerContext.
  • _scorer_runner() builds LDApiClient(api_token=...), but main renamed the argument to api_key. Four of the new tests fail with TypeError until that's changed.

With those changes, the full client suite passes on the trial merge apart from the two surface tests that need the new name (1575 passed).

3. Should fix: the new tests skip the real path. They call the private _run_scorer_for_result with hand-built rows, so no test shows the tool calls recorded during generation (trajectory.py:206) reaching a scorer through run(). The spec also asks for a two-parameter and a three-parameter scorer in the same run, and a handler that raises after one call. One run() test with two scorers and a tool-calling handler stub would cover the first two.

The feature stays inside the experimental boundary: it is scoped evaluations, which matches its EXPERIMENTAL.md row, and nothing in core changes. 79 tests in test_evaluations_run.py pass on this head.

]
if len(required) > 3:
raise ValueError("scorer fn must accept at most (row, output, context)")
return len(positional) >= 3

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This counts defaulted parameters too, so def contains(row, output, case_sensitive=False) gets a ScorerContext as case_sensitive and scores the wrong way (probed: 0.0 here, 1.0 on main). return len(required) >= 3 keeps existing scorers safe.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d402d86: only required positional parameters count, so (row, output, case_sensitive=False) is called with two arguments.

"LDApiError",
"RunSummary",
"Scorer",
"ScorerContext",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After #157 the public import is launchdarkly_ai_server.experimental.evaluations. ScorerContext needs to be exported there too, with + ScorerContext in the monorepo's experimental evaluations list.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d402d86: ScorerContext and ToolInvocation are exported from experimental.evaluations, in the surface test and the API report. The monorepo experimental list still needs the same addition (not done here).

from launchdarkly_ai_server.evaluations.runner import EvaluationsRunner

return EvaluationsRunner(
LDApiClient(api_token="token", transport=failing_transport)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

main renamed this argument to api_key, so after the rebase this helper raises TypeError and four of the new tests fail.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d402d86: the helper uses api_key=.

assert Scorer(name="a", fn=lambda row, output: 1.0).accepts_context is False
assert Scorer(name="b", fn=lambda row, output, ctx: 1.0).accepts_context is True
assert (
Scorer(name="c", fn=lambda row, output, ctx=None: 1.0).accepts_context is True

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case locks in the behaviour from finding 1. With the fix, ctx=None would be False. A scorer that wants an optional context can make it required.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flipped in d402d86: ctx=None now asserts _accepts_context is False.

… detection

Merge: LDApiClient is internal and takes api_key=; the public path is
experimental/evaluations.py, so evaluations/__init__.py stays as on main.

On top of the merge:
- A scorer gets a ScorerContext only when it REQUIRES a third positional
  parameter; (row, output, ctx=None) is called with two arguments.
- A required keyword-only parameter raises ValueError at construction,
  instead of failing on every row.
- Scorer.accepts_context is now _accepts_context.
- ScorerContext and ToolInvocation are exported from
  experimental.evaluations and added to the surface test; API report updated.
- New end-to-end run() test with a 2-parameter and a 3-parameter scorer.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jeffdupont
jeffdupont requested a review from a team as a code owner October 23, 2026 19:23

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)

Devin Review

]
if len(positional) > 3:
raise ValueError("scorer fn must accept at most (row, output, context)")
return len(positional) == 3

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Optional scorer context loses tool calls

When a scorer declares context=None as its third parameter, _takes_context returns false and the scorer receives no trajectory. Its tool-call checks then score the wrong data or fail.

Learn more

Scorers can accept a third positional context parameter with a default. The earlier signature check passed context to those functions, but _takes_context now counts only required parameters. The runner uses this boolean to decide whether to pass ScorerContext, so an optional context receives its default instead of the recorded tool calls. Keep the required-argument validation while detecting whether the signature declares a third positional slot.

Example: def score(row, output, context=None): return float(bool(context and context.tool_calls)) receives None despite a recorded tool call and records 0.0 instead of 1.0.

Recommended fix: In _takes_context, count all declared positional parameters for context detection after separately checking that no more than three positional parameters are required. Add a test that an optional third parameter receives populated ScorerContext.

Devin Review


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

@jeffdupont

Copy link
Copy Markdown
Contributor

Pushed d402d86 (merge of origin/main plus fixes):

  • A ScorerContext is passed only when the scorer REQUIRES a third positional parameter. The ctx=None test now expects False.
  • A required keyword-only parameter raises ValueError when the Scorer is built.
  • Scorer.accepts_context is now _accepts_context.
  • ScorerContext and ToolInvocation are exported from experimental.evaluations, added to the surface test, and the API report is regenerated.
  • Merge: the test helper uses LDApiClient(api_key=...); evaluations/__init__.py stays as on main.
  • New end-to-end run() test with a 2-parameter and a 3-parameter scorer.

Replied to all 5 open threads. Updated the PR body to match. Still open: the monorepo experimental-evaluations list needs ScorerContext too. Needs a human reviewer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants