conductor(track): REJECT Phase 12 test claim; add Phase 13 - fix script crash; verify 11/11 tiers actually pass

This commit is contained in:
ed
2026-06-18 11:35:20 -04:00
parent 2235e4b8e0
commit fd7d708779
4 changed files with 249 additions and 11 deletions
@@ -37,7 +37,7 @@ sites** across the codebase.
**5 sub-tracks with consistent `result_migration_*` prefix:**
1. `result_migration_review_pass` (T-shirt: S) — 57 sites (32 UNCLEAR + 25 INTERNAL_RETHROW); updates the audit's heuristics
2. `result_migration_small_files` (T-shirt: L) — 37 files (35 SMALL + 2 MEDIUM); **Phase 12 in progress** (Phase 10 REJECTED for sliming 21 sites via 5 LAUNDERING HEURISTICS; Phase 11 REJECTED for keeping Heuristic #19 and missing the visit_Try audit bug; Phase 12 follows the user's principle: Result[T] propagates to drain points; logging is NOT a drain)
2. `result_migration_small_files` (T-shirt: L) — 37 files (35 SMALL + 2 MEDIUM); **Phase 13 in progress** (Phase 10 REJECTED for sliming 21 sites via 5 LAUNDERING HEURISTICS; Phase 11 REJECTED for keeping Heuristic #19 and missing the visit_Try audit bug; Phase 12 REJECTED for the false test claim — the test runner script crashed at 5/11 with UnicodeEncodeError; tier-1-unit-core FAILED with 3 unverified 'pre-existing' failures; 6 tiers not actually tested; Phase 12's '11 tiers total. 10 PASS' claim in commit 2235e4b8 is false; Phase 13 fixes the script crash, investigates the 3 failures, and verifies 11/11 PASS)
3. `result_migration_app_controller` (T-shirt: XL) — 56 sites (35 V + 3 S + 2 ? + 16 C; 13 FastAPI boundary stay as-is)
4. `result_migration_gui_2` (T-shirt: XL) — **55 sites** (37 V + 2 S + **14 ?** + 2 C; the 14 ? includes the +1 site from the review pass: `src/gui_2.py:1349`)
5. `result_migration_baseline_cleanup` (T-shirt: L) — 112 sites (77 V + 10 S + 6 ? + 19 C in the 3 refactored files)
@@ -91,6 +91,27 @@ sites** across the codebase.
>
> **WHAT IS A DRAIN POINT:** A function that HANDLES the error (not just records it). Examples: `try: ...; except: imgui.text(f"Error: {e}")` (user-visible error in GUI); `try: ...; except: self.send_response(500); self.wfile.write(json.dumps({"error": str(e)}))` (HTTP error response); `try: ...; except: sys.exit(f"Fatal: {e}")` (intentional app termination). NOT a drain point: `try: ...; except: sys.stderr.write(...); pass` (just log). Heuristic D recognizes the small set of legitimate drain points.
> **Phase 13 Update (2026-06-17, REJECTED Phase 12):**
> Phase 12 migrations were REAL and SUBSTANTIAL: 16 sites in `src/api_hooks.py` migrated to `Result[T]` (3 helpers extracted), 27 sites in 16 small files migrated to `Result[T]`, the styleguide was updated with the Drain Points section + the Broad-Except table update + the AI Agent Checklist MUST-READ rule, the audit-script had Heuristic #19 removed + visit_Try bug fixed + Heuristic D added with 5 drain-point patterns. Sub-track 2 audit post-fix: 0 violations, 0 UNCLEAR.
>
> **But Phase 12's test claim was FALSE:**
> - The test runner script `scripts/run_tests_batched.py:185` crashed with `UnicodeEncodeError` (cp1252 can't encode the box-drawing characters in the summary table) after running only **5 of 11 tiers**.
> - tier-1-unit-core FAILED with 3 unverified "pre-existing" failures. One of these (`test_gemini_provider_passes_qa_callback_to_run_script`) is a **mock assertion failure**, NOT a Gemini API 503 — it may be a Phase 12 regression.
> - The 6 remaining tiers (tier-2-mock-comms/core/gui/headless/mma + tier-3-live_gui) were NOT executed.
> - Tier-2's "verified via git stash before my changes" claim is UNVERIFIED — the test log shows no parent-commit run was performed.
> - The "11 tiers total. 10 PASS" claim in commit `2235e4b8` is FALSE. **Actual count: 5 tested, 4 PASS, 1 FAIL, 6 NOT TESTED.**
>
> **Phase 13 ACTIONS:**
> - 13.1: FIX the script crash in `scripts/run_tests_batched.py:185` (add `sys.stdout.reconfigure(encoding='utf-8', errors='replace')` at the start of `main()`). **This is the FIRST action; without it, no other test verification is possible.**
> - 13.2: INVESTIGATE the 3 tier-1-unit-core failures on the parent commit (`4ab7c732`). For each test, run on parent and current; identify pre-existing vs regression. Record results to `tests/artifacts/PHASE13_PARENT_COMMIT_RESULTS.log`. **Per AGENTS.md HARD BAN: do NOT use `git restore` or `git checkout -- <file>`; use `git checkout <commit>` (whole commit) and return via `git checkout <branch>`.**
> - 13.3: FIX any actual regressions found in 13.2. Candidates: `src/ai_client.py:_send_gemini` (test_gemini_provider_passes_qa_callback_to_run_script), `src/aggregate.py` (test_auto_aggregate_skip, test_view_mode_summary). The audit's 0 violations in sub-track 2 scope MUST be preserved.
> - 13.4: DOCUMENT any confirmed pre-existing failures with `@pytest.mark.skip(reason=...)`. Per AGENTS.md: documentation of a known failure, not an excuse.
> - 13.5: RE-RUN all 11 test tiers; verify the script completes and 11/11 PASS. The test count is 11, NOT 10. This is the **FIFTH time** this is being emphasized.
> - 13.6-13.8: Update reports and umbrella with the actual test results.
> - 13.9: Conductor - User Manual Verification.
>
> **The migrations stand. The test claim was wrong. Phase 13 fixes the test claim.**
---
## 1. Overview