Release v0.12.0: controlled workflow proof#37
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (23)
📝 WalkthroughWalkthroughChangesWorkflow proof release
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Runner
participant Codex
participant WorkspaceEvaluator
participant Report
CLI->>Runner: provide fixtures, output directory, and model
Runner->>Codex: execute each memory arm
Codex-->>Runner: return structured response
Runner->>WorkspaceEvaluator: validate retained workspace outputs
WorkspaceEvaluator-->>Runner: return file checks
Runner->>Report: write JSON and Markdown summaries
Report-->>CLI: return completion status
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a controlled agent-workflow proof runner (v0.12.0) that compares agent performance across different memory arms (no-memory, raw-memory, and Tree Ring retrieval) using deterministic workspace validators. It adds the workflow scenario contract, workspace evaluation logic, a Codex adapter, test coverage, and three synthetic fixtures. The review feedback highlights several robust improvements: limiting the read size of workspace files to prevent OOM issues and distinguishing read failures from missing files; propagating and recording the actual context setup errors when building the Tree Ring context fails; and explicitly ignoring the generated workflow-proof-report.json in the fixture path loader to avoid parsing conflicts if the output and fixture directories overlap.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| pub fn evaluate_workspace( | ||
| scenario: &WorkflowScenario, | ||
| workspace_root: &Path, | ||
| ) -> Vec<WorkflowFileCheckReport> { | ||
| scenario | ||
| .expected_files | ||
| .iter() | ||
| .map(|expectation| { | ||
| let (exists, passed) = if let Ok(path_key) = | ||
| canonical_workflow_path("workflow file expectation path", &expectation.path) | ||
| { | ||
| match read_regular_workspace_file(workspace_root, &path_key) { | ||
| Some(content) => (true, evaluate_file_content(expectation, &content)), | ||
| None => (false, false), | ||
| } | ||
| } else { | ||
| (false, false) | ||
| }; | ||
|
|
||
| WorkflowFileCheckReport { | ||
| path: expectation.path.clone(), | ||
| contains: expectation.contains.clone(), | ||
| json_fields: expectation.json_fields.clone(), | ||
| exists, | ||
| passed, | ||
| } | ||
| }) | ||
| .collect() | ||
| } | ||
|
|
||
| fn read_regular_workspace_file(workspace_root: &Path, path_key: &str) -> Option<String> { | ||
| let mut file = open_regular_workspace_file(workspace_root, path_key)?; | ||
| let mut content = String::new(); | ||
| file.read_to_string(&mut content).ok()?; | ||
| Some(content) | ||
| } |
There was a problem hiding this comment.
The read_regular_workspace_file function currently reads the entire file into memory without any size limits, which could lead to out-of-memory (OOM) issues if an agent writes an excessively large file. Additionally, if the file exists but contains invalid UTF-8, read_to_string fails and returns None, causing evaluate_workspace to incorrectly report that the file does not exist.
We should limit the read size to a reasonable threshold (e.g., 10MB) and distinguish between a missing file and a read failure so exists is correctly reported as true.
pub fn evaluate_workspace(
scenario: &WorkflowScenario,
workspace_root: &Path,
) -> Vec<WorkflowFileCheckReport> {
scenario
.expected_files
.iter()
.map(|expectation| {
let (exists, passed) = if let Ok(path_key) =
canonical_workflow_path("workflow file expectation path", &expectation.path)
{
match read_regular_workspace_file(workspace_root, &path_key) {
Some(Ok(content)) => (true, evaluate_file_content(expectation, &content)),
Some(Err(_)) => (true, false),
None => (false, false),
}
} else {
(false, false)
};
WorkflowFileCheckReport {
path: expectation.path.clone(),
contains: expectation.contains.clone(),
json_fields: expectation.json_fields.clone(),
exists,
passed,
}
})
.collect()
}
fn read_regular_workspace_file(workspace_root: &Path, path_key: &str) -> Option<Result<String, std::io::Error>> {
let file = open_regular_workspace_file(workspace_root, path_key)?;
let mut content = String::new();
match file.take(10 * 1024 * 1024).read_to_string(&mut content) {
Ok(_) => Some(Ok(content)),
Err(error) => Some(Err(error)),
}
}| let tree_ring_trial = match tree_ring_context_builder(scenario) { | ||
| Ok(tree_ring_memory) => run_trial( | ||
| scenario, | ||
| scenario_id, | ||
| WorkflowArm::TreeRing, | ||
| tree_ring_memory, | ||
| output_dir, | ||
| agent, | ||
| )?, | ||
| Err(_) => run_tree_ring_context_error_trial(scenario, scenario_id, output_dir)?, | ||
| }; |
There was a problem hiding this comment.
The error returned by tree_ring_context_builder is currently discarded, and a generic tree_ring_context_error is used instead. Passing the actual error message to run_tree_ring_context_error_trial will greatly improve observability and debugging when context setup fails.
| let tree_ring_trial = match tree_ring_context_builder(scenario) { | |
| Ok(tree_ring_memory) => run_trial( | |
| scenario, | |
| scenario_id, | |
| WorkflowArm::TreeRing, | |
| tree_ring_memory, | |
| output_dir, | |
| agent, | |
| )?, | |
| Err(_) => run_tree_ring_context_error_trial(scenario, scenario_id, output_dir)?, | |
| }; | |
| let tree_ring_trial = match tree_ring_context_builder(scenario) { | |
| Ok(tree_ring_memory) => run_trial( | |
| scenario, | |
| scenario_id, | |
| WorkflowArm::TreeRing, | |
| tree_ring_memory, | |
| output_dir, | |
| agent, | |
| )?, | |
| Err(error) => run_tree_ring_context_error_trial(scenario, scenario_id, output_dir, error)?, | |
| }; |
| fn run_tree_ring_context_error_trial( | ||
| scenario: &WorkflowScenario, | ||
| scenario_id: &str, | ||
| output_dir: &Path, | ||
| ) -> Result<WorkflowProofTrialReport, String> { | ||
| let arm = WorkflowArm::TreeRing; | ||
| let workspace = trial_workspace(output_dir, scenario_id, &arm); | ||
| materialize_workspace(&workspace, scenario)?; | ||
| let file_checks = evaluate_workspace(scenario, &workspace); | ||
|
|
||
| Ok(WorkflowProofTrialReport { | ||
| arm, | ||
| workspace: workspace_report_path(&workspace, output_dir), | ||
| memory_context: Vec::new(), | ||
| agent_response: None, | ||
| file_checks, | ||
| status: WorkflowProofTrialStatus::Error, | ||
| errors: vec![TREE_RING_CONTEXT_ERROR.to_string()], | ||
| }) | ||
| } |
There was a problem hiding this comment.
Update run_tree_ring_context_error_trial to accept and record the actual context setup error in the trial report's errors list.
fn run_tree_ring_context_error_trial(
scenario: &WorkflowScenario,
scenario_id: &str,
output_dir: &Path,
error: String,
) -> Result<WorkflowProofTrialReport, String> {
let arm = WorkflowArm::TreeRing;
let workspace = trial_workspace(output_dir, scenario_id, &arm);
materialize_workspace(&workspace, scenario)?;
let file_checks = evaluate_workspace(scenario, &workspace);
Ok(WorkflowProofTrialReport {
arm,
workspace: workspace_report_path(&workspace, output_dir),
memory_context: Vec::new(),
agent_response: None,
file_checks,
status: WorkflowProofTrialStatus::Error,
errors: vec![TREE_RING_CONTEXT_ERROR.to_string(), error],
})
}| fn sorted_fixture_paths(fixture_dir: &Path) -> Result<Vec<PathBuf>, String> { | ||
| let entries = fs::read_dir(fixture_dir).map_err(|error| { | ||
| format!( | ||
| "fixture_directory_read_error {}: {error}", | ||
| fixture_dir.display() | ||
| ) | ||
| })?; | ||
| let mut paths = entries | ||
| .map(|entry| { | ||
| entry.map(|entry| entry.path()).map_err(|error| { | ||
| format!( | ||
| "fixture_directory_read_error {}: {error}", | ||
| fixture_dir.display() | ||
| ) | ||
| }) | ||
| }) | ||
| .collect::<Result<Vec<_>, _>>()?; | ||
| paths.retain(|path| path.is_file() && path.extension() == Some(OsStr::new("json"))); | ||
| paths.sort(); | ||
| Ok(paths) | ||
| } |
There was a problem hiding this comment.
If the user configures the output directory to be the same as the fixture directory, the generated workflow-proof-report.json will be written there. On subsequent runs, sorted_fixture_paths will attempt to parse this report as a scenario fixture, causing a parse failure. We should explicitly ignore workflow-proof-report.json when collecting fixture paths.
fn sorted_fixture_paths(fixture_dir: &Path) -> Result<Vec<PathBuf>, String> {
let entries = fs::read_dir(fixture_dir).map_err(|error| {
format!(
"fixture_directory_read_error {}: {error}",
fixture_dir.display()
)
})?;
let mut paths = entries
.map(|entry| {
entry.map(|entry| entry.path()).map_err(|error| {
format!(
"fixture_directory_read_error {}: {error}",
fixture_dir.display()
)
})
})
.collect::<Result<Vec<_>, _>>()?;
paths.retain(|path| {
path.is_file()
&& path.extension() == Some(OsStr::new("json"))
&& path.file_name() != Some(OsStr::new("workflow-proof-report.json"))
});
paths.sort();
Ok(paths)
}1 participant
Release candidate: v0.12.0
What changed
Validation
Observed controlled proof
Codex CLI 0.144.0 with
gpt-5.6-luna: Tree Ring 3/3, raw memory 3/3, no memory 2/3. Tree Ring recorded one observed win over no memory and none over raw memory. This is retained fixture evidence, not a universal performance claim.Summary by CodeRabbit
New Features
Bug Fixes
Documentation