feat: add agentic evaluator - #274
Merged
Merged
Conversation
Passing a local model path as model_id let EvalScope join it verbatim onto the reports/predictions/reviews directories, so artifacts escaped work_dir (e.g. into the model directory). Leave model_id unset so TaskConfig derives a filesystem-safe id via safe_filename; the adapter still keeps the caller's original model_name. Add a regression test covering path-like model ids.
There was a problem hiding this comment.
🟡 Changes recommended
The new evaluator test modules should skip cleanly when the optional EvalScope extra isn’t installed, and the new batcher should not start a non-daemon thread that can keep processes alive if cleanup is missed.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds an EvalScope Native “agentic evaluator” integration to Twinkle Agentic, providing a single-use Evaluator facade plus adapters that preserve agentic protocol details (reasoning/tool calls/stops/multi-choice) at the model boundary, with optional sampler micro-batching and updated HTTP sampler serialization.
Changes:
- Introduces
twinkle_agentic.evaluator.Evaluatorplus EvalScopeModelAPIadapters for protocol APIs and structural samplers. - Adds
SamplerBatchermicro-batching with DP tail padding, and updates the HTTPvLLMSamplerto serialize TwinkleSamplingParams. - Adds optional dependency (
twinkle-kit[eval]) and bilingual documentation + tests for the new evaluator flow.
File summaries
| File | Description |
|---|---|
| tests/twinkle_agentic/evaluator/test_evaluator.py | New tests covering evaluator config validation and single-use semantics. |
| tests/twinkle_agentic/evaluator/test_conversion.py | New tests for message/tool conversion and adapter output mapping. |
| tests/twinkle_agentic/evaluator/test_client_sampler.py | Regression test for HTTP sampler SamplingParams serialization shape. |
| tests/twinkle_agentic/evaluator/test_batcher.py | Tests for sampler micro-batching, padding, and error fan-out. |
| tests/twinkle_agentic/evaluator/conftest.py | Local recording fakes/fixtures for evaluator tests. |
| tests/twinkle_agentic/evaluator/init.py | Test package init for evaluator test suite. |
| src/twinkle_client/sampler/vllm_sampler.py | Accepts Twinkle SamplingParams and embeds num_samples into sampling_params payload. |
| src/twinkle_agentic/evaluator/evaluator.py | Single-use EvalScope runner facade with strict config ownership/validation. |
| src/twinkle_agentic/evaluator/_evalscope_adapter.py | EvalScope ModelAPI adapters + converters (roles/reasoning/tools/stops/logprobs). |
| src/twinkle_agentic/evaluator/_contracts.py | Shared evaluator error types and small protocol contracts. |
| src/twinkle_agentic/evaluator/_batcher.py | New single-worker micro-batcher for structurally compatible sampler calls. |
| src/twinkle_agentic/evaluator/init.py | Public evaluator exports. |
| pyproject.toml | Adds eval optional extra pinning EvalScope. |
| docs/source_zh/使用指引/Agentic评测.md | Chinese usage guide for the evaluator and capability boundaries. |
| docs/source_zh/index.rst | Adds the new Chinese doc page to the TOC. |
| docs/source_en/Usage Guide/Agentic-Evaluator.md | English usage guide for the evaluator and capability boundaries. |
| docs/source_en/index.rst | Adds the new English doc page to the TOC. |
Review details
- Files reviewed: 16/17 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Resolve sample() conflict in twinkle_client/sampler/vllm_sampler.py: keep main's async/data-plane additions (asyncio, json_utils, DataRef, sample_to_data_plane) while retaining this branch's SamplingParams support; drop the redundant top-level num_samples wire field (the /sample handler and SampleRequest only read sampling_params). Also align the evaluator package with the repo-wide module-naming convention (rename _contracts/_batcher/_evalscope_adapter to base/batcher/ evalscope_adapter) and clear the pre-existing lint failures (isort, yapf, flake8 E501) in that package.
hjh0119
approved these changes
Sep 8, 2026
tastelikefeet
approved these changes
Sep 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SamplingParamsserializationValidation
python -m pytest tests/twinkle_agentic/evaluator tests/twinkle_agentic tests/twinkle_client/test_client_multi_turn_rollout.py -qgeneral_qaevaluation passes with both a fake protocol API and fake samplerpython -m pip wheel --no-deps .git diff --checkScope
No GPU model or external endpoint is required for the included validation. EvalScope benchmark-specific extras and real sampler/API smoke tests remain caller-provided runtime configuration.