feat: add agent driver abstraction layer for evaluation pipeline#231
Merged
Conversation
Contributor
WalkthroughThe pull request introduces an agent driver abstraction to decouple turn execution from direct API client usage, refactors the evaluation pipeline and processor to route conversations through driver instances, and adds comprehensive unit and integration tests with system configuration examples for agents-based evaluation. ChangesAgent Driver Architecture and Pipeline Refactoring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
tests/unit/pipeline/evaluation/test_driver.py (1)
171-207: 💤 Low value
StubDriveris defined identically in both base-class test methods — extractable as a shared class-level fixture.Both
test_enabled_default_is_trueandtest_close_is_noopdefine an identicalStubDriverinner class. The duplication is minor but a class-level nested class (or a module-level class) would remove the repetition.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/pipeline/evaluation/test_driver.py` around lines 171 - 207, The two tests test_enabled_default_is_true and test_close_is_noop both define an identical inner StubDriver class; refactor by moving StubDriver out of the test methods into a shared scope (e.g., as a class-level attribute on TestAgentDriverBase or a module-level helper) so both tests instantiate the same StubDriver without duplication; ensure the moved StubDriver implements execute_turn and validate_config exactly as in the current inner definitions and update the tests to construct driver = StubDriver({}) and call driver.enabled / driver.close() accordingly.tests/integration/test_full_evaluation.py (1)
43-54: 💤 Low valueConsider failing loudly when neither
agents:nor a default agent is set.The fallback
return False, "query"silently treats a missing/empty agents config as "disabled query agent". Given thatmigrate_api_to_agentsalways populatesagentswhenapi:is present, hitting this branch in an integration config almost certainly indicates a misconfigured fixture YAML rather than legitimate behavior. Raising an explicit error (or asserting at fixture load time) would catch such mistakes earlier.♻️ Possible tightening
def get_agent_info(system_config: SystemConfig) -> tuple[bool, str]: """Resolve agent enabled status and endpoint_type from any config format. Works for both legacy api: and native agents: configs since migrate_api_to_agents() ensures agents is always populated when api: is present. """ if system_config.agents and system_config.agents.default.agent: name = system_config.agents.default.agent agent_def = system_config.agents.agents[name] return agent_def.enabled, agent_def.endpoint_type - return False, "query" + raise AssertionError( + "Integration config has neither agents: nor api: — " + "expected migration to populate agents.default.agent" + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/test_full_evaluation.py` around lines 43 - 54, The helper get_agent_info currently falls back to returning (False, "query") when no agents/default agent is present, which hides misconfigured fixtures; change it to raise an explicit error (e.g., raise RuntimeError or ValueError) when system_config.agents is missing or system_config.agents.default.agent is empty, including the problematic system_config (or its name/identifier) in the message so tests fail loudly and point to the bad fixture; preserve the existing branch that reads name = system_config.agents.default.agent and returns agent_def.enabled, agent_def.endpoint_type when present.src/lightspeed_evaluation/core/models/system.py (1)
805-829: 💤 Low valueUse
exclude_unset=Trueinmodel_dump()and return a new dict instead of mutating the input.Two optional improvements to
migrate_api_to_agents:
Forwarding unset fields (line 820):
api_data.model_dump(exclude={"enabled"})serializes every field including those withNonedefaults. This means the synthesized agent config will explicitly contain thoseNonevalues, preventing the agent's own defaults from taking effect if they differ from APIConfig's defaults. Useexclude_unset=Trueto forward only user-provided values.In-place mutation of the input dict (line 825):
data["agents"] = {...}mutates the caller's dict when they invokeSystemConfig.model_validate(my_dict). Return a shallow copy instead to avoid surprising side effects on shared dict references.Suggested adjustment
`@model_validator`(mode="before") `@classmethod` def migrate_api_to_agents(cls, data: Any) -> Any: """Auto-migrate api: to agents: when agents: is absent.""" if not isinstance(data, dict): return data if data.get("agents") is not None or data.get("api") is None: return data api_data = data["api"] if isinstance(api_data, dict): api_enabled = api_data.get("enabled", True) agent_fields = {k: v for k, v in api_data.items() if k != "enabled"} elif isinstance(api_data, BaseModel): api_enabled = getattr(api_data, "enabled", True) - agent_fields = api_data.model_dump(exclude={"enabled"}) + agent_fields = api_data.model_dump( + exclude={"enabled"}, exclude_unset=True + ) else: return data agent_fields["type"] = "http_api" - data["agents"] = { + return { + **data, "default": {"agent": "http_api" if api_enabled else None}, "http_api": agent_fields, - } - return data + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lightspeed_evaluation/core/models/system.py` around lines 805 - 829, migrate_api_to_agents currently mutates the input dict and uses api_data.model_dump(exclude={"enabled"}) which includes unset fields; change it to call model_dump(exclude_unset=True, exclude={"enabled"}) to only forward user-provided values, and return a shallow copy of the incoming mapping instead of mutating it: make a new_data = dict(data) (or copy.copy(data)), populate new_data["agents"] with the synthesized structure (ensure agent_fields is a new dict in both dict and BaseModel branches), and return new_data; references: migrate_api_to_agents, api_data.model_dump, data["agents"], agent_fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lightspeed_evaluation/core/models/data.py`:
- Around line 405-410: Update the Pydantic Field for the agent attribute in the
model defined in src/lightspeed_evaluation/core/models/data.py to use a fully
anchored regex so leading/trailing/embedded whitespace is rejected: replace the
current pattern r"\S" on the agent Field with r"^\S+$" (you can also remove the
now-redundant min_length=1) so lookups against agent names (e.g., in agents
config) won't silently fail due to surrounding spaces.
In `@src/lightspeed_evaluation/pipeline/evaluation/driver.py`:
- Around line 97-102: The constructor currently treats an explicit empty dict
the same as None and also adds a file-level pylint disable; change
AgentDriverRegistry.__init__ to preserve injected empty registries by using an
identity None check (if drivers is None: self._drivers = AGENT_DRIVERS else:
self._drivers = drivers) and remove the "# pylint:
disable=too-few-public-methods" comment from the class declaration so linting
isn't suppressed; keep the class small as-is (or convert to a tiny factory
elsewhere if preferred), referencing AgentDriverRegistry.__init__ and
AGENT_DRIVERS to locate the change.
- Around line 50-66: The execute_turn method must no-op when the driver is
disabled: add a guard at the start of execute_turn that checks self._enabled
and, if False, returns (None, conversation_id) without calling
self._amender.amend_single_turn; also avoid constructing side-effecting members
when disabled by early-returning or skipping creation of self._api_client and
self._amender in __init__ (check _enabled after parsing config) so disabled
drivers do nothing but still present the same API.
In `@src/lightspeed_evaluation/pipeline/evaluation/pipeline.py`:
- Around line 185-188: The save branch currently checks only
self._default_driver.enabled so per-conversation drivers that are enabled can be
missed; update the conversation driver resolution/processing code (where you
determine the effective driver for each conversation—e.g., the method that
resolves or applies drivers) to set a boolean flag like
self._used_enabled_driver whenever the effective driver for any processed
conversation is enabled, ensure this flag is initialized/reset at the start of
the evaluation run, and change the final save check to use
self._used_enabled_driver before calling
self._save_amended_data(evaluation_data) instead of checking
self._default_driver.enabled.
In `@tests/unit/pipeline/evaluation/test_driver.py`:
- Around line 99-112: The test_enabled_reflects_config test misses patching
APIClient so HttpApiDriver.__init__ creates a real client; patch
"lightspeed_evaluation.pipeline.evaluation.driver.APIClient" (e.g., with
mocker.patch) before instantiating HttpApiDriver in test_enabled_reflects_config
so the driver uses a mocked APIClient instance (similar to
test_close_with_api_client), ensuring isolation when mock_config.enabled = True.
---
Nitpick comments:
In `@src/lightspeed_evaluation/core/models/system.py`:
- Around line 805-829: migrate_api_to_agents currently mutates the input dict
and uses api_data.model_dump(exclude={"enabled"}) which includes unset fields;
change it to call model_dump(exclude_unset=True, exclude={"enabled"}) to only
forward user-provided values, and return a shallow copy of the incoming mapping
instead of mutating it: make a new_data = dict(data) (or copy.copy(data)),
populate new_data["agents"] with the synthesized structure (ensure agent_fields
is a new dict in both dict and BaseModel branches), and return new_data;
references: migrate_api_to_agents, api_data.model_dump, data["agents"],
agent_fields.
In `@tests/integration/test_full_evaluation.py`:
- Around line 43-54: The helper get_agent_info currently falls back to returning
(False, "query") when no agents/default agent is present, which hides
misconfigured fixtures; change it to raise an explicit error (e.g., raise
RuntimeError or ValueError) when system_config.agents is missing or
system_config.agents.default.agent is empty, including the problematic
system_config (or its name/identifier) in the message so tests fail loudly and
point to the bad fixture; preserve the existing branch that reads name =
system_config.agents.default.agent and returns agent_def.enabled,
agent_def.endpoint_type when present.
In `@tests/unit/pipeline/evaluation/test_driver.py`:
- Around line 171-207: The two tests test_enabled_default_is_true and
test_close_is_noop both define an identical inner StubDriver class; refactor by
moving StubDriver out of the test methods into a shared scope (e.g., as a
class-level attribute on TestAgentDriverBase or a module-level helper) so both
tests instantiate the same StubDriver without duplication; ensure the moved
StubDriver implements execute_turn and validate_config exactly as in the current
inner definitions and update the tests to construct driver = StubDriver({}) and
call driver.enabled / driver.close() accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fb7bfdd5-39f1-4be6-b236-a466ec5155a4
📒 Files selected for processing (21)
src/lightspeed_evaluation/core/constants.pysrc/lightspeed_evaluation/core/models/__init__.pysrc/lightspeed_evaluation/core/models/agents.pysrc/lightspeed_evaluation/core/models/data.pysrc/lightspeed_evaluation/core/models/system.pysrc/lightspeed_evaluation/core/system/loader.pysrc/lightspeed_evaluation/pipeline/evaluation/__init__.pysrc/lightspeed_evaluation/pipeline/evaluation/driver.pysrc/lightspeed_evaluation/pipeline/evaluation/pipeline.pysrc/lightspeed_evaluation/pipeline/evaluation/processor.pytests/integration/system-config-agents-query.yamltests/integration/system-config-agents-streaming.yamltests/integration/test_full_evaluation.pytests/unit/core/models/test_agents.pytests/unit/core/models/test_data.pytests/unit/core/models/test_system.pytests/unit/core/system/test_loader.pytests/unit/pipeline/evaluation/conftest.pytests/unit/pipeline/evaluation/test_driver.pytests/unit/pipeline/evaluation/test_pipeline.pytests/unit/pipeline/evaluation/test_processor.py
rioloc
force-pushed
the
feat/agent-driver-layer
branch
2 times, most recently
from
f8bd187 to
d7d84e8
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lightspeed_evaluation/pipeline/evaluation/pipeline.py (1)
173-190:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReset
_has_agent_amendmentsat the start of each run.The flag is only initialized in
__init__. If the sameEvaluationPipelineinstance is reused, one earlier run with an enabled driver leaves itTrue, and later runs will still try to persist amended data even when every effective driver is disabled.Possible fix
def run_evaluation( self, evaluation_data: list[EvaluationData], original_data_path: Optional[str] = None, ) -> list[EvaluationResult]: @@ """ self.original_data_path = original_data_path + self._has_agent_amendments = False logger.info("Starting evaluation")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lightspeed_evaluation/pipeline/evaluation/pipeline.py` around lines 173 - 190, Reset the instance-level flag self._has_agent_amendments at the start of each run to avoid carrying over state between runs; inside the method that initializes the run (the block that sets self.original_data_path, calls self.storage_backend.initialize(RunInfo(...)) and then calls self._process_eval_data), add a statement to set self._has_agent_amendments = False before processing so amended-data persistence only occurs for changes made during the current invocation.
🧹 Nitpick comments (1)
tests/integration/test_full_evaluation.py (1)
43-54: ⚡ Quick winReuse
resolve_agent_config()here to keep the test aligned with production resolution.This helper duplicates the default-agent lookup by reaching into
system_config.agents.agents[name]directly. Calling the same resolver the pipeline uses will make these tests follow future config-resolution changes automatically instead of drifting.Possible refactor
def get_agent_info(system_config: SystemConfig) -> tuple[bool, str]: @@ - if system_config.agents and system_config.agents.default.agent: - name = system_config.agents.default.agent - agent_def = system_config.agents.agents[name] - return agent_def.enabled, agent_def.endpoint_type + if system_config.agents and system_config.agents.default.agent: + _name, agent_config = system_config.agents.resolve_agent_config() + return ( + bool(agent_config.get("enabled", False)), + str(agent_config.get("endpoint_type", "query")), + ) return False, "query"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/test_full_evaluation.py` around lines 43 - 54, The helper get_agent_info duplicates config resolution; replace its manual lookup with the production resolver resolve_agent_config: if a default agent name exists (system_config.agents.default.agent) call resolve_agent_config(system_config, <default_name>) to obtain the resolved agent config and return its enabled and endpoint_type, and if resolve_agent_config returns no config fall back to returning (False, "query"); keep function signature get_agent_info(system_config: SystemConfig) and only change the body to use resolve_agent_config.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lightspeed_evaluation/pipeline/evaluation/pipeline.py`:
- Line 45: The class-level pylint suppression on EvaluationPipeline must be
removed and the attribute fan-out reduced: create a small data holder (e.g.,
EvaluationContext or PipelineContext) to encapsulate related runtime state and
collaborators currently stored as separate instance attributes on
EvaluationPipeline, move those attributes into that context object, update
EvaluationPipeline.__init__ to accept and store one context instance (and adjust
any references inside methods to use self.context.<name>), and delete the "#
pylint: disable=too-many-instance-attributes" comment; ensure tests and usages
construct the new context when instantiating EvaluationPipeline.
- Around line 147-157: The code currently raises ConfigurationError when
self.system_config.agents is None even if a per-conversation override exists;
update the logic in the block that checks self.system_config.agents so that if
conv_data.agent_config (or conv_data.agent) is provided and self._registry can
build a driver from that override, you bypass resolve_agent_config and call
self._registry.create_driver(agent_config_override) directly, returning (driver,
True); only raise ConfigurationError when no agents are configured and there is
no per-conversation agent_config to construct a driver. Ensure you reference
system_config.agents, conv_data.agent_config, resolve_agent_config,
ConfigurationError, and _registry.create_driver in the change.
---
Outside diff comments:
In `@src/lightspeed_evaluation/pipeline/evaluation/pipeline.py`:
- Around line 173-190: Reset the instance-level flag self._has_agent_amendments
at the start of each run to avoid carrying over state between runs; inside the
method that initializes the run (the block that sets self.original_data_path,
calls self.storage_backend.initialize(RunInfo(...)) and then calls
self._process_eval_data), add a statement to set self._has_agent_amendments =
False before processing so amended-data persistence only occurs for changes made
during the current invocation.
---
Nitpick comments:
In `@tests/integration/test_full_evaluation.py`:
- Around line 43-54: The helper get_agent_info duplicates config resolution;
replace its manual lookup with the production resolver resolve_agent_config: if
a default agent name exists (system_config.agents.default.agent) call
resolve_agent_config(system_config, <default_name>) to obtain the resolved agent
config and return its enabled and endpoint_type, and if resolve_agent_config
returns no config fall back to returning (False, "query"); keep function
signature get_agent_info(system_config: SystemConfig) and only change the body
to use resolve_agent_config.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a840a7b3-29ba-43df-8487-0867d78897c0
📒 Files selected for processing (6)
src/lightspeed_evaluation/core/models/data.pysrc/lightspeed_evaluation/pipeline/evaluation/driver.pysrc/lightspeed_evaluation/pipeline/evaluation/pipeline.pytests/integration/test_full_evaluation.pytests/unit/pipeline/evaluation/test_driver.pytests/unit/pipeline/evaluation/test_pipeline.py
✅ Files skipped from review due to trivial changes (1)
- tests/unit/pipeline/evaluation/test_driver.py
🚧 Files skipped from review as they are similar to previous changes (3)
- src/lightspeed_evaluation/core/models/data.py
- src/lightspeed_evaluation/pipeline/evaluation/driver.py
- tests/unit/pipeline/evaluation/test_pipeline.py
rioloc
force-pushed
the
feat/agent-driver-layer
branch
from
d7d84e8 to
4c91ba8
Compare
7 tasks
rioloc
force-pushed
the
feat/agent-driver-layer
branch
4 times, most recently
from
c5bfc3f to
361d20f
Compare
Introduce AgentDriver/HttpApiDriver/AgentDriverRegistry to decouple agent execution from the evaluation pipeline. The driver abstraction enables future agent types (subprocess, MCP) without modifying the pipeline core. Key changes: - AgentDriver ABC with execute_turn/validate_config/close interface - HttpApiDriver wraps existing API client logic behind the driver API - AgentDriverRegistry factory creates drivers by type from config - Pipeline resolves per-conversation agent overrides via EvaluationData - AgentsConfig.enabled top-level kill switch passed at driver construction (not stored per-agent, since HttpApiAgentConfig uses extra="forbid") - Processor uses driver.enabled instead of checking config directly Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
rioloc
force-pushed
the
feat/agent-driver-layer
branch
from
361d20f to
de6e59f
Compare
asamal4
reviewed
May 15, 2026
Collaborator
asamal4
left a comment
asamal4
left a comment
There was a problem hiding this comment.
Thank you !!
Note that we need to make caching independent of individual agent driver, as it will be applicable for all.. Perhaps a different change before we add agentic ols flow.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/integration/test_full_evaluation.py (1)
110-114: ⚡ Quick winUse
xfailinstead of commenting out streaming scenarios.Commented tests hide streaming coverage in CI. Marking known failures as
xfailkeeps visibility and detects accidental fixes.Suggested refactor
[ ("query_config_path", "query"), ("agents_query_config_path", "query"), - # TODO: investigate streaming parse error "No final response found - # in streaming output" — fails for both legacy api: and agents: - # config formats, not related to agent driver changes - # ("streaming_config_path", "streaming"), - # ("agents_streaming_config_path", "streaming"), + pytest.param( + "streaming_config_path", + "streaming", + marks=pytest.mark.xfail( + reason='Known issue: "No final response found in streaming output"' + ), + ), + pytest.param( + "agents_streaming_config_path", + "streaming", + marks=pytest.mark.xfail( + reason='Known issue: "No final response found in streaming output"' + ), + ), ],Also applies to: 228-231
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/test_full_evaluation.py` around lines 110 - 114, Replace the commented-out streaming parameter entries with pytest xfail markers so CI still runs them but records expected failures: locate the parametrize block that currently contains the commented ("streaming_config_path", "streaming") and ("agents_streaming_config_path", "streaming") entries (also the similar block around 228-231), and instead add pytest.param(...) entries wrapped with pytest.mark.xfail(reason="streaming parse error: No final response found in streaming output", strict=False) referencing the exact parameter names streaming_config_path and agents_streaming_config_path so the test harness runs these scenarios but marks them as expected failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/integration/test_full_evaluation.py`:
- Around line 50-53: get_agent_info currently assumes
system_config.agents.default.agent exists in system_config.agents.agents and
will raise a raw KeyError; add a guard in get_agent_info to check that
default.agent is present in the agents mapping before indexing, and if missing
either raise a clear ValueError (or log an explicit error) that includes the
missing agent name and context (e.g., use system_config.agents.default.agent and
system_config.agents.enabled in the message), and only return
(system_config.agents.enabled, agent_def.endpoint_type) after safely retrieving
agent_def from system_config.agents.agents[name].
---
Nitpick comments:
In `@tests/integration/test_full_evaluation.py`:
- Around line 110-114: Replace the commented-out streaming parameter entries
with pytest xfail markers so CI still runs them but records expected failures:
locate the parametrize block that currently contains the commented
("streaming_config_path", "streaming") and ("agents_streaming_config_path",
"streaming") entries (also the similar block around 228-231), and instead add
pytest.param(...) entries wrapped with pytest.mark.xfail(reason="streaming parse
error: No final response found in streaming output", strict=False) referencing
the exact parameter names streaming_config_path and agents_streaming_config_path
so the test harness runs these scenarios but marks them as expected failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6308208c-1673-460c-a2fb-651f30cc51a5
📒 Files selected for processing (11)
src/lightspeed_evaluation/pipeline/evaluation/__init__.pysrc/lightspeed_evaluation/pipeline/evaluation/driver.pysrc/lightspeed_evaluation/pipeline/evaluation/pipeline.pysrc/lightspeed_evaluation/pipeline/evaluation/processor.pytests/integration/system-config-agents-query.yamltests/integration/system-config-agents-streaming.yamltests/integration/test_full_evaluation.pytests/unit/pipeline/evaluation/conftest.pytests/unit/pipeline/evaluation/test_driver.pytests/unit/pipeline/evaluation/test_pipeline.pytests/unit/pipeline/evaluation/test_processor.py
🚧 Files skipped from review as they are similar to previous changes (8)
- src/lightspeed_evaluation/pipeline/evaluation/init.py
- tests/integration/system-config-agents-query.yaml
- tests/integration/system-config-agents-streaming.yaml
- tests/unit/pipeline/evaluation/test_pipeline.py
- tests/unit/pipeline/evaluation/test_driver.py
- src/lightspeed_evaluation/pipeline/evaluation/processor.py
- src/lightspeed_evaluation/pipeline/evaluation/driver.py
- tests/unit/pipeline/evaluation/test_processor.py
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
2 participants
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
Introduces an agent driver abstraction layer (
AgentDriverRegistry → AgentDriver(ABC) → HttpApiDriver) that replaces directAPIDataAmenderusage in the evaluation pipeline. This enables per-conversation agent resolution and lays the groundwork for supporting multiple agent types beyond HTTP API.AgentDriver(ABC)abstract base withexecute_turn()/validate_config()/close()interface, andHttpApiDriverimplementation wrapping existingAPIDataAmenderAgentDriverRegistryfactory for driver creation by typeEvaluationData.agent/agent_configfieldsAgentsConfig.enabledkill switch passed at driver construction time (not stored per-agent, sinceHttpApiAgentConfigusesextra="forbid"). When disabled, the pipeline returns the default (disabled) driver for all conversationsagents:format configs alongside legacyapi:configs, verifying both backward compatibility and the new config pathArchitecture
graph TD subgraph Pipeline EP[EvaluationPipeline] EP -->|creates| ADR[AgentDriverRegistry] EP -->|resolves config| DAC[_resolve_default_agent_config] EP -->|resolves per-conv| RDC[_resolve_driver_for_conversation] end subgraph "Driver Layer" ADR -->|create_driver| AD[AgentDriver ABC] AD -->|implements| HAD[HttpApiDriver] AD -.->|future| FD[Other Drivers...] HAD -->|wraps| ADA[APIDataAmender] end subgraph Processor CP[ConversationProcessor] CP -->|execute_turn| AD end EP -->|default driver shared| CP RDC -->|per-conv driver| CP style AD fill:#f9f,stroke:#333 style HAD fill:#bbf,stroke:#333 style FD fill:#ddd,stroke:#999,stroke-dasharray: 5 5Key design decisions
execute_turn()receivesconversation_idas parameter and returns it — no internal state, safe for thread sharingEvaluationPipeline(SRP — processor has zero config responsibility)AgentsConfig:enabledis resolved fromAgentsConfig.enabledand passed as a constructor kwarg throughcreate_driver → AgentDriver. The processor checksdriver.enabledto decide whether to execute or skipChanges
Production code
pipeline/evaluation/driver.pyAgentDriver(ABC),HttpApiDriver,AgentDriverRegistrypipeline/evaluation/pipeline.pyAPIClient/APIDataAmenderwith driver architecturepipeline/evaluation/processor.pyAgentDriverper-call instead of storedAPIDataAmenderpipeline/evaluation/__init__.pyTests
tests/unit/pipeline/evaluation/test_driver.pytests/unit/pipeline/evaluation/test_pipeline.pytests/unit/pipeline/evaluation/test_processor.pytests/unit/pipeline/evaluation/conftest.pytests/integration/test_full_evaluation.pyTestFullApiEvaluation, extended parametrization for both config formatstests/integration/system-config-agents-query.yamlagents:format configtests/integration/system-config-agents-streaming.yamlagents:format configTest plan
make black-format— all files formattedmake pre-commit— bandit, pyright, docstyle, ruff, pylint, black-check passmake test— 877 unit tests pass, 0 failures🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests