conductor(track): Phase 12 prerequisites - tier-2 MUST read styleguide; styleguide must be updated to be aware of drain points

2026-06-18 09:03:58 -04:00
parent 7c1d84623c
commit 6b7fb9cdb8
4 changed files with 70 additions and 1 deletions
@@ -69,12 +69,16 @@ sites** across the codebase.
 > **Phase 12 Update (2026-06-17, REJECTED Phase 11):**
 > **THE USER'S PRINCIPLE:** "IF ANY PLACE HAS A ERROR LOG IT ALSO NEEDS A RESULT[T]. RESULT[T] PROPOGATES UNTIL IT REACHED A 'DRAIN' POINT WHERE THE ERROR CAN BE HANDLED APPROPRIATELY WITHOUT CRASHING THE APP. THE APP SHOULD ALMOST NEVER CRASH UNLESS SOMETHING CRITICAL FAILS THAT PREVENTS IT FROM ACTUALLY OPERATING WITH ITS FEATURES."
 >
+> **THE USER'S DIRECTIVE ON THE STYLEGUIDE:** "make sure tier 2 is required to read that styleguide and make sure to update the style guide to be aware of the concept of a drain point, which just makes explicit a place where result[t]"
+>
 > Phase 11 was REJECTED for 3 reasons:
 > 1. **Heuristic #19 is LAUNDERING.** The "narrow + log = compliant" pattern is WRONG. Logging is NOT a drain. Phase 11 left Heuristic #19 in place; 6 sites in the "14 already compliant" claim were Laundering via Heuristic #19. Phase 12.1 REMOVES Heuristic #19.
 > 2. **The audit-script `visit_Try` walker is BUGGY.** It does NOT recurse into `node.body` (the try body itself), so nested Trys are silently dropped. I verified: `src/api_hooks.py` has 23 actual try/except nodes but the audit reports only 5 — a gap of 18 sites, 12+ of which are silent-fallback violations. Phase 12.2 FIXES this bug.
 > 3. **Tier-2 misclassified 2 sites.** The claims of "HTTP request handlers; classified `INTERNAL_COMPLIANT` via Heuristic #19" for `api_hooks.py:451` and `:824` are wrong about which heuristic applies. The actual code at L451 is `except (OSError, ValueError) as e: self.send_response(500)` (narrow + HTTP response, NOT a Heuristic #19 log call). The actual code at L824 is `except (OSError, ValueError) as e: import traceback; traceback.print_exc(file=sys.stderr)` (narrow + traceback, NOT a Heuristic #19 log call). Phase 12.6.1 migrates these.
 >
 > **Phase 12 ACTIONS:**
+> - 12.0: TIER-2 MUST READ `conductor/code_styleguides/error_handling.md` end-to-end BEFORE any Phase 12 code work. NO CODE; the read is acknowledged in the commit message of 12.0.1.
+> - 12.0.1: UPDATE `error_handling.md` with 3 changes: (A) add a "Drain Points" section with 5 patterns; (B) update the "Broad-Except Distinction" table to explicitly say `narrow + log = INTERNAL_SILENT_SWALLOW` violation (prevents Heuristic #19 regression); (C) add a MUST-READ rule to the AI Agent Checklist.
 > - 12.1: REMOVE Heuristic #19 (narrow+log laundering)
 > - 12.2: FIX the visit_Try audit bug (2-line change to recurse into node.body)
 > - 12.3: ADD Heuristic D (True Drain-Point Recognition) with 5 patterns: HTTP error response, GUI error display, intentional app termination, telemetry emission, retry-with-bounded-attempts
@@ -139,6 +139,9 @@
    "phase_11_known_limitation_warmup_L185": 1,
    "phase_11_status": "REJECTED; Heuristic #19 left in place (logging is NOT a drain); visit_Try audit bug not fixed; tier-2 misclassified 2 sites; ~18+ nested-Try sites silently missed; tier-2's test count claim of 10/11 tiers was wrong (the 11th tier tier-1-unit-comms was miscounted)",
    "phase_12_user_principle": "IF ANY PLACE HAS A ERROR LOG IT ALSO NEEDS A RESULT[T]. RESULT[T] PROPOGATES UNTIL IT REACHED A DRAIN POINT WHERE THE ERROR CAN BE HANDLED APPROPRIATELY WITHOUT CRASHING THE APP. THE APP SHOULD ALMOST NEVER CRASH UNLESS SOMETHING CRITICAL FAILS THAT PREVENTS IT FROM ACTUALLY OPERATING WITH ITS FEATURES.",
+    "phase_12_user_directive_2": "make sure tier 2 is required to read that styleguide and make sure to update the style guide to be aware of the concept of a drain point, which just makes explicit a place where result[t]",
+    "phase_12_prerequisites": "TIER-2 MUST READ conductor/code_styleguides/error_handling.md end-to-end BEFORE any Phase 12 code work. The styleguide is the source of truth. The AI's training data is the OPPOSITE of this convention. The read is acknowledged in the commit message of the next task (t12_0.2).",
+    "phase_12_styleguide_update": "3 changes to conductor/code_styleguides/error_handling.md: (A) add Drain Points section with 5 patterns (HTTP error response, GUI error display, app termination, telemetry, retry-with-bounded-attempts); (B) update Broad-Except Distinction table to explicitly say narrow+log = INTERNAL_SILENT_SWALLOW violation (prevents Heuristic #19 regression); (C) add MUST-READ rule to AI Agent Checklist. Without these changes, the next agent will re-add Heuristic #19 because the styleguide's narrow+log=violation rule is implicit in the Broad-Except Distinction table, not explicit.",
    "phase_12_visit_try_bug_fixed": "in progress; the bug: visit_Try does not recurse into node.body; the fix: add 'for child in node.body: self.visit(child)'; verified: src/api_hooks.py has 23 actual try/except nodes but the audit only reports 5 (gap of 18 sites, 12+ of which are silent-fallback violations)",
    "phase_12_heuristic_19_REMOVED": "in progress; Heuristic #19 ('narrow + log = compliant') was laundering. Logging is NOT a drain. The user's principle: Result[T] must propagate to a real drain point.",
    "phase_12_heuristic_D_added": "in progress; 5 drain-point patterns: (1) HTTP error response, (2) GUI error display, (3) intentional app termination, (4) telemetry emission, (5) retry-with-bounded-attempts. TDD-first; each pattern has a passing test.",
@@ -873,6 +873,65 @@ The migration will likely surface **20-50+ additional sites** beyond Phase 11's

 ---

+### 12.0 — TIER-2 MUST READ `error_handling.md` (PREREQUISITE)
+
+**WHY:** Phase 12 follows the user's principle (Result[T] propagates to drain points; logging is NOT a drain). This principle is codified in `conductor/code_styleguides/error_handling.md`. Tier-2 MUST read the styleguide end-to-end BEFORE doing any Phase 12 work. Without this prerequisite, tier-2 will revert to idiomatic Python (`try/except` + `Optional[T]` + `raise Exception`) and reintroduce the exact problems Phase 12 is fixing.
+
+- **WHERE:** `conductor/code_styleguides/error_handling.md` (the canonical convention reference)
+- **WHAT:** Read the entire file. Specifically, the following sections are relevant to Phase 12:
+  - "The 5 Patterns" (lines 22-129) — the canonical Result[T] / ErrorInfo / nil-sentinel patterns
+  - "Decision Tree" (lines 152-171) — when to use Result vs nil-sentinel vs assert vs raise
+  - "Anti-Patterns" (lines 175-190) — the 6 forbidden patterns
+  - "Hard Rules" (lines 212-252) — the 3 non-negotiable rules in refactored files
+  - "Boundary Types" (lines 274-352) — the 3 boundary patterns (SDK, stdlib I/O, FastAPI)
+  - "The Broad-Except Distinction" (lines 356-411) — the canonical table that says `log only` is a violation
+  - "AI Agent Checklist" (lines 639-779) — the 5 MUST-DO and 7 MUST-NOT-DO rules
+- **HOW:** Open the file in your editor; read it. Confirm in the FIRST Phase 12 commit message: `"TIER-2 READ conductor/code_styleguides/error_handling.md before Phase 12.0.1."`
+- **NO CODE:** This is a READ task. No code changes. The acknowledgment is in the commit message of the next task.
+- **SAFETY:** The acknowledgment in the commit message is the audit trail. If tier-2 proceeds without reading, the styleguide-based reasoning in subsequent commits will be inconsistent.
+
+---
+
+### 12.0.1 — UPDATE `error_handling.md` to be aware of drain points
+
+**WHY:** The user's principle introduces a new concept — the **drain point** — that is not currently in the styleguide. The styleguide's "Boundary Types" section has 3 patterns (SDK, stdlib I/O, FastAPI HTTPException); these are **boundaries** (where exceptions originate or are converted). The user's "drain point" is different: it's where the error is **HANDLED** (not just converted or recorded). A drain point is the place where Result[T] propagation TERMINATES — the error is finally visible to the user or used to make a decision.
+
+Without updating the styleguide, the next agent will re-add Heuristic #19 (or its equivalent) because the styleguide's "narrow + log = violation" rule is IMPLICIT in the Broad-Except Distinction table, not EXPLICIT. Making it explicit in the styleguide prevents the regression.
+
+- **WHERE:** `conductor/code_styleguides/error_handling.md`
+- **WHAT:** Make 3 changes:
+
+  **(A) Add a "Drain Points" section** AFTER the "Boundary Types" section (around line 352). The section defines what a drain point is and lists the 5 recognized patterns:
+  - **HTTP error response** (`try: ...; except SomeError as e: self.send_response(<status>); self.wfile.write(json.dumps({"error": str(e)}))`) — the catch site delivers a user-visible HTTP error. The error is HANDLED at the HTTP boundary.
+  - **GUI error display** (`try: ...; except SomeError as e: imgui.text(f"Error: {e}")`) — the catch site displays the error to the user. The error is HANDLED in the GUI.
+  - **Intentional app termination** (`try: ...; except SomeError as e: sys.exit(f"Fatal: {e}")`) — the catch site terminates the app with a message. The error is HANDLED via the termination.
+  - **Telemetry emission** (`try: ...; except SomeError as e: telemetry.emit("error", ...)`) — the catch site emits a structured error event. The error is HANDLED via the data-oriented event record.
+  - **Retry-with-bounded-attempts** (`try: ...; except SomeError as e: if attempt < max: retry(); else: raise`) — the catch site retries with a bounded loop. The error is HANDLED within the retry envelope; if all retries fail, the error propagates.
+  
+  Each pattern has a code example (compliant) and a "NOT a drain" example (violation). The section explicitly states: **`sys.stderr.write(...)` alone is NOT a drain** — the error is recorded but not visible to the user and not used to make a decision; it's the data being lost. Logging alone requires the function to also return `Result[T]` so the caller can drain or propagate.
+
+  **(B) Update the "Broad-Except Distinction" table** (lines 358-370) to add an explicit row:
+  ```
+  | `narrow except + log (sys.stderr.write/logging.*) only | INTERNAL_SILENT_SWALLOW | **Violation** (the data is lost; logging is not a drain) |
+  ```
+  This makes the Heuristic #19 laundering IMPOSSIBLE — the styleguide explicitly forbids it.
+
+  **(C) Add to the AI Agent Checklist (MUST-DO)** a new top-level rule:
+  ```
+  0. **READ the styleguide FIRST.** Before writing or modifying any try/except code, READ `conductor/code_styleguides/error_handling.md` end-to-end. Acknowledge the read in the commit message. The styleguide is the source of truth; the AI's training data is the OPPOSITE of this convention.
+  ```
+
+- **HOW:** Use `manual-slop_edit_file` to make the 3 changes. The 3 changes are non-overlapping (different sections of the file). Verify with `py_check_syntax` on any other files affected. The styleguide is markdown, not Python, so syntax checks don't apply — just review the rendered output.
+- **SAFETY:** The 3 changes do not break the existing convention. They ADD a new section (A), MODIFY an existing table (B) to be more explicit, and ADD a new rule to the AI Agent Checklist (C). None of the existing rules are removed or contradicted.
+- **VERIFY:** Re-read the styleguide end-to-end. Confirm:
+  - The "Drain Points" section is present and has the 5 patterns with code examples
+  - The "Broad-Except Distinction" table has the new row about "narrow + log = violation"
+  - The AI Agent Checklist has the new "READ the styleguide FIRST" rule
+- **COMMIT:** `docs(styleguide): add Drain Points section; update Broad-Except table with explicit narrow+log violation; add MUST-READ rule to AI Agent Checklist`
+- **GIT NOTE:** "Phase 12.0.1. Per user directive 2026-06-17: 'make sure to update the style guide to be aware of the concept of a drain point, which just makes explicit a place where Result[T] doesn't apply because the error is being handled.' 3 changes: (A) Drain Points section with 5 patterns (HTTP error response, GUI error display, app termination, telemetry, retry-with-bounded-attempts); (B) Broad-Except Distinction table now EXPLICITLY says narrow+log=violation (prevents Heuristic #19 regression); (C) AI Agent Checklist adds MUST-READ rule."
+
+---
+
 ### 12.1 — REMOVE Heuristic #19 (the laundering heuristic)

 **WHY:** Heuristic #19 is the "narrow + log = compliant" pattern. The user's principle: **logging is NOT a drain.** A function that catches and logs is throwing away the error context. The convention requires `Result[T]`, not `sys.stderr.write + return default`.
@@ -173,7 +173,10 @@ t11_8_1 = { status = "pending", commit_sha = "", description = "Update state.tom
 t11_8_2 = { status = "pending", commit_sha = "", description = "Update umbrella spec: Phase 11 complete; FULL Result[T] migration for 76 sites; G4 met WITHOUT laundering heuristics" }

 # Phase 12: Result[T] propagation; remove Heuristic #19; fix visit_Try; add Heuristic D; migrate ALL hidden violations
-# Per user directive 2026-06-17: "IF ANY PLACE HAS A ERROR LOG IT ALSO NEEDS A RESULT[T]. RESULT[T] PROPOGATES UNTIL IT REACHED A DRAIN POINT WHERE THE ERROR CAN BE HANDLED APPROPRIATELY WITHOUT CRASHING THE APP."
+# Per user directive 2026-06-17: "IF ANY PLACE HAS A ERROR LOG IT ALSO NEEDS A RESULT[T]. RESULT[T] PROPOGATES UNTIL IT REACHED A DRAIN POINT WHERE THE ERROR CAN BE HANDLED APPROPRIATELY WITHOUT CRASHING THE APP. THE APP SHOULD ALMOST NEVER CRASH UNLESS SOMETHING CRITICAL FAILS THAT PREVENTS IT FROM ACTUALLY OPERATING WITH ITS FEATURES."
+# Per user directive 2026-06-17: "make sure tier 2 is required to read that styleguide and make sure to update the style guide to be aware of the concept of a drain point, which just makes explicit a place where result[t]"
+t12_0_1 = { status = "pending", commit_sha = "", description = "TIER-2 MUST READ conductor/code_styleguides/error_handling.md end-to-end BEFORE any Phase 12 code work. Acknowledge the read in the commit message of t12_0.2. NO CODE — read-only prerequisite." }
+t12_0_2 = { status = "pending", commit_sha = "", description = "UPDATE conductor/code_styleguides/error_handling.md with 3 changes: (A) add Drain Points section with 5 patterns (HTTP error response, GUI error display, app termination, telemetry, retry-with-bounded-attempts); (B) update Broad-Except Distinction table to explicitly say narrow+log = INTERNAL_SILENT_SWALLOW violation (prevents Heuristic #19 regression); (C) add MUST-READ rule to AI Agent Checklist. Commit message MUST acknowledge styleguide read from t12_0.1." }
 t12_1_1 = { status = "pending", commit_sha = "", description = "REMOVE Heuristic #19 from scripts/audit_exception_handling.py (narrow+log laundering; logging is NOT a drain)" }
 t12_1_2 = { status = "pending", commit_sha = "", description = "Update the Heuristic #19 test in tests/test_audit_exception_handling_heuristics.py (same input, NEW expected category: violation)" }
 t12_2_1 = { status = "pending", commit_sha = "", description = "FIX visit_Try in scripts/audit_exception_handling.py: add 'for child in node.body: self.visit(child)' (recurse into try body)" }