From 7fbd4802b841b14fa98b4b0d6be96dd8d876950e Mon Sep 17 00:00:00 2001 From: Ayoub Date: Wed, 9 Sep 2026 12:55:30 +0800 Subject: [PATCH 1/3] test(agent): add regression tests for log-path-does-not-block-execution Verify that AgentEngineFlow.run_step() executes the tool even when the step log path is unusable (e.g. a directory), and that a failing tool still becomes Incomplete without leaving any step stuck in Ongoing. --- test/engine/test_state_machine_regression.py | 108 ++++++++++++++++++- 1 file changed, 107 insertions(+), 1 deletion(-) diff --git a/test/engine/test_state_machine_regression.py b/test/engine/test_state_machine_regression.py index c878b2af..1639ce70 100644 --- a/test/engine/test_state_machine_regression.py +++ b/test/engine/test_state_machine_regression.py @@ -10,11 +10,20 @@ import os from pathlib import Path from types import SimpleNamespace +from unittest.mock import MagicMock import pytest from chipcompiler import tools -from chipcompiler.data import EccOutput, EccStep, OriginDesign, StateEnum, StepEnum, Workspace +from chipcompiler.data import ( + EccOutput, + EccStep, + LogPaths, + OriginDesign, + StateEnum, + StepEnum, + Workspace, +) from chipcompiler.data.workspace import Flow from chipcompiler.engine.flow import _VALID_TRANSITIONS, EngineFlow from chipcompiler.tools.ecc.runner import EccDesignReadError @@ -489,6 +498,103 @@ def test_agent_incomplete_step_normalized_on_resume(self, tmp_path, monkeypatch) assert persisted["steps"][1]["state"] == StateEnum.Success.value +def test_agent_flow_unusable_log_path_does_not_block_execution(tmp_path, monkeypatch): + """AgentEngineFlow: unusable step-log path must not block tool execution.""" + import agent.engine as agent_engine + + step_dir = tmp_path / "Floorplan_ecc" + step_dir.mkdir() + + workspace = Workspace(directory=tmp_path, flow=Flow(path=tmp_path / "flow.json")) + workspace.flow.data = { + "steps": [ + { + "name": "Floorplan", + "tool": "ecc", + "state": StateEnum.Unstart.value, + "runtime": "", + "peak memory (mb)": 0, + "info": {}, + } + ] + } + workspace.logger = Logger() + + agent_flow = agent_engine.AgentEngineFlow.__new__(agent_engine.AgentEngineFlow) + agent_flow.workspace = workspace + agent_flow.workspace_steps = [ + EccStep( + name="Floorplan", + tool="ecc", + directory=step_dir, + output=EccOutput(verilog=step_dir / "design.v"), + log=LogPaths(file=tmp_path), # directory — open() raises IsADirectoryError + ) + ] + agent_flow.engine_db = SimpleNamespace(engine=None) + + mock_run = MagicMock(return_value=True) + monkeypatch.setattr(agent_engine, "run_agent_step", mock_run) + monkeypatch.setattr(agent_flow, "check_step_result", lambda **_kw: True) + + result = agent_flow.run_step(agent_flow.workspace_steps[0], rerun=False) + + assert mock_run.call_count == 1 + assert result == StateEnum.Success + + persisted = json.loads((tmp_path / "flow.json").read_text()) + assert persisted["steps"][0]["state"] != StateEnum.Ongoing.value + + +def test_agent_flow_step_failure_not_silently_swallowed(tmp_path, monkeypatch): + """AgentEngineFlow: run_agent_step() failure must not be swallowed.""" + import agent.engine as agent_engine + + log_file = tmp_path / "agent_step.log" + step_dir = tmp_path / "Floorplan_ecc" + step_dir.mkdir() + + workspace = Workspace(directory=tmp_path, flow=Flow(path=tmp_path / "flow.json")) + workspace.flow.data = { + "steps": [ + { + "name": "Floorplan", + "tool": "ecc", + "state": StateEnum.Unstart.value, + "runtime": "", + "peak memory (mb)": 0, + "info": {}, + } + ] + } + workspace.logger = Logger() + + agent_flow = agent_engine.AgentEngineFlow.__new__(agent_engine.AgentEngineFlow) + agent_flow.workspace = workspace + agent_flow.workspace_steps = [ + EccStep( + name="Floorplan", + tool="ecc", + directory=step_dir, + output=EccOutput(verilog=step_dir / "design.v"), + log=LogPaths(file=log_file), + ) + ] + agent_flow.engine_db = SimpleNamespace(engine=None) + + def _raise(**_kw): + raise RuntimeError("tool crashed") + + monkeypatch.setattr(agent_engine, "run_agent_step", _raise) + + result = agent_flow.run_step(agent_flow.workspace_steps[0], rerun=False) + + assert result == StateEnum.Imcomplete + + persisted = json.loads((tmp_path / "flow.json").read_text()) + assert persisted["steps"][0]["state"] != StateEnum.Ongoing.value + + class TestRunStepsLedgerCompleteness: """run_steps verifies full-ledger coverage by default; callers binding execution to a reconciled range narrower than the persisted ledger opt From 2380fe15f1b5ab343d4edddec975cee571cb3e26 Mon Sep 17 00:00:00 2001 From: Ayoub Date: Fri, 11 Sep 2026 13:34:55 +0800 Subject: [PATCH 2/3] fix(cli): resume existing runs through rerun.run_resume --- chipcompiler/cli/project/run_existing.py | 48 ++++---------- test/cli/commands/conftest.py | 5 +- test/cli/commands/test_flow_continuation.py | 70 ++++++++++++++++++++- 3 files changed, 85 insertions(+), 38 deletions(-) diff --git a/chipcompiler/cli/project/run_existing.py b/chipcompiler/cli/project/run_existing.py index afeda0b3..56b0867c 100644 --- a/chipcompiler/cli/project/run_existing.py +++ b/chipcompiler/cli/project/run_existing.py @@ -8,13 +8,11 @@ wiring belongs next to the ledger it owns. """ -import sys from pathlib import Path from chipcompiler.cli.core.output import disclosure_cmd from chipcompiler.cli.core.types import CommandResult from chipcompiler.cli.project.run_prepare import _write_back_status -from chipcompiler.data import is_finished_step_state def run_existing_workspace( @@ -187,7 +185,9 @@ def mismatch_error(reason: str) -> CommandResult: ] ) + from chipcompiler.cli.rendering.progress import preserve_cli_stdio from chipcompiler.engine import EngineFlow + from chipcompiler.engine.rerun import bounded_resume_names, run_resume, selected_step_names try: engine_flow = EngineFlow(workspace=workspace) @@ -195,41 +195,19 @@ def mismatch_error(reason: str) -> CommandResult: if result.outcome != "no_op": # Re-read the ledger: reconcile may have appended suffix steps # after load_workspace populated the in-memory copy. - from chipcompiler.utility import json_read + engine_flow.load() - flow_data = json_read(workspace.flow.path or Path(run_dir) / "home" / "flow.json") - target_names = set(result.target) - executable = { - step["name"] - for step in flow_data.get("steps", []) - if isinstance(step, dict) - and isinstance(step.get("name"), str) - and not is_finished_step_state(step.get("state")) - and step["name"] in target_names - } - engine_flow.create_step_workspaces(executable_steps=executable) - # executable_steps only gates dependency verification; the - # actual runner iterates every workspace step. Bind execution - # to the reconciled target so a wider persisted ledger (e.g. - # RCX/sta beyond the requested end) never runs on resume. - engine_flow.workspace_steps = [ - step - for step in getattr(engine_flow, "workspace_steps", None) or [] - if step.name in target_names - ] - - from chipcompiler.cli.rendering.progress import ( - run_flow_with_progress, - should_enable_run_progress, - ) - - if should_enable_run_progress(ctx, sys.stderr): - flow_ok = run_flow_with_progress(engine_flow, ctx, project, sys.stderr) + through = result.target[-1] if result.target else None + if through is not None: + selected = bounded_resume_names(engine_flow, through) else: - # The persisted ledger may be wider than the reconciled - # target by design (workspace_steps is bound above), so - # the full-ledger completeness check does not apply. - flow_ok = engine_flow.run_steps(require_full_ledger=False) + selected = selected_step_names(engine_flow) + if selected: + engine_flow.create_step_workspaces(executable_steps=set(selected)) + + with preserve_cli_stdio(): + run_result = run_resume(engine_flow, through=through) + flow_ok = run_result.ok except Exception as exc: if workspace_registered: _write_back_status(project_dir, run_name, "failed", warnings) diff --git a/test/cli/commands/conftest.py b/test/cli/commands/conftest.py index 10cd9702..a986567d 100644 --- a/test/cli/commands/conftest.py +++ b/test/cli/commands/conftest.py @@ -27,7 +27,10 @@ def has_init(self): def add_step(self, step, tool, state): self.added_steps.append((step, tool, state)) - def create_step_workspaces(self): + def load(self): + return True + + def create_step_workspaces(self, **_kwargs): self.create_called = True def run_steps(self, **_kwargs): diff --git a/test/cli/commands/test_flow_continuation.py b/test/cli/commands/test_flow_continuation.py index ea451480..5cb40891 100644 --- a/test/cli/commands/test_flow_continuation.py +++ b/test/cli/commands/test_flow_continuation.py @@ -95,6 +95,68 @@ def create_step_workspaces(self, *, executable_steps=None): assert records[0]["status"] == "success" assert records[0]["no_op"] == "True" + def test_stale_suffix_reexecuted_via_run_resume( + self, + tmp_path, + capsys, + create_cli_project, + minimal_ics55_pdk_factory, + monkeypatch, + plain_records, + ): + """Regression: place=Incomplete + CTS=Success must call run_resume + (not run_steps), so the stale suffix is re-executed.""" + pdk_root = minimal_ics55_pdk_factory(tmp_path / "ics55") + project_dir = create_cli_project(pdk_root=pdk_root) + monkeypatch.setattr( + "chipcompiler.cli.project.config._validate_pdk_contents", + lambda name, root, overrides=None: None, + ) + run_dir = os.path.join(project_dir, "default") + _write_existing_workspace( + run_dir, + RTL2GDS_NAMES, + states=( + ["Success", "Success", "Success", "Incomplete", "Success"] + + ["Unstart"] * (len(RTL2GDS_NAMES) - 5) + ), + pdk_root=pdk_root, + ) + + from chipcompiler.engine.rerun import StepRunResult + + resume_calls = [] + + def spy_run_resume(flow, *, through=None): + resume_calls.append(through) + return StepRunResult(ok=True, executed=()) + + class Flow: + def __init__(self, workspace): + self.workspace = workspace + + def create_step_workspaces(self, *, executable_steps=None): + return None + + def load(self): + from chipcompiler.utility import json_read + + path = self.workspace.flow.path + if path: + self.workspace.flow.data = json_read(path) + return bool(self.workspace.flow.data.get("steps", [])) + + monkeypatch.setattr("chipcompiler.engine.EngineFlow", Flow) + monkeypatch.setattr("chipcompiler.engine.rerun.run_resume", spy_run_resume) + + rc = cli_main.run(["run", "--project", project_dir, "--plain"]) + + assert rc == 0 + assert len(resume_calls) == 1 + assert resume_calls[0] == RTL2GDS_NAMES[-1] + records = _records(capsys, plain_records) + assert records[0]["status"] == "success" + def test_set_rejected_on_existing_run( self, tmp_path, @@ -411,10 +473,14 @@ def __init__(self, workspace): def create_step_workspaces(self, *, executable_steps=None): return None - def run_steps(self, **_kwargs): - raise RuntimeError("engine exploded") + def load(self): + return True + + def fake_run_resume(*_args, **_kwargs): + raise RuntimeError("engine exploded") monkeypatch.setattr("chipcompiler.engine.EngineFlow", Flow) + monkeypatch.setattr("chipcompiler.engine.rerun.run_resume", fake_run_resume) rc = cli_main.run(["run", "--project", project_dir, "--plain"]) From a775ca14306802468346f05941ddb4f9be4ebbd9 Mon Sep 17 00:00:00 2001 From: Ayoub Date: Fri, 11 Sep 2026 15:51:14 +0800 Subject: [PATCH 3/3] fix: fixing test --- .gitignore | 3 --- 1 file changed, 3 deletions(-) diff --git a/.gitignore b/.gitignore index 9f761a35..c80938d0 100644 --- a/.gitignore +++ b/.gitignore @@ -192,6 +192,3 @@ task_plan.md .ccache .trellis - -# Stray runtime artifact from cwd-polluted test runs -/checklist.json