diff --git a/conductor/tracks/fix_mma_concurrent_tracks_sim_20260627/state.toml b/conductor/tracks/fix_mma_concurrent_tracks_sim_20260627/state.toml index 1c867bd8..17bc32ca 100644 --- a/conductor/tracks/fix_mma_concurrent_tracks_sim_20260627/state.toml +++ b/conductor/tracks/fix_mma_concurrent_tracks_sim_20260627/state.toml @@ -15,7 +15,7 @@ post_module_taxonomy_de_cruft_20260627 = "shipped (the parent track; this is the [phases] phase_0 = { status = "completed", checkpointsha = "75fdebb0", name = "Instrument + diagnose (3 commits: stderr diag, file-based diag, NameError root cause identification)" } -phase_1 = { status = "in_progress", checkpointsha = "e9919059", name = "Fix the root cause (2 commits: TrackMetadata import fix, mock session_id routing fix, mock epic catch-all fix)" } +phase_1 = { status = "in_progress", checkpointsha = "e9919059", name = "Fix the root cause (3 commits: TrackMetadata import, mock session_id routing, mock epic catch-all, mock worker fallback, refresh_from_project task removal)" } phase_2 = { status = "pending", checkpointsha = "23862d35", name = "Remove instrumentation + write report (3 commits: cleanup, mock fix, TRACK_COMPLETION)" } [tasks] @@ -24,13 +24,14 @@ t0_1b = { status = "completed", commit_sha = "d046394a", description = "Add file t0_2 = { status = "completed", commit_sha = "75fdebb0", description = "Run the test in isolation; capture log; identify NameError as root cause" } t1_1 = { status = "completed", commit_sha = "e9919059", description = "Add TrackMetadata to import; change models.Metadata to TrackMetadata" } t1_1b = { status = "completed", commit_sha = "913aa48c", description = "Fix mock sprint routing (replace session_id-based with prompt-content-based)" } -t1_1c = { status = "completed", commit_sha = "fad1755b", description = "Fix mock epic routing to be a catch-all for any non-empty prompt (stress test prompt 'STRESS TEST: TRACK A AND TRACK B' was not matched by the old literal 'PATH: Epic Initialization' check)" } -t1_2 = { status = "completed", commit_sha = "e9919059", description = "Run the test in isolation to verify the fix (5 consecutive PASS runs of execution test)" } -t1_2b = { status = "completed", commit_sha = "fad1755b", description = "Run both tests in isolation to verify the stress test fix (3 consecutive PASS runs)" } -t1_3 = { status = "completed", commit_sha = "e9919059", description = "Verify no regressions in related tests (test_app_controller_result, test_conductor_tech_lead all pass except pre-existing broad_except test)" } +t1_1c = { status = "completed", commit_sha = "fad1755b", description = "Fix mock epic routing to be a catch-all for any non-empty prompt" } +t1_1d = { status = "completed", commit_sha = "d28e373e", description = "Fix mock worker routing (remove session_id fallback that caused stale session_id to match)" } +t1_1e = { status = "completed", commit_sha = "55dae159", description = "Remove 'refresh_from_project' task that overwrote self.tracks with a disk read returning 0 tracks" } +t1_2 = { status = "completed", commit_sha = "55dae159", description = "Run the test in isolation AND in batched combination (3 consecutive PASS runs of the failing combination at 100.57s, 100.29s, 100.18s)" } +t1_3 = { status = "completed", commit_sha = "55dae159", description = "Verify no regressions (15 wider tests pass at 237.63s)" } t2_1 = { status = "completed", commit_sha = "23862d35", description = "Remove the stderr and file-based instrumentation from _start_track_logic_result" } -t2_2 = { status = "completed", commit_sha = "fad1755b", description = "Update OUTSTANDING_MMA_TEST_FAILURES_20260627.md to add section 6 (stress test fix)" } -t2_3 = { status = "in_progress", commit_sha = "", description = "Update TRACK_COMPLETION_fix_mma_concurrent_tracks_sim_20260627.md to include the stress test fix" } +t2_2 = { status = "completed", commit_sha = "55dae159", description = "Update OUTSTANDING_MMA_TEST_FAILURES_20260627.md to add section 7" } +t2_3 = { status = "in_progress", commit_sha = "", description = "Update TRACK_COMPLETION_fix_mma_concurrent_tracks_sim_20260627.md to include all 5 fixes" } t2_4 = { status = "pending", commit_sha = "", description = "Update state.toml to status = completed; final SHIPPED commit" } [verification] @@ -39,7 +40,7 @@ phase_1_complete = true phase_2_complete = false phase_0_diagnosis = "NameError: name 'models' is not defined at src/app_controller.py:4830" -phase_1_fix_commits = ["e9919059", "913aa48c", "fad1755b"] +phase_1_fix_commits = ["e9919059", "913aa48c", "fad1755b", "d28e373e", "55dae159"] phase_2_cleanup_commits = ["23862d35"] [track_specific] @@ -55,15 +56,19 @@ prior_fixes_in_635ca552 = [ root_causes_identified = [ "NameError: name 'models' is not defined at src/app_controller.py:4830 (missing TrackMetadata import after de-cruft migration removed 'from src import models')", "Mock sprint routing fragile to test ordering and session_id chain pattern (session_id='mock-sprint-A' incorrectly routed to sprint-A instead of sprint-B)", - "Mock epic branch only matched literal 'PATH: Epic Initialization' (stress test prompt 'STRESS TEST: TRACK A AND TRACK B' fell to Default which returns text, not JSON)" + "Mock epic branch only matched literal 'PATH: Epic Initialization' (stress test prompt 'STRESS TEST: TRACK A AND TRACK B' fell to Default which returns text, not JSON)", + "Mock worker check had session_id.startswith('mock-worker-') fallback that incorrectly matched the stress test's epic call when the gemini_cli_adapter's session_id persisted from the execution test's worker call", + "Production: 'refresh_from_project' task in _start_track_logic_result and _cb_accept_tracks._bg_task overwrote self.tracks with a disk read that returned 0 tracks in batched test environments, losing the in-memory tracks that were just appended" ] fixes_shipped = [ "e9919059: Added TrackMetadata to 'from src.mma import' line; changed 'models.Metadata(...)' to 'TrackMetadata(...)'", "913aa48c: Replaced session_id-based mock sprint routing with prompt-content-based routing", - "fad1755b: Restructured mock routing so sprint/worker checked first, then epic catch-all for any non-empty prompt" + "fad1755b: Restructured mock routing so sprint/worker checked first, then epic catch-all for any non-empty prompt", + "d28e373e: Removed session_id.startswith('mock-worker-') fallback from worker check (route on prompt content only)", + "55dae159: Removed 'refresh_from_project' task appends from _start_track_logic_result and _cb_accept_tracks._bg_task (the bg_task already updates self.tracks directly via self.tracks.append(...))" ] -stability_test = "3 consecutive PASS runs of BOTH tests (13.94s, 14.81s, 14.13s)" -flakiness_rate = "0% (was previously 100% for stress test, ~25% for execution test)" +stability_test = "3 consecutive PASS runs of the failing combination (100.57s, 100.29s, 100.18s); 15 wider tests pass at 237.63s" +flakiness_rate = "0% (was previously 100% for stress test in batch)" audit_main_thread_imports = "OK: 28 files in main-thread import graph; no heavy top-level imports" audit_weak_types = "informational; no new violations" pre_existing_failures_remaining = ["test_app_controller_result.py::test_app_controller_does_not_use_broad_except (8 INTERNAL_BROAD_CATCH sites; not introduced by this track)"] diff --git a/docs/reports/OUTSTANDING_MMA_TEST_FAILURES_20260627.md b/docs/reports/OUTSTANDING_MMA_TEST_FAILURES_20260627.md index a7f87548..8792eaa5 100644 --- a/docs/reports/OUTSTANDING_MMA_TEST_FAILURES_20260627.md +++ b/docs/reports/OUTSTANDING_MMA_TEST_FAILURES_20260627.md @@ -144,3 +144,22 @@ The stress test (`tests/test_mma_concurrent_tracks_stress_sim.py::test_mma_concu **Status:** ✅ **FIXED** in commit `fad1755b` (restructured routing so sprint and worker are checked first, and any non-empty prompt that doesn't match those patterns is treated as an epic request returning 2 tracks). **Verification:** 3 consecutive PASS runs of both `test_mma_concurrent_tracks_execution` AND `test_mma_concurrent_tracks_stress` (13.94s, 14.81s, 14.13s). + + +### 7. ✅ **RESOLVED** — Production bug: 'refresh_from_project' task overwrites self.tracks + +**Date:** 2026-06-27 (discovered after the second batched test run) + +After the epic catch-all fix, the batched test still failed. Diagnostic logging revealed that `self.tracks` was being replaced between track appends (different `id(self.tracks)` values in the log). Root cause: + +`_start_track_logic_result` (and `_cb_accept_tracks._bg_task`) appended a `'refresh_from_project'` task to `_pending_gui_tasks` at the end. The main thread processed this task by calling `_refresh_from_project`, which does: + + self.tracks = project_manager.get_all_tracks(self.active_project_root) + +This REPLACED `self.tracks` with a fresh disk read. In batched test environments, the disk read returned 0 tracks (due to timing or path issues), losing the in-memory tracks that were just appended by `self.tracks.append(...)`. + +**Fix:** Remove the `'refresh_from_project'` task appends from both `_start_track_logic_result` and `_cb_accept_tracks._bg_task`. The bg_task already updates `self.tracks` directly via `self.tracks.append(...)`. The refresh is unnecessary for the accept flow because the other state (files, disc_entries, etc.) doesn't change during the accept. + +**Status:** ✅ **FIXED** in commit `55dae159`. + +**Verification:** 3 consecutive PASS runs of the failing test combination (test_context_sim_live + test_mma_concurrent_tracks_execution + test_mma_concurrent_tracks_stress) at 100.57s, 100.29s, 100.18s. Also passes 15 wider tests (237.63s) with no regressions.