Conversation
Reviewer's GuideRefactors runtime state into a single CorralState, replaces mutable TaskGroup with an immutable task graph and explicit InputRef-based dependencies, enforces topological execution with chain short-circuiting, and introduces a unified Toolset API while updating server/router and tests accordingly. Sequence diagram for chained execution with dependency checks and short-circuitingsequenceDiagram
participant Runner as RunBenchmark
participant Router as RouterClient
participant Server as BackendServer
participant Env as Environment
Runner->>Router: get_dependency_graph()
Router->>Server: GET /dependency_graph
Server-->>Router: {task_id: [deps]}
Router-->>Runner: graph
Runner->>Runner: assert_dependencies_selected(task_ids, graph)
Runner->>Runner: ordered_ids = order_selected(task_ids, graph)
loop trial_round over trials_per_task
loop task_id in ordered_ids
Runner->>Runner: missing = _unsatisfied_dependency(task_id, graph, task_results, trial_round)
alt missing is not None
Runner->>Runner: result = unreachable_trial_result(task_id, trial_round, missing)
Runner->>Runner: task_results[task_id].trials.append(result)
else no missing dependency
Runner->>Env: execute_single_trial(task_id, trial_round, ...)
Env->>Env: reset_state()
Env->>Env: get_task_prompt()
Env->>Env: configure_additional_apps()
Env->>Env: submit_answer(answer)
Env->>Env: score()
Env-->>Runner: TaskTrialResult
Runner->>Runner: task_results[task_id].trials.append(result)
end
end
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reached
Next review available in: 49 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR replaces the mutable TaskGroup model with immutable task definitions, explicit dependency graphs, and CorralState-backed runtime state. Environments, chained execution, server APIs, benchmark integrations, documentation, and tests are migrated to the new architecture. ChangesCore architecture
Estimated code review effort: 4 (Complex) | ~75 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
CorralState.start_new_trial, only trials in states {submitted, surrendered, scored, finalized} get archived intotrials; if a trial is started but never reaches one of these states (e.g. agent / tool crash before submit), callingreset_statewill drop that history — consider explicitly archiving any prior trial once it has beenrunningfor robustness/debuggability. - In
Toolset.resolve, missing tools from the pool are logged and silently skipped; if a task declares a tool that’s not available this is likely a configuration error rather than a soft warning, so consider raising (or optionally toggling between strict/lenient modes) to fail fast on misconfigured tool names.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `CorralState.start_new_trial`, only trials in states {submitted, surrendered, scored, finalized} get archived into `trials`; if a trial is started but never reaches one of these states (e.g. agent / tool crash before submit), calling `reset_state` will drop that history — consider explicitly archiving any prior trial once it has been `running` for robustness/debuggability.
- In `Toolset.resolve`, missing tools from the pool are logged and silently skipped; if a task declares a tool that’s not available this is likely a configuration error rather than a soft warning, so consider raising (or optionally toggling between strict/lenient modes) to fail fast on misconfigured tool names.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR refactors Corral’s runtime execution model to centralize mutable run state in a new CorralState, replaces mutable task-group runtime behavior with an explicit dependency graph API (including topological ordering and dependency-closure checks), and simplifies tool configuration via a single Toolset object.
Changes:
- Introduces
CorralState/TaskRunStateas the unified mutable runtime state and updates server/state APIs to return serialized snapshots. - Replaces implicit/chained task-group execution with explicit dependency graph utilities (
assert_dependencies_selected,order_selected,topological_order) and adds broken-chain short-circuiting in chained runs. - Consolidates tool configuration into
Toolset(pool/common/workspace tool factory) and updates environment + tests to use the new constructor/API.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_tool_processing.py | Updates environment test scaffolding to the new Environment(..., task, toolset=...) constructor and Toolset usage. |
| tests/test_task_graph.py | Adds coverage for dependency closure, ordering, and chained short-circuit behavior. |
| tests/test_configure_additional_apps.py | Updates DummyEnv construction to pass a TaskDefinition. |
| src/corral/utils/task_group.py | Removes the old mutable task-group runtime environment implementation. |
| src/corral/utils/init.py | Drops task_group exports to match removal. |
| src/corral/run.py | Enforces dependency-closed selection + topological order in chained mode; adds “unreachable” short-circuit results and dependency checks per round. |
| src/corral/router/routes.py | Adds router support for fetching the dependency graph from the server. |
| src/corral/backend/task.py | Introduces TaskDefinition.input_map + InputRef, graph utilities, and dependency-closure + ordering helpers. |
| src/corral/backend/state.py | Adds new unified mutable runtime state (CorralState) and snapshot/tool-statistics serialization. |
| src/corral/backend/server.py | Adds /dependency_graph endpoint and switches state responses to serialized snapshots. |
| src/corral/backend/env.py | Implements Toolset, rewires environment runtime state to CorralState, adds graph-derived environment building, and updates scoring/tool execution paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| missing = {tid: deps for tid, deps in missing.items() if deps} | ||
| if missing: | ||
| lines = "; ".join(f"{t} needs {d}" for t, d in missing.items()) |
| missing = _unsatisfied_dependency(task_id, graph, task_results, trial_round) | ||
| if missing is not None: | ||
| logger.warning( | ||
| f"Skipping task {task_id}: dependency {missing!r} did not " | ||
| f"produce a usable output this round (chain broken upstream)." |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/corral/backend/env.py (2)
675-680:⚠️ Potential issue | 🟠 Major | ⚡ Quick winParse string verbosity values instead of forcing
FULL.On Lines 677-680, valid string inputs like
"brief"and"detailed"are ignored and coerced toToolVerbosity.FULL. This silently breaks caller intent.💡 Suggested fix
if verbosity is None: resolved_verbosity = ToolVerbosity.DETAILED elif isinstance(verbosity, ToolVerbosity): resolved_verbosity = verbosity + elif isinstance(verbosity, str): + try: + resolved_verbosity = ToolVerbosity(verbosity.lower()) + except ValueError: + logger.warning( + f"Unknown verbosity {verbosity!r}; defaulting to FULL." + ) + resolved_verbosity = ToolVerbosity.FULL else: resolved_verbosity = ToolVerbosity.FULL🤖 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/corral/backend/env.py` around lines 675 - 680, The current branch that sets resolved_verbosity forces any non-ToolVerbosity value to ToolVerbosity.FULL and ignores valid string inputs; update the logic in the resolver (the code that assigns resolved_verbosity) to detect str inputs and map them to the enum (e.g., normalize with verbosity.lower() or .upper() and look up the corresponding ToolVerbosity member), handling invalid strings by falling back to a safe default (e.g., ToolVerbosity.FULL) or raising a clear error; specifically change the else branch so that if isinstance(verbosity, str) you convert/lookup to ToolVerbosity (with a try/except for unknown names) instead of unconditionally assigning ToolVerbosity.FULL.
456-464:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle missing hidden args without raising runtime exceptions.
On Line 462, raising
KeyErrorabortscall_tool()and skips failed-call recording. This breaks the method’s error-handling contract and can crash execution for configuration mistakes.💡 Suggested fix
if hasattr(tool, "hidden_args") and tool.hidden_args: # tool.hidden_args is a list of argument names to hide for hidden_arg in tool.hidden_args: if hidden_arg in self.state.hidden_args: call_args[hidden_arg] = self.state.hidden_args[hidden_arg] else: - raise KeyError( - f"Hidden argument '{hidden_arg}' required by tool '{tool_name}' not found in environment's hidden_args." - ) + duration = time.perf_counter() - start_time + tool_call = ToolCall( + tool_name=tool_name, + arguments=original_arguments, + result=None, + status=ToolCallStatus.INVALID_ARGS, + error_message=( + f"Hidden argument '{hidden_arg}' required by tool " + f"'{tool_name}' not found in environment hidden_args." + ), + duration=duration, + ) + self.state.record_tool_call(tool_call) + return tool_call🤖 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/corral/backend/env.py` around lines 456 - 464, The code currently raises a KeyError when a required hidden arg is missing which aborts call_tool() and prevents the normal failed-call recording; instead, do not raise—construct a clear error message, call the existing failure-reporting path used by call_tool() (e.g., invoke the method that records failed tool calls such as record_failed_call or the call_tool() error-return path) with that message, log a warning via the environment logger, and return an error result (for example {'success': False, 'error': error_msg}) so execution continues and the failure is recorded; update the block handling tool.hidden_args (the loop over hidden_arg, self.state.hidden_args and call_args) to implement this non-throwing failure flow.
🤖 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/corral/router/routes.py`:
- Around line 69-73: get_dependency_graph currently calls requests.get without a
timeout and can hang; update the requests.get call in get_dependency_graph to
pass a timeout (e.g. timeout=10) so the HTTP request fails fast on network
stalls. If you prefer configurability, add an instance attribute like
self.http_timeout (set in __init__) and use
requests.get(f"{self.base_url}/dependency_graph", timeout=self.http_timeout)
instead; keep the response.raise_for_status() and return response.json() as-is.
---
Outside diff comments:
In `@src/corral/backend/env.py`:
- Around line 675-680: The current branch that sets resolved_verbosity forces
any non-ToolVerbosity value to ToolVerbosity.FULL and ignores valid string
inputs; update the logic in the resolver (the code that assigns
resolved_verbosity) to detect str inputs and map them to the enum (e.g.,
normalize with verbosity.lower() or .upper() and look up the corresponding
ToolVerbosity member), handling invalid strings by falling back to a safe
default (e.g., ToolVerbosity.FULL) or raising a clear error; specifically change
the else branch so that if isinstance(verbosity, str) you convert/lookup to
ToolVerbosity (with a try/except for unknown names) instead of unconditionally
assigning ToolVerbosity.FULL.
- Around line 456-464: The code currently raises a KeyError when a required
hidden arg is missing which aborts call_tool() and prevents the normal
failed-call recording; instead, do not raise—construct a clear error message,
call the existing failure-reporting path used by call_tool() (e.g., invoke the
method that records failed tool calls such as record_failed_call or the
call_tool() error-return path) with that message, log a warning via the
environment logger, and return an error result (for example {'success': False,
'error': error_msg}) so execution continues and the failure is recorded; update
the block handling tool.hidden_args (the loop over hidden_arg,
self.state.hidden_args and call_args) to implement this non-throwing failure
flow.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 26e9d569-d2d2-4b6e-93a0-1a5200dc4c33
📒 Files selected for processing (11)
src/corral/backend/env.pysrc/corral/backend/server.pysrc/corral/backend/state.pysrc/corral/backend/task.pysrc/corral/router/routes.pysrc/corral/run.pysrc/corral/utils/__init__.pysrc/corral/utils/task_group.pytests/test_configure_additional_apps.pytests/test_task_graph.pytests/test_tool_processing.py
💤 Files with no reviewable changes (2)
- src/corral/utils/task_group.py
- src/corral/utils/init.py
| def get_dependency_graph(self) -> dict[str, list[str]]: | ||
| """Fetch the task dependency graph as a ``{task_id: [deps]}`` dict.""" | ||
| response = requests.get(f"{self.base_url}/dependency_graph") | ||
| response.raise_for_status() | ||
| return response.json() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify timeout usage for dependency graph call and nearby router GET requests.
rg -n -C2 'def get_dependency_graph|requests\.get\(' src/corral/router/routes.pyRepository: lamalab-org/corral
Length of output: 2684
🏁 Script executed:
#!/bin/bash
# Check whether any requests in this file already use timeouts.
rg -n "timeout\s*=" src/corral/router/routes.pyRepository: lamalab-org/corral
Length of output: 144
🏁 Script executed:
#!/bin/bash
# Inspect around the one timeout usage and locate how `timeout` is defined/passed.
sed -n '140,220p' src/corral/router/routes.pyRepository: lamalab-org/corral
Length of output: 3115
🏁 Script executed:
#!/bin/bash
# Find all requests.get calls that already include timeout= in this file.
rg -n "requests\.get\([^)]*timeout\s*=" src/corral/router/routes.pyRepository: lamalab-org/corral
Length of output: 44
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find call sites for get_dependency_graph
rg -n "get_dependency_graph\(" -S . --glob '!**/node_modules/**' --glob '!**/.venv/**'Repository: lamalab-org/corral
Length of output: 307
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find other usage of dependency graph endpoint string
rg -n "/dependency_graph" -S srcRepository: lamalab-org/corral
Length of output: 228
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find how dependency chaining is handled (control path hints)
rg -n "dependency_chain|get_dependency_graph" src/corral -SRepository: lamalab-org/corral
Length of output: 976
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find call sites for get_dependency_graph
rg -n "get_dependency_graph\(" -S . --glob '!**/node_modules/**' --glob '!**/.venv/**'Repository: lamalab-org/corral
Length of output: 307
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find other usage of dependency graph endpoint string
rg -n "/dependency_graph" -S srcRepository: lamalab-org/corral
Length of output: 228
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find how dependency chaining is handled (control path hints)
rg -n "dependency_chain|get_dependency_graph" src/corral -SRepository: lamalab-org/corral
Length of output: 976
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '760,830p' src/corral/run.pyRepository: lamalab-org/corral
Length of output: 2814
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,140p' src/corral/router/routes.pyRepository: lamalab-org/corral
Length of output: 5588
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "requests\.Session|timeout\s*[:=]" src/corral/router/routes.pyRepository: lamalab-org/corral
Length of output: 280
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "\bget_dependency_graph\(" -S src/corral/run.pyRepository: lamalab-org/corral
Length of output: 128
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '760,830p' src/corral/run.pyRepository: lamalab-org/corral
Length of output: 2814
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,140p' src/corral/router/routes.pyRepository: lamalab-org/corral
Length of output: 5588
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "requests\.Session|timeout\s*[:=]" src/corral/router/routes.pyRepository: lamalab-org/corral
Length of output: 280
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "\bget_dependency_graph\(" -S src/corral/run.pyRepository: lamalab-org/corral
Length of output: 128
Add an HTTP timeout to dependency-graph fetch
In src/corral/router/routes.py, get_dependency_graph() calls requests.get(.../dependency_graph) without a timeout; src/corral/run.py uses this on the dependency-chained execution path before run_chained_trials, so a stalled connection can block the run indefinitely.
💡 Suggested fix
def get_dependency_graph(self) -> dict[str, list[str]]:
"""Fetch the task dependency graph as a ``{task_id: [deps]}`` dict."""
- response = requests.get(f"{self.base_url}/dependency_graph")
+ response = requests.get(f"{self.base_url}/dependency_graph", timeout=30.0)
response.raise_for_status()
return response.json()🧰 Tools
🪛 ast-grep (0.43.0)
[info] 70-70: no timeout was given on call to external resource
Context: requests.get(f"{self.base_url}/dependency_graph")
Note: [CWE-1088].
(requests-timeout)
🪛 Ruff (0.15.15)
[error] 71-71: Probable use of requests call without timeout
(S113)
🤖 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/corral/router/routes.py` around lines 69 - 73, get_dependency_graph
currently calls requests.get without a timeout and can hang; update the
requests.get call in get_dependency_graph to pass a timeout (e.g. timeout=10) so
the HTTP request fails fast on network stalls. If you prefer configurability,
add an instance attribute like self.http_timeout (set in __init__) and use
requests.get(f"{self.base_url}/dependency_graph", timeout=self.http_timeout)
instead; keep the response.raise_for_status() and return response.json() as-is.
Source: Linters/SAST tools
|
Sorry for the delay. I just now reached this part of my inbox and will try to look at it tomorrow. |
Preserve functionality from both branches: - main: new black-box agents (Codex, Claude Code, Terminus, OpenHands), task-scoped MCP server, AgentRunResult return type, cumulative token usage. - new_state: reworked backend Environment/task-graph (state.py, task.py), bench_from_traces / _run_benchmark orchestration in run.py. Conflict resolution: - run.py, routes.py, reflexion_agent.py, base_agent.py: docstring-only conflicts (backtick formatting) — kept the backticked wording; code merged automatically (execute_single_trial consumes AgentRunResult). - uv.lock: regenerated from the merged pyproject via `uv lock`. - tests/backend/test_mcp_server.py: adapted DummyEnv to new_state's Environment(task_id, task, ...) signature (added required TaskDefinition), so main's MCP tests run against the merged API. Verified: 426 passed. Remaining test_metrics_registry parallel failures are a pre-existing local-venv multiprocessing-spawn artifact (byte-identical to both parents, unrelated to the merge). Agent SDK tests (codex/claude_code/ openhands) require optional extras not installed here. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tasks/wetlab/wetlab/env.py (1)
299-331: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winThread a work directory through WetLab env creation
create_qualysis_environments()builds every environment with the default emptybase_work_dir, so all tasks end up reading and writing the samecompositions.pklin the current directory. Pass a per-runwork_dirintobuild_environments(...)here, like the other benchmark entrypoints.🤖 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 `@tasks/wetlab/wetlab/env.py` around lines 299 - 331, Update create_qualysis_environments to accept a per-run work_dir parameter and pass it through to build_environments, ensuring each WetLab run uses its own directory for compositions.pkl instead of the default current directory.
🧹 Nitpick comments (1)
src/corral/router/routes.py (1)
86-101: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd HTTP timeout to
get_mcp_tool_schema.The new
get_mcp_tool_schemamethod callsrequests.get(...)without a timeout, consistent with the pre-existing pattern in this file but introducing the same hang risk. Consider adding a timeout here (and consolidating across the file via a sharedself.http_timeoutattribute or arequests.Sessionwith default timeouts).♻️ Proposed fix
def get_mcp_tool_schema( self, task_id: str, verbosity: str | None = None ) -> dict[str, Any]: """Get the task's tools in MCP representation plus their schema digest.""" verbosity = verbosity or self.current_verbosity params = {"verbosity": verbosity} response = requests.get( - f"{self.base_url}/tasks/{task_id}/tools/mcp", params=params + f"{self.base_url}/tasks/{task_id}/tools/mcp", params=params, timeout=30.0 ) response.raise_for_status() return response.json()🤖 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/corral/router/routes.py` around lines 86 - 101, Update get_mcp_tool_schema to pass the established HTTP timeout when calling requests.get, reusing a shared self.http_timeout or the file’s existing timeout configuration rather than leaving the request unbounded. Preserve the current URL, parameters, response validation, and JSON return behavior.Source: Linters/SAST tools
🤖 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 `@docs/index.md`:
- Around line 136-141: Update the build_environments example to pass my_tools
using the toolset keyword instead of available_tools, matching the parameter
accepted by Environment.__init__.
- Around line 143-144: Update the documentation example around task_input so
resolve_inputs is called only after the upstream analyze_structure task
completes; alternatively, demonstrate it using already-resolved shared state. Do
not imply that build_environments(...) populates retrieve_data before task
execution.
In `@tasks/catalyst/src/catalyst/env.py`:
- Around line 103-104: Update the environment-building function containing the
name parameter so its default is unique per configuration rather than the fixed
"catalyst" label. Derive the label from the available task_type and level
values, or make name required and update callers accordingly, ensuring trace and
LaTeX metadata remains distinguishable.
---
Outside diff comments:
In `@tasks/wetlab/wetlab/env.py`:
- Around line 299-331: Update create_qualysis_environments to accept a per-run
work_dir parameter and pass it through to build_environments, ensuring each
WetLab run uses its own directory for compositions.pkl instead of the default
current directory.
---
Nitpick comments:
In `@src/corral/router/routes.py`:
- Around line 86-101: Update get_mcp_tool_schema to pass the established HTTP
timeout when calling requests.get, reusing a shared self.http_timeout or the
file’s existing timeout configuration rather than leaving the request unbounded.
Preserve the current URL, parameters, response validation, and JSON return
behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 13c8411f-93b8-41d6-943f-10398cb0d25a
⛔ Files ignored due to path filters (1)
tasks/catalyst/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
docs/documentation/explanation.mddocs/documentation/how_tos/task_chaining.mddocs/documentation/reference.mddocs/index.mdsrc/corral/agents/reflexion_agent.pysrc/corral/backend/env.pysrc/corral/backend/server.pysrc/corral/backend/state.pysrc/corral/backend/task.pysrc/corral/router/routes.pysrc/corral/run.pytasks/afm/src/env.pytasks/catalyst/src/catalyst/env.pytasks/catalyst/tests/test_environment_integration.pytasks/corral_md/src/corral_md/env.pytasks/ml/src/ml/env.pytasks/resistor_network/src/resistor_network/env.pytasks/retrosynthesis/retrosynthesis/env.pytasks/samplemath/README.mdtasks/samplemath/samplemath/env.pytasks/samplemath/samplemath/env_subtask.pytasks/spectra_elucidation/spectra_elucidation/env.pytasks/wetlab/wetlab/env.pytests/backend/test_mcp_server.pytests/test_task_graph.py
💤 Files with no reviewable changes (1)
- tasks/samplemath/samplemath/env.py
🚧 Files skipped from review as they are similar to previous changes (5)
- src/corral/run.py
- tests/test_task_graph.py
- src/corral/backend/state.py
- src/corral/backend/task.py
- src/corral/backend/env.py
| environments = build_environments( | ||
| {"retrieve_data": task1, "analyze_structure": task2}, | ||
| base_work_dir="workdir", | ||
| name="molecular_workflow", | ||
| available_tools=my_tools, | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP 'def build_environments' -A 30 src/corral/backend/env.pyRepository: lamalab-org/corral
Length of output: 1762
🏁 Script executed:
#!/bin/bash
sed -n '128,150p' docs/index.mdRepository: lamalab-org/corral
Length of output: 985
🏁 Script executed:
#!/bin/bash
sed -n '835,930p' src/corral/backend/env.pyRepository: lamalab-org/corral
Length of output: 1338
🏁 Script executed:
#!/bin/bash
rg -nP 'class Environment|def __init__' src/corral/backend/env.py -A 40Repository: lamalab-org/corral
Length of output: 2831
Use toolset= here, not available_tools=. build_environments(..., **env_kwargs) forwards extra keywords to Environment.__init__, which only accepts toolset; this example will fail if copied as written.
🤖 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 `@docs/index.md` around lines 136 - 141, Update the build_environments example
to pass my_tools using the toolset keyword instead of available_tools, matching
the parameter accepted by Environment.__init__.
| # Access input from previous task (resolved from the shared state) | ||
| task_input = environments["analyze_structure"].state.resolve_inputs(task2) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '120,170p' docs/index.md && printf '\n---\n' && rg -n "def resolve_inputs|resolve_inputs\(" -S .Repository: lamalab-org/corral
Length of output: 2641
🏁 Script executed:
sed -n '150,240p' src/corral/backend/state.py && printf '\n---\n' && sed -n '200,270p' src/corral/backend/env.py && printf '\n---\n' && sed -n '135,150p' docs/index.mdRepository: lamalab-org/corral
Length of output: 7192
🏁 Script executed:
rg -n "def build_environments|build_environments\(" src docs -S && printf '\n---\n' && sed -n '1,220p' src/corral/backend/env.pyRepository: lamalab-org/corral
Length of output: 9520
🏁 Script executed:
sed -n '805,940p' src/corral/backend/env.py && printf '\n---\n' && sed -n '1,220p' src/corral/backend/task.pyRepository: lamalab-org/corral
Length of output: 10253
Don’t call resolve_inputs before the upstream task has run. build_environments(...) only shares state; it does not populate retrieve_data’s output, so this example will raise RuntimeError as written. Move it after the first task completes or show a resolved state instead.
🤖 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 `@docs/index.md` around lines 143 - 144, Update the documentation example
around task_input so resolve_inputs is called only after the upstream
analyze_structure task completes; alternatively, demonstrate it using
already-resolved shared state. Do not imply that build_environments(...)
populates retrieve_data before task execution.
| name: str = "catalyst", | ||
| ) -> dict[str, Environment]: |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate relevant files and inspect the catalyst env definition plus downstream consumers.
git ls-files | rg 'tasks/catalyst/src/catalyst/(env\.py|.*\.py)$|build_environments|dependency|graph|trace|latex' || true
printf '\n--- env.py outline ---\n'
ast-grep outline tasks/catalyst/src/catalyst/env.py --view expanded || true
printf '\n--- env.py excerpt ---\n'
nl -ba tasks/catalyst/src/catalyst/env.py | sed -n '1,220p'
printf '\n--- search for name usage in catalyst task ---\n'
rg -n "\bname\b" tasks/catalyst/src/catalyst -g '*.py' || true
printf '\n--- search for build_environments symbol across repo ---\n'
rg -n "def build_environments|build_environments\(" . -g '*.py' || trueRepository: lamalab-org/corral
Length of output: 1518
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- env.py create_environments section ---\n'
sed -n '95,170p' tasks/catalyst/src/catalyst/env.py
printf '\n--- env.py remainder around parser/main ---\n'
sed -n '170,240p' tasks/catalyst/src/catalyst/env.py
printf '\n--- name usage within catalyst package ---\n'
rg -n "\bname\b" tasks/catalyst/src/catalyst -g '*.py' || true
printf '\n--- create_environments call sites ---\n'
rg -n "create_environments\(" . -g '*.py' || trueRepository: lamalab-org/corral
Length of output: 8528
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- build_environments definition search ---\n'
rg -n "def build_environments|class .*build_environments|build_environments\(" . -g '*.py' || true
printf '\n--- task graph / trace / latex name usage search ---\n'
rg -n "\bname\b|base_name|label|trace|latex" src tasks -g '*.py' | sed -n '1,220p' || trueRepository: lamalab-org/corral
Length of output: 21749
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- backend env.py build_environments section ---\n'
sed -n '760,940p' src/corral/backend/env.py
printf '\n--- backend task.py ---\n'
sed -n '1,220p' src/corral/backend/task.py
printf '\n--- search for environment name propagation ---\n'
rg -n "\bname\b|base_name|benchmark|trace|latex|group" src/corral/backend -g '*.py' || trueRepository: lamalab-org/corral
Length of output: 20920
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- env.py latex section ---\n'
sed -n '640,790p' src/corral/backend/env.py
printf '\n--- task_group_id search ---\n'
rg -n "\btask_group_id\b" . -g '*.py' || trueRepository: lamalab-org/corral
Length of output: 6586
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- component_id search ---\n'
rg -n "\bcomponent_id\b" . -g '*.py' || true
printf '\n--- task_group_id search ---\n'
rg -n "\btask_group_id\b" . -g '*.py' || trueRepository: lamalab-org/corral
Length of output: 870
name should default to a per-config label
name="catalyst" makes trace/LaTeX metadata ambiguous when multiple catalyst configs are created in one run. Derive a default from task_type/level, or require callers to pass one.
🤖 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 `@tasks/catalyst/src/catalyst/env.py` around lines 103 - 104, Update the
environment-building function containing the name parameter so its default is
unique per configuration rather than the fixed "catalyst" label. Derive the
label from the available task_type and level values, or make name required and
update callers accordingly, ensuring trace and LaTeX metadata remains
distinguishable.
Core changes
The changes in this PR only include the
srccode. When the PR gets accepted I will push thetasksupdate. This is to keep the PR reviewable and focused on the core refactor, which is already quite large.Runtime state (CorralState)
src/corral/backend/state.pywith a singleCorralStatedataclass as the one mutable runtime object per runEnvironment(trial_states,trial_counter, workspace) andTaskGroup(results,scores) — all now live inCorralStateEnvironment, tools, scoring, and the server all read/write throughCorralState; task definitions remain immutable configTask graph & dependency handling
TaskGroupas a mutable runtime object; replace with an immutableTaskGraph/ helper functions (topological_order,validate_task_graph,ready_tasks)InputRef+input_maponTaskDefinitionfor explicit, named dependency wiring (replaces the magicresult_from_<task_id>key convention)CorralStateresolves dependency inputs viaresolve_inputs(task)and stores per-task outputs intask_runs[task_id]Execution order & correctness
assert_dependencies_selected— raises a clear error up front if a selected subset omits a required dependencyget_task_prompt()call at env construction; prompts are now only resolved after dependencies are satisfied, makingNOT YET AVAILABLEplaceholders in prompts unnecessary and removedscore=0.0 / unreachablewith 0 iterations — no agent call, no exceptionTool API cleanup
available_tools,common_tools,file_tools,file_tool_factory) with a singleToolsetvalue object onEnvironment.__init__andbuild_environmentsToolset.resolve(task, workspace)collapses_add_task_tools+_setup_file_toolsinto one call per trial resetexcluded_toolsto baseTaskDefinition; wetlab's_add_task_toolsoverride is deletedOther cleanups
TaskDefinitionwith optionalinput_map);group_idis no longer a public argumentprompt_fn/setup_fnhooks onTaskDefinitionreplace per-benchmarkEnvironmentsubclasses for custom prompts and hardware setupSummary by Sourcery
Unify runtime task state management around a new CorralState, replace mutable task groups with an immutable task graph and dependency API, and simplify tool configuration into a single Toolset while updating routing, server, and tests accordingly.
Enhancements:
Tests:
Summary by CodeRabbit
Release Notes
New Features
Refactor
Bug Fixes
Documentation / Tests