From 09debfe30d5cfbab73aef5f811ef92d44d4155f6 Mon Sep 17 00:00:00 2001 From: Ed_ Date: Wed, 17 Jun 2026 18:59:11 -0400 Subject: [PATCH] docs(track): result_migration_small_files Phase 2 per-site decisions (4 UNCLEAR sites classified) Classifies the 4 UNCLEAR sites in the SMALL bucket: 1. src/outline_tool.py:49 - Migration-target (narrow except SyntaxError + return formatted str; should return Result[str]) 2. src/summarize.py:36 - Migration-target (same pattern as outline_tool; queued for Phase 7 t7_8) 3. src/conductor_tech_lead.py:120 - Compliant (wrap-and-rethrow with descriptive message; public API; stays as-is) 4. src/openai_compatible.py:87 - Compliant (already migrated Result-based SDK boundary; audit heuristic gap noted as follow-up) Per-site rationale is in docs/reports/RESULT_MIGRATION_SMALL_FILES_20260617.md section "Site N" entries. Migration targets: 2 sites added to Phase 7 (t7_6 outline_tool, t7_8 summarize). Compliant-no-migration: 2 sites (conductor_tech_lead, openai_compatible). --- .../RESULT_MIGRATION_SMALL_FILES_20260617.md | 138 ++++++++++++++++++ 1 file changed, 138 insertions(+) create mode 100644 docs/reports/RESULT_MIGRATION_SMALL_FILES_20260617.md diff --git a/docs/reports/RESULT_MIGRATION_SMALL_FILES_20260617.md b/docs/reports/RESULT_MIGRATION_SMALL_FILES_20260617.md new file mode 100644 index 00000000..833456ef --- /dev/null +++ b/docs/reports/RESULT_MIGRATION_SMALL_FILES_20260617.md @@ -0,0 +1,138 @@ +# Result Migration Sub-Track 2 — Per-Site Decisions for the 4 SMALL UNCLEAR Sites + +This document records the per-site classification decisions for the 4 UNCLEAR sites identified in the `result_migration_review_pass_20260617` audit. Each site is reviewed and either classified as **Compliant (no migration)** or **Migration-target** (queued for Phase 3+ migration). + +The pre-Phase-1 audit reported 4 UNCLEAR sites in the SMALL bucket. After Phase 1's audit-script bug fixes, the audit counts are slightly different (see audit_post_phase1.json). The decisions below use the post-Phase-1 site lines. + +--- + +## Site 1: `src/outline_tool.py:49` — **Migration-target** + +**Snippet (lines 45-52):** +```python +def outline(self, code: str) -> str: + code = code.lstrip(chr(0xFEFF)) + try: + tree = ast.parse(code) + except SyntaxError as e: + return f"ERROR parsing code: {e}" +``` + +**Classification rationale:** +- Function signature: `def outline(self, code: str) -> str` +- `ast.parse()` is stdlib I/O that can raise `SyntaxError` +- The except handler returns an error string, NOT a Result or ErrorInfo +- Caller cannot distinguish a valid outline from an error message + +**Decision:** Migration-target. The function should return `Result[str]` where the success path returns `Result(data=outline_str)` and the parse-error path returns `Result(data=NIL_T, errors=[ErrorInfo(category="syntax_error", message=str(e), source="outline_tool")])`. The caller is updated to check `result.ok` and `result.errors`. + +**Migration site:** `Phase 7: src/outline_tool.py` (task t7_6, included in the 3 sites for that file). + +--- + +## Site 2: `src/summarize.py:36` — **Migration-target** + +**Snippet (lines 33-40):** +```python +def _summarise_python(path: Path, content: str) -> str: + lines = content.splitlines() + line_count = len(lines) + parts = [f"**Python** — {line_count} lines"] + try: + tree = ast.parse(content.lstrip(chr(0xFEFF)), filename=str(path)) + except SyntaxError as e: + parts.append(f"_Parse error: {e}_") + return "\n".join(parts) +``` + +**Classification rationale:** +- Function signature: `def _summarise_python(path: Path, content: str) -> str` +- `ast.parse()` is stdlib I/O that can raise `SyntaxError` +- The except handler appends to `parts` and returns the joined string +- Caller cannot distinguish a valid summary from a parse-error message + +**Decision:** Migration-target. Same pattern as outline_tool.py:49. Function should return `Result[str]` with proper ErrorInfo conversion. + +**Migration site:** `Phase 7: src/summarize.py` (task t7_8, included in the 2 sites for that file). + +--- + +## Site 3: `src/conductor_tech_lead.py:120` — **Compliant (no migration)** + +**Snippet (lines 116-122):** +```python +try: + sorted_ids = dag.topological_sort() +except ValueError as e: + raise ValueError(f"DAG Validation Error: {e}") +``` + +**Classification rationale:** +- Function is part of a public API (`generate_tickets` or similar; the function returns `list[dict]`) +- `dag.topological_sort()` is internal code that raises `ValueError` for cycle detection (programmer-error / validation failure) +- The except handler catches `ValueError` and re-raises with a more descriptive message (`"DAG Validation Error: ..."`) +- This is the **wrap-and-rethrow** pattern: catch + augment message + re-raise same exception type +- Migrating to `Result[List[Ticket]]` would change the public API contract; out of scope for sub-track 2 + +**Decision:** Compliant. Keep the rethrow pattern. The function's validation failure is a programmer-error signal (the DAG has a cycle, which is a bug in the input data, not a runtime condition). Document the decision in the per-site table; no migration. + +**Migration site:** None (stays as-is). + +--- + +## Site 4: `src/openai_compatible.py:87` — **Compliant (already migrated; audit heuristic gap)** + +**Snippet (lines 78-90):** +```python +try: + if request.stream: + response = _send_streaming(client, kwargs, request.stream_callback) + else: + response = _send_blocking(client, kwargs) + return Result(data=response) +except OpenAIError as exc: + empty_resp = NormalizedResponse(text="", tool_calls=[], usage_input_tokens=0, ...) + return Result(data=empty_resp, errors=[_classify_openai_compatible_error(exc, source="openai_compatible")]) +``` + +**Classification rationale:** +- Function signature: `def send_openai_compatible(client: Any, request: OpenAICompatibleRequest, *, capabilities: Any) -> Result[NormalizedResponse]` +- `OpenAIError` is a third-party SDK exception +- Both paths return `Result[NormalizedResponse]`; the except path converts to `Result(data=empty_resp, errors=[ErrorInfo])` +- This is a **properly-migrated SDK-boundary site** following the data-oriented convention +- The audit's heuristic classifies it as UNCLEAR because: + - The function is named `send_openai_compatible`, NOT `*_result` (so the `is_in_result_func` heuristic at #3 doesn't fire) + - The third-party SDK is called via `client.chat.completions.create(...)`, not a literal `openai.*` reference (so `is_third_party` heuristic at #4 doesn't fire) + - The except body is a multi-line Result construction (not a simple `return Result(...)`) + +**Decision:** Compliant. The site is already a textbook example of the data-oriented convention: catch SDK exception, convert to ErrorInfo, return Result with errors. The audit's heuristic gap is a follow-up improvement. + +**Audit heuristic gap (optional follow-up):** Add a heuristic that recognizes "try/except SDK_error + body returns Result with errors list" pattern. This would catch future sites that follow the same pattern without requiring a literal `openai.*` module reference. See "Audit Heuristic Improvement" section below. + +**Migration site:** None (already migrated). + +--- + +## Per-Site Summary + +| Site | File:Line | Decision | Migration Plan | +|---|---|---|---| +| 1 | `src/outline_tool.py:49` | Migration-target | Phase 7 (t7_6): migrate to `Result[str]` | +| 2 | `src/summarize.py:36` | Migration-target | Phase 7 (t7_8): migrate to `Result[str]` | +| 3 | `src/conductor_tech_lead.py:120` | Compliant (no migration) | Stays as-is (wrap-and-rethrow) | +| 4 | `src/openai_compatible.py:87` | Compliant (already migrated) | Stays as-is (Result-based) | + +**Migration-target count:** 2 sites (added to Phase 7 batches t7_6 and t7_8). +**Compliant-no-migration count:** 2 sites (no code change). + +--- + +## Audit Heuristic Improvement (Optional Follow-up) + +The 4 UNCLEAR classifications suggest 2 heuristic gaps: + +1. **`outline_tool.py:49` / `summarize.py:36` (SyntaxError + return formatted str)**: The audit doesn't have a heuristic for "narrow except (SyntaxError) + return formatted error string." This is a common pattern but the convention says functions should return Result. A heuristic could flag these as migration-targets (INTERNAL_BROAD_CATCH-style violation) so they're caught in future audits. + +2. **`openai_compatible.py:87` (Result-based SDK boundary)**: The audit doesn't have a heuristic for "try/except SDK_error + body returns Result with errors list." This is the canonical migrated pattern. A heuristic could classify these as BOUNDARY_SDK or INTERNAL_COMPLIANT. + +These heuristic improvements are deferred to a follow-up track. The sub-track 2 migrations (Phase 7) handle the 2 migration-target sites directly.