Merged
Conversation
Contributor
|
Important Review skippedThis PR was authored by the user configured for CodeRabbit reviews. By default, CodeRabbit skips reviewing PRs authored by this user. It's recommended to use a dedicated user account to post CodeRabbit review feedback. To trigger a single review, invoke the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAgents now accept validated Pydantic configs (BaseAgentConfig); tests and models gained agent_config with validators and merging; CLI instantiates agents with validated configs and adjusted setup returns; cloudai_gym action-space typing changed and first_sweep added; tests and docs updated. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
podkidyshev
commented
Feb 25, 2026
podkidyshev
requested review from
amaslenn,
jeffnvidia and
srivatsankrishnan
as code owners
![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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cloudai/configurator/base_agent.py (1)
90-108: 🧹 Nitpick | 🔵 TrivialMinor type-hint style inconsistency:
Dict(line 101) vsdict(line 91).Lines 91 and 101 use different styles for the same generic dict type. Consider unifying to lowercase
dictfor consistency with the rest of the file.📝 Proposed fix
`@abstractmethod` - def update_policy(self, _feedback: Dict[str, Any]) -> None: + def update_policy(self, _feedback: dict[str, Any]) -> None:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cloudai/configurator/base_agent.py` around lines 90 - 108, Unify the type-hint style by changing the uppercase typing.Dict usage to the lowercase built-in dict in the abstract methods; specifically update select_action's return type and docstring and update_policy's _feedback parameter annotation to use dict[str, Any] so both select_action and update_policy (and their docstrings) consistently use dict rather than Dict.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cloudai/configurator/base_agent.py`:
- Around line 27-33: The random_seed default in BaseAgentConfig uses
int(time.time()) which only has second-level granularity and can produce
duplicate seeds; change the default_factory for the random_seed Field in class
BaseAgentConfig to use a higher-resolution source (e.g. time.time_ns() or a
cryptographic random int like random.SystemRandom().randint(0, 2**31-1)) and
ensure the corresponding import (time or random) is present so new configs
created in tight loops receive unique seeds.
- Around line 85-88: The return annotation type[CfgT] on the `@staticmethod`
get_config_class is unbound from the class's generic TypeVar; change the API so
static/class typing resolves correctly: either make get_config_class a
`@classmethod` and annotate its return as Type[CfgT] (so the TypeVar is bound to
the subclass) or, if you prefer not to change to classmethod, annotate the
return as Type[BaseAgentConfig] to satisfy static type checkers; update the
get_config_class declaration (and any callers) accordingly and keep the method
abstract (reference: get_config_class, CfgT, BaseAgentConfig).
- Around line 69-76: Update the __init__ docstring in class base_agent to
reflect the actual parameter types: replace the stale "BaseGym" with
"CloudAIGymEnv" (and ensure CfgT remains documented), and mention that the
config class is defined by get_config_class; specifically edit the __init__
docstring for __init__(self, env: CloudAIGymEnv, config: CfgT) to show env:
CloudAIGymEnv and config: CfgT so the types match the signature and references
the get_config_class static method.
In `@src/cloudai/models/scenario.py`:
- Around line 143-156: Extract the duplicated agent-validation logic into a
shared helper function, e.g. _validate_agent_in_registry(agent: str | None) ->
str | None, that checks Registry().agents_map for the agent and raises
ValueError if missing (preserving Optional handling), then replace both
field_validator methods named validate_agent in TestRunModel and TestDefinition
to call this helper; similarly refactor validate_agent_config to reuse the
helper (or call validate_agent to ensure agent is valid) and then perform the
agent_config class lookup via
Registry().agents_map[self.agent].get_config_class().model_validate(self.agent_config),
keeping existing return types and error behavior.
---
Outside diff comments:
In `@src/cloudai/configurator/base_agent.py`:
- Around line 90-108: Unify the type-hint style by changing the uppercase
typing.Dict usage to the lowercase built-in dict in the abstract methods;
specifically update select_action's return type and docstring and
update_policy's _feedback parameter annotation to use dict[str, Any] so both
select_action and update_policy (and their docstrings) consistently use dict
rather than Dict.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
src/cloudai/cli/handlers.pysrc/cloudai/configurator/base_agent.pysrc/cloudai/configurator/base_gym.pysrc/cloudai/configurator/grid_search.pysrc/cloudai/core.pysrc/cloudai/models/scenario.pysrc/cloudai/models/workload.pytests/test_agents.pytests/test_cloudaigym.pytests/test_handlers.pytests/test_test_scenario.py
Contributor
Greptile SummaryThis PR implements agent configuration capabilities for the CloudAI framework, enabling users to configure agent behavior through structured config objects. Key changes:
Implementation notes:
Confidence Score: 5/5
Important Files Changed
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/cloudai/models/scenario.py (1)
143-156: Duplicate validation logic acrossTestRunModelandTestDefinition.The
validate_agentandvalidate_agent_configvalidators here (lines 143–156) are near-identical to their counterparts insrc/cloudai/models/workload.py(lines 145–157). Consider extracting a shared helper to reduce duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cloudai/models/scenario.py` around lines 143 - 156, Both TestRunModel/TestDefinition contain duplicate validators validate_agent and validate_agent_config; extract the shared logic into a single helper and call it from each model to remove duplication. Create a reusable function (e.g., validate_agent_value(agent: str|None) -> str|None) that checks Registry().agents_map and raises the same ValueError, and another helper (e.g., validate_agent_config_for_instance(instance) or validate_agent_config(agent, agent_config)) that resolves agent_class via Registry().agents_map, calls get_config_class() and model_validate on agent_config; replace the current `@field_validator`("agent") and `@model_validator` methods in TestRunModel and TestDefinition to delegate to these helpers. Ensure the helpers are imported where needed and retain identical error messages and behavior.src/cloudai/configurator/base_agent.py (1)
73-75:⚠️ Potential issue | 🟡 MinorUpdate stale docstring type for
env.The constructor signature uses
CloudAIGymEnv, but the docstring still saysBaseGym.📝 Proposed fix
- env (BaseGym): The environment instance for the agent. + env (CloudAIGymEnv): The environment instance for the agent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cloudai/configurator/base_agent.py` around lines 73 - 75, The docstring for the constructor incorrectly types the env parameter as BaseGym while the signature uses CloudAIGymEnv; update the Args section for env to reference CloudAIGymEnv (and ensure CloudAIGymEnv is imported or fully-qualified if necessary) in the __init__ of the class in base_agent.py so the docstring matches the actual parameter type and the get_config_class reference remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cloudai/cli/handlers.py`:
- Around line 147-149: The code reads agent config from tr.test.agent_config
while the local environment was created from test_run (deep copy); to be
consistent, replace the read to use test_run.test.agent_config when constructing
agent_config_data (the call to agent_class.get_config_class().model_validate
should receive test_run.test.agent_config) so agent_config_data, agent =
agent_class(env, agent_config), and env all reference the same test_run-derived
object.
In `@src/cloudai/configurator/base_agent.py`:
- Line 83: Replace the debug log that prints the full config payload
(logging.debug(f"Initializing agent {self.__class__.__name__} with config:
{self.config.model_dump()}")) so it does not emit secrets/PII; instead log only
safe metadata such as the agent class name (self.__class__.__name__) and either
the list of config keys (self.config.__fields__.keys() or equivalent) or a
curated allowlist of non-sensitive fields, and if needed redact or omit any
values from self.config.model_dump() before logging; update the log site in
base_agent.py where model_dump() is called to use the safe summary instead.
In `@src/cloudai/configurator/grid_search.py`:
- Around line 35-38: Update the docstring for the `config` parameter in
`grid_search.py` to remove "not used" and state that the config is passed to and
handled by `BaseAgent.__init__` (it is stored and logged by the base class);
reference the `config` parameter in the function/class docstring and briefly
describe its role rather than claiming it's unused so readers understand it is
consumed by `BaseAgent.__init__`.
---
Duplicate comments:
In `@src/cloudai/configurator/base_agent.py`:
- Around line 73-75: The docstring for the constructor incorrectly types the env
parameter as BaseGym while the signature uses CloudAIGymEnv; update the Args
section for env to reference CloudAIGymEnv (and ensure CloudAIGymEnv is imported
or fully-qualified if necessary) in the __init__ of the class in base_agent.py
so the docstring matches the actual parameter type and the get_config_class
reference remains unchanged.
In `@src/cloudai/models/scenario.py`:
- Around line 143-156: Both TestRunModel/TestDefinition contain duplicate
validators validate_agent and validate_agent_config; extract the shared logic
into a single helper and call it from each model to remove duplication. Create a
reusable function (e.g., validate_agent_value(agent: str|None) -> str|None) that
checks Registry().agents_map and raises the same ValueError, and another helper
(e.g., validate_agent_config_for_instance(instance) or
validate_agent_config(agent, agent_config)) that resolves agent_class via
Registry().agents_map, calls get_config_class() and model_validate on
agent_config; replace the current `@field_validator`("agent") and `@model_validator`
methods in TestRunModel and TestDefinition to delegate to these helpers. Ensure
the helpers are imported where needed and retain identical error messages and
behavior.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
src/cloudai/cli/handlers.pysrc/cloudai/configurator/base_agent.pysrc/cloudai/configurator/base_gym.pysrc/cloudai/configurator/grid_search.pysrc/cloudai/core.pysrc/cloudai/models/scenario.pysrc/cloudai/models/workload.pytests/test_agents.pytests/test_cloudaigym.pytests/test_handlers.pytests/test_test_scenario.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/cloudai/cli/handlers.py (1)
147-149: Agent config construction is correct and consistent.
test_run(the deep copy) is used uniformly forenv,agent_config_data, andagent; the prior inconsistency withtr.test.agent_confighas been resolved. LGTM.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cloudai/cli/handlers.py` around lines 147 - 149, The use of the deep-copied test_run for constructing env, agent_config_data, and agent is now consistent: keep agent_config_data = test_run.test.agent_config or {}, validate it with agent_class.get_config_class().model_validate(agent_config_data), and instantiate the agent with agent = agent_class(env, agent_config); no further changes required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cloudai/cli/handlers.py`:
- Around line 560-563: The agents listing appends agent.__doc__ directly which
can be None and lacks a separating space; change the verbose branch in the block
that builds string (using idx, name, agent.__name__) to safely read the
docstring (doc = agent.__doc__ or "" / use getattr(agent, "__doc__", "") ), only
append it if truthy and prefix with a space (e.g., f" {doc}"), so printing via
print(string) never yields "None" and always has a proper separator when verbose
is True.
---
Duplicate comments:
In `@src/cloudai/cli/handlers.py`:
- Around line 147-149: The use of the deep-copied test_run for constructing env,
agent_config_data, and agent is now consistent: keep agent_config_data =
test_run.test.agent_config or {}, validate it with
agent_class.get_config_class().model_validate(agent_config_data), and
instantiate the agent with agent = agent_class(env, agent_config); no further
changes required.
2f9c9c9 to
483050f
Compare
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
podkidyshev
force-pushed
the
ipod/cloudai-20/agent-config
branch
from
483050f to
e0bc356
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/cloudai/cli/handlers.py (1)
560-563:⚠️ Potential issue | 🟡 MinorAdd a separator before appending agent docstrings in verbose output.
At Line 562, docstring text is appended directly, producing concatenated output (e.g.,
class=GridSearchAgentAgent ...).✏️ Proposed fix
string = f'{idx}. "{name}" class={agent.__name__}' if verbose: - string += f"{agent.__doc__}" + string += f" {agent.__doc__}" print(string)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cloudai/cli/handlers.py` around lines 560 - 563, The verbose output concatenates the agent docstring directly onto the class portion (string built from idx, name, agent.__name__), so update the print construction (the block that builds `string` using `idx`, `name`, `agent.__name__`, `verbose`, and `agent.__doc__`) to insert a clear separator (e.g., " - " or "\n") before appending `agent.__doc__` when `verbose` is true so docstrings are not concatenated with the class text.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cloudai/cli/handlers.py`:
- Around line 147-149: The current call to
agent_class.get_config_class().model_validate(agent_config_data) can raise
ValidationError and crash the whole command; wrap that validation in a
try/except that catches pydantic's ValidationError (or whichever ValidationError
is used) and handle it the same way as the missing-agent-case: log an error
referencing the specific test_run (test_run.test.name or id) and the validation
details, mark this test_run as failed/errored, and continue the loop without
instantiating agent or raising; update the block around agent_config =
agent_class.get_config_class().model_validate(...) and agent = agent_class(env,
agent_config) to perform validation inside the try/except and only construct
agent when validation succeeds.
In `@tests/test_test_scenario.py`:
- Around line 577-613: The test name claims a deep merge but the scenario
overrides all keys; update the test to either (A) demonstrate a true deep merge
by adding an extra key to the base NCCLTestDefinition.agent_config (e.g.,
"keep_me": "value") that is not present in the scenario and assert it remains in
tdef.agent_config, or (B) rename the test function
test_agent_config_is_deep_merged_with_scenario_override to
test_agent_config_scenario_overrides_test_definition to reflect current
behavior; locate and change the test function definition
(test_agent_config_is_deep_merged_with_scenario_override) and its assertions
accordingly (tdef = test_scenario_parser._prepare_tdef(model.tests[0]) and the
final assert).
---
Duplicate comments:
In `@src/cloudai/cli/handlers.py`:
- Around line 560-563: The verbose output concatenates the agent docstring
directly onto the class portion (string built from idx, name, agent.__name__),
so update the print construction (the block that builds `string` using `idx`,
`name`, `agent.__name__`, `verbose`, and `agent.__doc__`) to insert a clear
separator (e.g., " - " or "\n") before appending `agent.__doc__` when `verbose`
is true so docstrings are not concatenated with the class text.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
doc/USER_GUIDE.rstsrc/cloudai/cli/handlers.pysrc/cloudai/configurator/base_agent.pysrc/cloudai/configurator/cloudai_gym.pysrc/cloudai/configurator/grid_search.pysrc/cloudai/core.pysrc/cloudai/models/scenario.pysrc/cloudai/models/workload.pytests/test_agents.pytests/test_cloudaigym.pytests/test_handlers.pytests/test_test_scenario.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
tests/test_handlers.py (1)
33-53: 🧹 Nitpick | 🔵 TrivialAdd
super().__init__()call to align withBaseAgentcontract.
StubAgent.__init__manually assignsself.env,self.config, andself.max_stepswithout callingsuper().__init__(env, config). While this works for the current test, it bypasses any future initialization logic inBaseAgentand creates inconsistency with production agent implementations.♻️ Proposed fix
class StubAgent(BaseAgent): + """Stub agent for testing that records received configurations.""" + received_configs: ClassVar[list[StubAgentConfig]] = [] def __init__(self, env, config: StubAgentConfig): - self.env = env - self.config = config - self.max_steps = 0 + super().__init__(env, config) StubAgent.received_configs.append(config)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_handlers.py` around lines 33 - 53, StubAgent.__init__ bypasses BaseAgent initialization; update StubAgent.__init__ to call super().__init__(env, config) before setting any stub-specific fields (retain or initialize received_configs and max_steps as needed) so the BaseAgent contract runs; ensure the method signature remains def __init__(self, env, config: StubAgentConfig) and keep appending config to StubAgent.received_configs if still required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cloudai/configurator/cloudai_gym.py`:
- Around line 53-56: The property action_first currently does {k: v[0] for k, v
in self.define_action_space().items()} which will raise IndexError for empty
lists; update action_first to validate each value from define_action_space():
ensure it is a list and non-empty, and either skip keys with empty lists or
raise a clear ValueError with the key name (e.g., "empty action space for
parameter '...') so failures are explicit; reference the action_first property
and define_action_space() when implementing the check and error handling.
In `@src/cloudai/configurator/grid_search.py`:
- Around line 31-44: GridSearchAgent.__init__ is duplicating BaseAgent
initialization by setting self.env, self.config, and self.action_space directly;
replace that with a call to super().__init__(env, config) to let
BaseAgent.__init__ initialize those attributes (and any future fields like
self.max_steps), then call self.configure(self.action_space) afterwards; remove
the manual assignments to self.env, self.config, and self.action_space so
initialization is centralized in BaseAgent.
---
Duplicate comments:
In `@tests/test_handlers.py`:
- Around line 33-53: StubAgent.__init__ bypasses BaseAgent initialization;
update StubAgent.__init__ to call super().__init__(env, config) before setting
any stub-specific fields (retain or initialize received_configs and max_steps as
needed) so the BaseAgent contract runs; ensure the method signature remains def
__init__(self, env, config: StubAgentConfig) and keep appending config to
StubAgent.received_configs if still required.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
doc/USER_GUIDE.rstsrc/cloudai/cli/handlers.pysrc/cloudai/configurator/base_agent.pysrc/cloudai/configurator/cloudai_gym.pysrc/cloudai/configurator/grid_search.pysrc/cloudai/core.pysrc/cloudai/models/scenario.pysrc/cloudai/models/workload.pytests/test_agents.pytests/test_cloudaigym.pytests/test_handlers.pytests/test_test_scenario.py
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
amaslenn
reviewed
Feb 26, 2026
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/test_handlers.py (1)
36-40:⚠️ Potential issue | 🟡 MinorMissing
super().__init__()call.
StubAgent.__init__does not callsuper().__init__(env, config)and skips initializingself.action_space = {}, resulting in a partially initialized object. While the current test works, this deviates from the parent class contract and may break ifBaseAgent.__init__changes.🔧 Proposed fix
def __init__(self, env, config: StubAgentConfig): + super().__init__(env, config) self.env = env self.config = config self.max_steps = 0 StubAgent.received_configs.append(config)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_handlers.py` around lines 36 - 40, StubAgent.__init__ currently omits calling the parent initializer, so update StubAgent.__init__ to call super().__init__(env, config) at the start (or appropriate place) so BaseAgent.__init__ runs and initializes attributes like self.action_space = {}; keep the existing assignments (self.env, self.config, self.max_steps) only if needed after the super call and ensure StubAgent.received_configs.append(config) remains but is executed after calling super to avoid a partially initialized object.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@tests/test_handlers.py`:
- Around line 36-40: StubAgent.__init__ currently omits calling the parent
initializer, so update StubAgent.__init__ to call super().__init__(env, config)
at the start (or appropriate place) so BaseAgent.__init__ runs and initializes
attributes like self.action_space = {}; keep the existing assignments (self.env,
self.config, self.max_steps) only if needed after the super call and ensure
StubAgent.received_configs.append(config) remains but is executed after calling
super to avoid a partially initialized object.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
src/cloudai/configurator/cloudai_gym.pysrc/cloudai/models/scenario.pysrc/cloudai/models/workload.pytests/test_handlers.py
amaslenn
approved these changes
Feb 27, 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
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
Implementation of FRs_Specs_CloudAI_Productionizing:CloudAI- Agent Configs
Now it is possible to configure
Refs CLOUDAI-20
Test Plan
Additional Notes