From a4901fa24a1d95f1f9d8825bb248b16922077cdc Mon Sep 17 00:00:00 2001 From: Ed_ Date: Sat, 27 Jun 2026 11:54:09 -0400 Subject: [PATCH] fix(post_de_cruft_iter4): fix 3 new failures revealed by full batched run 1. tier-1-unit-core::test_app_controller_warmup_done_ts_none_until_completed - Race condition: warmup_done_ts was set before the test could read it (warmup runs in a background thread that can complete in milliseconds). - Fix: use defer_warmup=True + call start_warmup() explicitly so we can observe the initial state before warmup begins. 2. tier-1-unit-core::test_fetch_models_aggregates_per_provider_errors - Race condition: _fetch_models submits do_fetch to the IO pool; the test asserted _model_fetch_errors synchronously before the worker ran. - Fix: call wait_io_pool_idle() before asserting the side effect. - Test passes in isolation but fails when run as part of the full file (IO pool is hot from prior tests). 3. tier-3-live_gui::test_context_sim_live - Production bug: _do_generate mutated the frozen ProjectContext dataclass returned by flat_config (flat['files'] = ...). flat_config was converted from dict[str, Any] to ProjectContext dataclass by cruft_elimination_20260627 Phase 2 but the consumer code wasn't updated. - Fix: call flat.to_dict() to get a mutable dict before mutation. - Same bug existed in /api/project endpoint (returns the ProjectContext directly; json.dumps fails silently on dataclass), now also calls to_dict() at the wire boundary. --- src/api_hooks.py | 6 +++++- src/app_controller.py | 3 +++ tests/test_app_controller_result.py | 3 +++ tests/test_warmup_canaries.py | 5 ++++- 4 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/api_hooks.py b/src/api_hooks.py index a895e02e..ccda56d5 100644 --- a/src/api_hooks.py +++ b/src/api_hooks.py @@ -211,8 +211,12 @@ class HookHandler(BaseHTTPRequestHandler): self.send_response(200) self.send_header("Content-Type", "application/json") self.end_headers() + # flat_config returns a ProjectContext dataclass; json.dumps cannot serialize + # dataclasses directly, so call .to_dict() first. The endpoint is the wire + # boundary; consumers expect the legacy dict[str, Any] shape. flat = project_manager.flat_config(_get_app_attr(app, "project")) - self.wfile.write(json.dumps({"project": flat}).encode("utf-8")) + flat_dict = flat.to_dict() if hasattr(flat, "to_dict") else flat + self.wfile.write(json.dumps({"project": flat_dict}).encode("utf-8")) elif self.path == "/api/project_switch_status": # Determinstic signal for tests waiting on a project switch to complete. # Polling /api/project returns derived state that may be stale from prior diff --git a/src/app_controller.py b/src/app_controller.py index 2505b27c..0e3100b5 100644 --- a/src/app_controller.py +++ b/src/app_controller.py @@ -4025,6 +4025,9 @@ class AppController: self.save_config() track_id = self.active_track.id if self.active_track else None flat = project_manager.flat_config(self.project, self.active_discussion, track_id=track_id) + # flat_config returns a frozen ProjectContext dataclass; convert to a mutable dict + # before overriding files.paths with the runtime context_files list. + flat = flat.to_dict() if hasattr(flat, "to_dict") else dict(flat) import copy flat["files"] = copy.copy(flat.get("files", {})) diff --git a/tests/test_app_controller_result.py b/tests/test_app_controller_result.py index 6cb1cd96..b683bc91 100644 --- a/tests/test_app_controller_result.py +++ b/tests/test_app_controller_result.py @@ -460,6 +460,9 @@ def test_fetch_models_aggregates_per_provider_errors(): # do_fetch is the inner function; we need to access it. Easiest: call _fetch_models # and inspect the resulting side effect on all_available_models. ctrl._fetch_models("anthropic") + # _fetch_models submits do_fetch to the IO pool; wait for it to complete + # before asserting on the side effect. + assert ctrl.wait_io_pool_idle(timeout=5.0), "do_fetch did not complete" # Per-provider errors should be accumulated in self._model_fetch_errors assert "gemini" in ctrl._model_fetch_errors assert isinstance(ctrl._model_fetch_errors["gemini"], ErrorInfo) diff --git a/tests/test_warmup_canaries.py b/tests/test_warmup_canaries.py index b9045434..56ccb4d6 100644 --- a/tests/test_warmup_canaries.py +++ b/tests/test_warmup_canaries.py @@ -255,9 +255,12 @@ def test_app_controller_init_start_ts_is_set() -> None: def test_app_controller_warmup_done_ts_none_until_completed() -> None: """warmup_done_ts is None before wait, float after.""" from src.app_controller import AppController - ctrl = AppController(log_to_stderr=False) + # defer_warmup=True avoids the race where warmup completes before we can + # observe the initial state (warmup runs in a background thread). + ctrl = AppController(defer_warmup=True, log_to_stderr=False) initial = ctrl.warmup_done_ts assert initial is None + ctrl.start_warmup() assert ctrl.wait_for_warmup(timeout=60.0) is True assert isinstance(ctrl.warmup_done_ts, float) assert ctrl.warmup_done_ts > 0