Private
Public Access
conductor(spec+plan): phase2_4_5_call_site_completion_20260621 + code_path_audit pre-flight adjustments + Phase 3 analysis
PHASE 2/4/5 FOLLOW-UP TRACK (Tier 1 decided SHINK to 6a + 6b + 6d): - Phase 6a: Fix HookServer.broadcast() callers (app_controller.py + events.py + gui_2.py) Adds tests/test_websocket_broadcast_regression.py with no-TypeError assertion - Phase 6b: Complete _send_grok/_send_minimax/_send_llama OpenAICompatibleRequest migration - Phase 6d: Update those 3 senders' NormalizedResponse to use UsageStats Total: ~16 atomic commits, ~3 hours Tier 2 work. Unblocks code_path_audit_20260607. CODE_PATH_AUDIT_20260607 PRE-FLIGHT ADJUSTMENTS (per handoffs): - Add 2 new actions: provider_history_append + websocket_broadcast - Add 5 micro-benchmarks: NormalizedResponse.__init__, WebSocketMessage.__init__, UsageStats.__init__, ProviderHistory.lock, ToolSpec.__init__ - Add no-TypeError-errors-on-any-thread assertion (backs test_websocket_broadcast_regression.py) - Add 89 fat-struct sites from ANY_TYPE_AUDIT_20260621.md as instrumented targets - BLOCKER: phase2_4_5_call_site_completion_20260621 (broadcast() TypeError) PHASE 3 HYPOTHETICAL ANALYSIS (separate doc): docs/reports/PHASE3_HYPOTHETICAL_PROMOTION.md - dataclass definitions (already on tier2 branch), per-provider codepath catalog (112 sites), qualitative cost estimation (~+1-2ms per session, ~+8-15us per _send_anthropic turn). Input for the audit; the audit quantifies the cost. REGISTRATION: conductor/tracks.md updated: new row 27 (follow-up), new row 28 (parent any_type_componentization), row 17 (code_path_audit) updated with pre-flight adjustments note. Files: - conductor/tracks/phase2_4_5_call_site_completion_20260621/spec.md (NEW; 633 lines) - conductor/tracks/phase2_4_5_call_site_completion_20260621/plan.md (NEW; 7 phases, 23 tasks) - conductor/tracks/phase2_4_5_call_site_completion_20260621/metadata.json (NEW; 8.8KB) - conductor/tracks/phase2_4_5_call_site_completion_20260621/state.toml (NEW; 11.8KB) - docs/reports/PHASE3_HYPOTHETICAL_PROMOTION.md (NEW; 380 lines; qualitative cost analysis) - conductor/tracks/code_path_audit_20260607/spec.md (MODIFIED; +93 lines Pre-Flight Adjustments) - conductor/tracks.md (MODIFIED; +35 lines: 3 new entries + 1 stale row fix)
This commit is contained in:
@@ -305,6 +305,79 @@ This track has **no blockers** and **no conflicts**. It can ship independently o
|
||||
|
||||
This track's analysis is **read-only** — it doesn't modify `src/`, doesn't change the public API, doesn't add tests to the existing test suite. The only new files are `src/code_path_audit.py` (the tool), `tests/test_code_path_audit.py` (the tests), and the report under `docs/reports/code_path_audit/2026-06-07/`.
|
||||
|
||||
## Pre-Flight Adjustments (2026-06-21, per handoffs from `any_type_componentization_20260621`)
|
||||
|
||||
The `any_type_componentization_20260621` track (shipped 2026-06-21 with 48/89 sites promoted) revealed that **the 4 foundational tracks this audit was deferred behind have evolved**. Specifically, 5 new hot-path dataclasses (`ToolSpec`, `ChatMessage`, `UsageStats`, `ToolCall`, `WebSocketMessage`) and 1 new module (`provider_state.ProviderHistory`) now exist. This audit must instrument them.
|
||||
|
||||
**Per `docs/handoffs/PROMPT_FOR_TIER_1.md` and `HANDOFF_CODE_PATH_AUDIT_FROM_any_type_componentization.md`, the following 4 adjustments are added to this audit's scope:**
|
||||
|
||||
### A1. Add 2 new actions to the per-action profiling
|
||||
|
||||
The existing 3 actions (`ai_message_lifecycle`, `discussion_save_load`, `gui_startup`) become 5:
|
||||
|
||||
| Action | Codepath | Measures |
|
||||
|---|---|---|
|
||||
| `provider_history_append` (NEW) | `get_history(p).append(msg)` (or legacy `_anthropic_history.append(msg)`) | Per-turn append latency + lock acquire time + memory allocation per call. The hot path Phase 3 will refactor. |
|
||||
| `websocket_broadcast` (NEW) | `broadcast(WebSocketMessage(...))` (post-Phase 6a) | Per-broadcast overhead (allocation + JSON serialization + WebSocket send). The GUI thread's per-event cost. |
|
||||
| `ai_message_lifecycle` (existing) | `_send_<provider>` end-to-end | Total per-turn latency delta pre/post Phase 3 (`provider_state.ProviderHistory`). The 3 OpenAI-compatible providers (`grok`, `minimax`, `llama`) are **newly instrumented** (currently unprofiled). |
|
||||
| `discussion_save_load` (existing) | `reset_session()` + project switch | Cold-path cost. The `clear_all()` migration's per-call delta. |
|
||||
| `gui_startup` (existing) | `_PROVIDER_HISTORIES` dict init at module load | One-time init cost (6 `ProviderHistory()` instances + 6 locks). |
|
||||
|
||||
### A2. Add 5 micro-benchmarks to the audit's `optimization_candidates.md`
|
||||
|
||||
The audit's per-call cost estimates should include these 5 micro-benchmarks (added per `HANDOFF_FOLLOWUP_TRACK_FROM_any_type_componentization.md` §7):
|
||||
|
||||
| Micro-benchmark | Purpose | Expected overhead |
|
||||
|---|---|---|
|
||||
| `NormalizedResponse.__init__` | Dataclass construction vs the old 6-field dict literal | <1μs; immaterial |
|
||||
| `WebSocketMessage.__init__` | Dataclass construction per broadcast | <5μs; the hot path concern |
|
||||
| `UsageStats.__init__` | Nested dataclass construction per response | <500ns; negligible (4 int fields) |
|
||||
| `ProviderHistory.lock` acquire | threading.Lock acquire overhead | <500ns; the threading hot path |
|
||||
| `ToolSpec.__init__` | Dataclass construction per tool (45 tools, cold path) | <2μs; only at registration |
|
||||
|
||||
The benchmarks are emitted to `docs/reports/code_path_audit/<date>/micro_benchmarks.md`.
|
||||
|
||||
### A3. Add the "no-TypeError-errors-on-any-thread" assertion
|
||||
|
||||
The audit's per-action profiling runs the 5 actions in a controlled harness. The audit MUST assert that no `worker[queue_fallback] error: WebSocketServer.broadcast() takes 2 positional arguments but 3 were given` (or any TypeError on any thread) appears in the harness output during profiling.
|
||||
|
||||
This assertion catches the broadcast() regression that `any_type_componentization_20260621` introduced. The regression test that backs this assertion lives in `tests/test_websocket_broadcast_regression.py` (added by the `phase2_4_5_call_site_completion_20260621` follow-up track).
|
||||
|
||||
If the assertion fires, the audit's output should:
|
||||
1. Mark the affected action's profile as `INSTRUMENTATION_CONTAMINATED`
|
||||
2. List the offending thread + traceback in the report's `errors.md`
|
||||
3. Recommend re-running the audit AFTER `phase2_4_5_call_site_completion_20260621` merges
|
||||
|
||||
### A4. Add the 89 fat-struct sites as instrumented targets
|
||||
|
||||
The audit reads `docs/reports/ANY_TYPE_AUDIT_20260621.md` §3's table and tags each `Any` usage with `(file:line, hot_path, cold_path, init_path)`. The 89 sites become per-action cost estimates that flow into `optimization_candidates.md`.
|
||||
|
||||
For the 48 promoted sites, the audit compares pre-refactor (legacy globals + dict literals) vs post-refactor (dataclass + registry). For the 41 deferred Phase 3 sites, the audit produces per-call cost estimates that inform the future Phase 3 follow-up track (see `docs/reports/PHASE3_HYPOTHETICAL_PROMOTION.md` for the qualitative estimates).
|
||||
|
||||
### A5. Sequencing (BLOCKER)
|
||||
|
||||
**This audit is now blocked by `phase2_4_5_call_site_completion_20260621` (the broadcast() fix).** Until Phase 6a merges, the GUI thread's `worker[queue_fallback]` TypeError spam contaminates the audit's per-action profiling.
|
||||
|
||||
**Recommended sequence:**
|
||||
```
|
||||
T0: Tier 1 approves follow-up track (decision: SHRINK to 6a + 6b + 6d)
|
||||
T1: Tier 2 implements Phase 6a + 6b + 6d (~3 hours, ~16 commits)
|
||||
T2: Tier 1 reviews + merges follow-up track
|
||||
T3: Tier 1 launches code_path_audit_20260607
|
||||
T4: Tier 2 implements Phase 3 + cross-phase coupling (separate track, post-audit)
|
||||
```
|
||||
|
||||
### A6. New coordination with `any_type_componentization_20260621`
|
||||
|
||||
This audit now has **new dependencies** beyond the original 4 foundational tracks:
|
||||
|
||||
| Track | Status | Provides to this audit |
|
||||
|---|---|---|
|
||||
| `any_type_componentization_20260621` | Shipped 2026-06-21 (48/89 promoted) | The 5 dataclasses + 1 module; the 200-site dataclass-coverage baseline |
|
||||
| `phase2_4_5_call_site_completion_20260621` | Spec'd 2026-06-21; not yet merged | The fix for the broadcast() TypeError; the "no-TypeError" assertion |
|
||||
|
||||
This audit is `blocked_by` both tracks (post-merge).
|
||||
|
||||
## Follow-up
|
||||
|
||||
- **`pipeline_runtime_profiling_20260607`** (the user-requested follow-up; NOT in this track): adds a runtime profiling harness using the existing `src/performance_monitor.py` + a per-action test fixture. Measures real costs for the 3 actions. Calibrates the heuristic cost model (`EXPENSIVE_THRESHOLD` + per-class weights). Catches "things that aren't easy to resolve statically" — import cost, JIT effects, GC pauses, C-extension call cost (imgui-bundle, tree-sitter native), decorator-driven dispatch. Output: `scripts/runtime_profiler.py` + updated `code_path_audit.py` cost model.
|
||||
|
||||
@@ -0,0 +1,111 @@
|
||||
{
|
||||
"track_id": "phase2_4_5_call_site_completion_20260621",
|
||||
"name": "Phase 2/4/5 Call-Site Completion (post any_type_componentization)",
|
||||
"initialized": "2026-06-21",
|
||||
"owner": "tier2-tech-lead",
|
||||
"priority": "A",
|
||||
"status": "active",
|
||||
"type": "bugfix + refactor + test-infrastructure",
|
||||
"scope": {
|
||||
"new_files": [
|
||||
"tests/test_websocket_broadcast_regression.py",
|
||||
"docs/reports/TRACK_COMPLETION_phase2_4_5_call_site_completion_20260621.md"
|
||||
],
|
||||
"modified_files": [
|
||||
"src/app_controller.py",
|
||||
"src/events.py",
|
||||
"src/gui_2.py",
|
||||
"src/ai_client.py",
|
||||
"tests/test_grok_provider.py",
|
||||
"tests/test_minimax_provider.py",
|
||||
"tests/test_llama_provider.py"
|
||||
],
|
||||
"deleted_files": []
|
||||
},
|
||||
"blocked_by": [],
|
||||
"blocks": ["code_path_audit_20260607"],
|
||||
"estimated_phases": 3,
|
||||
"spec": "spec.md",
|
||||
"plan": "plan.md",
|
||||
"priority_order": "A (Phase 6a broadcast fix) > A (Phase 6b OpenAICompatibleRequest) > B (Phase 6d NormalizedResponse) > C (regression protocol)",
|
||||
"parent_track": {
|
||||
"id": "any_type_componentization_20260621",
|
||||
"spec": "conductor/tracks/any_type_componentization_20260621/spec.md",
|
||||
"handoff_docs": [
|
||||
"docs/handoffs/PROMPT_FOR_TIER_1.md",
|
||||
"docs/handoffs/HANDOFF_FOLLOWUP_TRACK_FROM_any_type_componentization.md",
|
||||
"docs/handoffs/HANDOFF_CODE_PATH_AUDIT_FROM_any_type_componentization.md"
|
||||
]
|
||||
},
|
||||
"phases": {
|
||||
"phase_6a": {
|
||||
"name": "Fix HookServer.broadcast() callers",
|
||||
"scope": "Migrate broadcast(channel, payload) callers in app_controller.py + events.py + gui_2.py to broadcast(WebSocketMessage(...))",
|
||||
"estimated_commits": 7,
|
||||
"new_test_file": "tests/test_websocket_broadcast_regression.py"
|
||||
},
|
||||
"phase_6b": {
|
||||
"name": "Complete OpenAICompatibleRequest migration",
|
||||
"scope": "_send_grok + _send_minimax + _send_llama construct OpenAICompatibleRequest(messages=[ChatMessage(...)])",
|
||||
"estimated_commits": 5
|
||||
},
|
||||
"phase_6d": {
|
||||
"name": "Update NormalizedResponse construction",
|
||||
"scope": "Same 3 senders: usage_input_tokens/etc -> usage=UsageStats(...)",
|
||||
"estimated_commits": 4
|
||||
}
|
||||
},
|
||||
"total_estimated_commits": 16,
|
||||
"deferred_work": {
|
||||
"phase_3_provider_state": {
|
||||
"deferred_to": "separate track post code_path_audit_20260607",
|
||||
"rationale": "Phase 3 has runtime hot-path concerns (per-LLM-turn history manipulation); the code_path_audit should measure cost BEFORE the refactor",
|
||||
"estimated_sites": 112,
|
||||
"estimation_method": "grep -c '_<provider>_history(?!_)' on src/ai_client.py per HANDOFF_CODE_PATH_AUDIT"
|
||||
},
|
||||
"cross_phase_coupling": {
|
||||
"deferred_to": "separate track",
|
||||
"rationale": "OpenAICompatibleRequest.tools: list[dict[str, Any]] -> list[ToolSpec] is a follow-up"
|
||||
},
|
||||
"audit_tier2_leaks_fix": {
|
||||
"deferred_to": "infrastructure track",
|
||||
"rationale": "3 sandbox-pollution failures; need --allowlist for mcp_paths.toml, opencode.json, .opencode/*"
|
||||
},
|
||||
"pre_existing_gui2_parity_flake": {
|
||||
"deferred_to": "investigation",
|
||||
"rationale": "test_gui2_custom_callback_hook_works flake; not introduced by this track"
|
||||
}
|
||||
},
|
||||
"unblocks": {
|
||||
"code_path_audit_20260607": "TypeError spam from broadcast() contaminates per-action profiling; Phase 6a fixes the underlying regression"
|
||||
},
|
||||
"verification_criteria": [
|
||||
"src/app_controller.py:_run_pending_tasks_once_result uses broadcast(WebSocketMessage(...))",
|
||||
"src/events.py broadcast callers use WebSocketMessage",
|
||||
"src/gui_2.py:_process_pending_gui_tasks broadcast callers use WebSocketMessage",
|
||||
"tests/test_websocket_broadcast_regression.py exists; asserts no broadcast() TypeError",
|
||||
"_send_grok constructs OpenAICompatibleRequest(messages=[ChatMessage(...)], ...)",
|
||||
"_send_minimax constructs OpenAICompatibleRequest(messages=[ChatMessage(...)], ...)",
|
||||
"_send_llama constructs OpenAICompatibleRequest(messages=[ChatMessage(...)], ...)",
|
||||
"_send_grok constructs NormalizedResponse(text=..., usage=UsageStats(...), ...)",
|
||||
"_send_minimax constructs NormalizedResponse(text=..., usage=UsageStats(...), ...)",
|
||||
"_send_llama constructs NormalizedResponse(text=..., usage=UsageStats(...), ...)",
|
||||
"All 11-tier batched test run passes (no stop-on-failure)",
|
||||
"audit_weak_types.py --strict exits 0",
|
||||
"audit_dataclass_coverage.py --strict exits 0",
|
||||
"End-of-track report at docs/reports/TRACK_COMPLETION_phase2_4_5_call_site_completion_20260621.md"
|
||||
],
|
||||
"sequencing_note": "This track unblocks code_path_audit_20260607. Run this track first; after merge, run the audit. The Phase 3 follow-up track runs AFTER the audit completes.",
|
||||
"ai_performance_analysis": {
|
||||
"win": "Fixes 1 runtime bug (broadcast() TypeError) + completes the Phase 2/5 migration for 3 senders (grok/minimax/llama). Makes code_path_audit_20260607 instrumentable.",
|
||||
"cost": "~16 commits; ~3 hours Tier 2.",
|
||||
"caveat": "The deferred Phase 3 (112 sites in ai_client.py) is still the biggest remaining work. The audit will quantify the cost before Phase 3 is migrated.",
|
||||
"honest_assessment": "Tight, focused track. Fits Tier 2's 1-4 hour budget. Unblocks the audit without ballooning scope."
|
||||
},
|
||||
"links": {
|
||||
"parent_track": "conductor/tracks/any_type_componentization_20260621/",
|
||||
"audit_track": "conductor/tracks/code_path_audit_20260607/",
|
||||
"phase3_hypothetical_analysis": "docs/reports/PHASE3_HYPOTHETICAL_PROMOTION.md",
|
||||
"handoff_docs": "docs/handoffs/"
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,508 @@
|
||||
# Phase 2/4/5 Call-Site Completion Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Fix the `HookServer.broadcast()` runtime bug + complete the Phase 2 `_send_grok` / `_send_minimax` / `_send_llama` migration to `OpenAICompatibleRequest(messages=[ChatMessage(...)])` and `NormalizedResponse(usage=UsageStats(...))`. Adds `tests/test_websocket_broadcast_regression.py` with a "no-TypeError-errors-on-any-thread" assertion that `code_path_audit_20260607` will reuse.
|
||||
|
||||
**Architecture:** 3 phases (Phase 6a + 6b + 6d). Phase 6a is the runtime bug fix (broadcast callers in 3 files). Phase 6b completes the t2_6 deferred OpenAI-compatible sender migration. Phase 6d updates those senders' `NormalizedResponse` to use `UsageStats`. No new modules; only consumer migration + 1 new regression test file.
|
||||
|
||||
**Tech Stack:** Python 3.11+ stdlib. Existing `src/openai_schemas.py` (Phase 2 of parent track) provides `ChatMessage`, `UsageStats`, `ToolCall`. Existing `src/api_hooks.py` (Phase 5 of parent track) provides `WebSocketMessage`.
|
||||
|
||||
**Reference Files:**
|
||||
- `docs/handoffs/PROMPT_FOR_TIER_1.md` — Tier 1 brief
|
||||
- `docs/handoffs/HANDOFF_FOLLOWUP_TRACK_FROM_any_type_componentization.md` — test failure categorization
|
||||
- `docs/handoffs/HANDOFF_CODE_PATH_AUDIT_FROM_any_type_componentization.md` — runtime cost framing
|
||||
- `conductor/tracks/phase2_4_5_call_site_completion_20260621/spec.md` — the design
|
||||
- `conductor/tracks/any_type_componentization_20260621/spec.md` — parent track
|
||||
- `src/openai_schemas.py` — ChatMessage + UsageStats + NormalizedResponse + OpenAICompatibleRequest
|
||||
- `src/api_hooks.py` — WebSocketMessage + HookServer.broadcast
|
||||
|
||||
**Code Style:** 1-space indentation, CRLF line endings, no comments in source code, type hints mandatory (per `conductor/workflow.md` Code Style section).
|
||||
|
||||
---
|
||||
|
||||
## File Structure
|
||||
|
||||
```
|
||||
src/
|
||||
app_controller.py # MODIFIED (Phase 6a): _run_pending_tasks_once_result broadcast callers
|
||||
events.py # MODIFIED (Phase 6a): broadcast callers
|
||||
gui_2.py # MODIFIED (Phase 6a): _process_pending_gui_tasks broadcast callers
|
||||
ai_client.py # MODIFIED (Phase 6b+6d): _send_grok/_send_minimax/_send_llama
|
||||
api_hooks.py # UNCHANGED (the broadcast() change is correct)
|
||||
|
||||
tests/
|
||||
test_websocket_broadcast_regression.py # NEW (Phase 6a): no-TypeError assertion
|
||||
test_grok_provider.py # MODIFIED (Phase 6b+6d): verify ChatMessage + UsageStats
|
||||
test_minimax_provider.py # MODIFIED (Phase 6b+6d): verify ChatMessage + UsageStats
|
||||
test_llama_provider.py # MODIFIED (Phase 6b+6d): verify ChatMessage + UsageStats
|
||||
|
||||
docs/reports/
|
||||
TRACK_COMPLETION_phase2_4_5_call_site_completion_20260621.md # NEW (verify)
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Phase 6a: Fix HookServer.broadcast() Callers
|
||||
|
||||
Focus: Replace `broadcast(channel, payload)` with `broadcast(WebSocketMessage(channel=, payload=))` at all internal call sites in `src/`.
|
||||
|
||||
### Task 6a.1: Catalog all broadcast() callers
|
||||
|
||||
**Files:**
|
||||
- Search: `src/app_controller.py`, `src/events.py`, `src/gui_2.py`
|
||||
|
||||
- [ ] **Step 1: Grep for all internal callers**
|
||||
|
||||
Run: `Select-String -Path src/app_controller.py,src/events.py,src/gui_2.py -Pattern '\.broadcast\('`
|
||||
Expected: 5-10 sites (per HANDOFF_FOLLOWUP §5: app_controller.py:_run_pending_tasks_once_result 1-3, events.py 1-3, gui_2.py 1-3)
|
||||
|
||||
- [ ] **Step 2: Document the list**
|
||||
|
||||
For each call site, record `(file:line, current_call_signature, replacement_call_signature)` in your working notes. Example:
|
||||
- `src/app_controller.py:N broadcast(channel_str, payload_dict)` → `broadcast(WebSocketMessage(channel=channel_str, payload=payload_dict))`
|
||||
|
||||
### Task 6a.2: Write failing regression test
|
||||
|
||||
**Files:**
|
||||
- Create: `tests/test_websocket_broadcast_regression.py`
|
||||
|
||||
- [ ] **Step 1: Write the test**
|
||||
|
||||
```python
|
||||
"""Regression test for the HookServer.broadcast() runtime TypeError bug.
|
||||
|
||||
This test ensures that no internal caller of HookServer.broadcast() passes
|
||||
the OLD (channel, payload) signature after Phase 5 changed it to
|
||||
(message: WebSocketMessage). The audit (code_path_audit_20260607) reuses
|
||||
this assertion.
|
||||
"""
|
||||
import asyncio
|
||||
import sys
|
||||
from src.api_hooks import WebSocketMessage
|
||||
|
||||
|
||||
def test_broadcast_accepts_websocket_message() -> None:
|
||||
"""HookServer.broadcast must accept a single WebSocketMessage argument."""
|
||||
from src.api_hooks import HookServer
|
||||
import inspect
|
||||
sig = inspect.signature(HookServer.broadcast)
|
||||
params = list(sig.parameters.keys())
|
||||
# self + 1 positional arg
|
||||
assert len(params) == 2, f"expected 2 params (self + message), got {len(params)}: {params}"
|
||||
|
||||
|
||||
def test_broadcast_rejects_legacy_2arg_call() -> None:
|
||||
"""Calling broadcast with 2 positional args (legacy signature) must raise TypeError."""
|
||||
from src.api_hooks import HookServer
|
||||
server = HookServer()
|
||||
try:
|
||||
server.broadcast("channel", {"key": "value"})
|
||||
except TypeError as e:
|
||||
assert "takes 2 positional arguments" in str(e) or "takes 1 positional argument" in str(e)
|
||||
return
|
||||
assert False, "broadcast should reject legacy 2-arg call"
|
||||
|
||||
|
||||
def test_internal_callers_use_websocket_message_signature() -> None:
|
||||
"""Grep all internal callers of broadcast() and assert they use the new signature."""
|
||||
import subprocess
|
||||
result = subprocess.run(
|
||||
["grep", "-rn", r"\.broadcast\(", "src/"],
|
||||
capture_output=True, text=True,
|
||||
)
|
||||
lines = [l for l in result.stdout.split("\n") if l and "tests/" not in l]
|
||||
for line in lines:
|
||||
file, lineno, content = line.split(":", 2)
|
||||
# The new signature is broadcast(WebSocketMessage(...))
|
||||
# The old signature is broadcast("string", {...})
|
||||
if "WebSocketMessage(" not in content and 'broadcast("' in content:
|
||||
assert False, f"{file}:{lineno} uses legacy signature: {content.strip()}"
|
||||
|
||||
|
||||
def test_no_typeerror_during_gui_task_processing() -> None:
|
||||
"""Smoke test: simulate a GUI task that triggers broadcast; assert no TypeError on any thread."""
|
||||
import logging
|
||||
import io
|
||||
# Capture stderr to detect worker[queue_fallback] error spam
|
||||
captured = io.StringIO()
|
||||
handler = logging.StreamHandler(captured)
|
||||
handler.setLevel(logging.ERROR)
|
||||
logging.getLogger().addHandler(handler)
|
||||
try:
|
||||
# Trigger a task that would have hit the broadcast bug
|
||||
# (This is a structural test — the actual GUI thread simulation is in live_gui tests)
|
||||
import asyncio
|
||||
from src.api_hooks import HookServer, WebSocketMessage
|
||||
server = HookServer()
|
||||
msg = WebSocketMessage(channel="test", payload={"key": "value"})
|
||||
server.broadcast(msg) # must not raise
|
||||
finally:
|
||||
logging.getLogger().removeHandler(handler)
|
||||
stderr_output = captured.getvalue()
|
||||
assert "WebSocketServer.broadcast()" not in stderr_output, f"TypeError detected: {stderr_output}"
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run test to verify first one fails**
|
||||
|
||||
Run: `uv run pytest tests/test_websocket_broadcast_regression.py -v`
|
||||
Expected: The first test passes (the signature is already `(self, message)`); the second passes (legacy call raises); the THIRD may FAIL (internal callers still use old signature — that's what we're fixing); the fourth passes (the smoke test).
|
||||
|
||||
### Task 6a.3: Fix `src/app_controller.py:_run_pending_tasks_once_result` broadcast callers
|
||||
|
||||
- [ ] **Step 1: Find the call sites**
|
||||
|
||||
Run: `Select-String -Path src/app_controller.py -Pattern '\.broadcast\('`
|
||||
Expected: 1-3 lines in `_run_pending_tasks_once_result`
|
||||
|
||||
- [ ] **Step 2: For each call site, replace**
|
||||
|
||||
Old:
|
||||
```python
|
||||
self.web_socket_server.broadcast(channel_str, payload_dict)
|
||||
```
|
||||
|
||||
New:
|
||||
```python
|
||||
from src.api_hooks import WebSocketMessage
|
||||
self.web_socket_server.broadcast(WebSocketMessage(channel=channel_str, payload=payload_dict))
|
||||
```
|
||||
|
||||
(Add the import at the top of the function or file if not already present.)
|
||||
|
||||
- [ ] **Step 3: Run regression test**
|
||||
|
||||
Run: `uv run pytest tests/test_websocket_broadcast_regression.py::test_internal_callers_use_websocket_message_signature -v`
|
||||
Expected: should fail for events.py + gui_2.py still; pass for app_controller.py
|
||||
|
||||
### Task 6a.4: Fix `src/events.py` broadcast callers
|
||||
|
||||
- [ ] **Step 1: Find call sites**
|
||||
|
||||
Run: `Select-String -Path src/events.py -Pattern '\.broadcast\('`
|
||||
|
||||
- [ ] **Step 2: Replace each with `WebSocketMessage(...)` wrapper**
|
||||
|
||||
- [ ] **Step 3: Run regression test**
|
||||
|
||||
Run: `uv run pytest tests/test_websocket_broadcast_regression.py::test_internal_callers_use_websocket_message_signature -v`
|
||||
|
||||
### Task 6a.5: Fix `src/gui_2.py:_process_pending_gui_tasks` broadcast callers
|
||||
|
||||
- [ ] **Step 1: Find call sites**
|
||||
|
||||
Run: `Select-String -Path src/gui_2.py -Pattern '\.broadcast\('`
|
||||
|
||||
- [ ] **Step 2: Replace each with `WebSocketMessage(...)` wrapper**
|
||||
|
||||
- [ ] **Step 3: Run regression test**
|
||||
|
||||
Run: `uv run pytest tests/test_websocket_broadcast_regression.py -v`
|
||||
Expected: all 4 tests pass
|
||||
|
||||
### Task 6a.6: Run tier-1-unit-core FULLY per the regression protocol
|
||||
|
||||
- [ ] **Step 1: Run the full tier-1-unit-core tier (no stop-on-failure)**
|
||||
|
||||
Run: `uv run python scripts/run_tests_batched.py --tier tier-1-unit-core`
|
||||
Expected: all PASS (the "no-TypeError" assertion catches the broadcast bug; any other regressions surface)
|
||||
|
||||
### Task 6a.7: Phase 6a checkpoint
|
||||
|
||||
- [ ] **Step 1: Commit**
|
||||
|
||||
```bash
|
||||
git add src/app_controller.py src/events.py src/gui_2.py tests/test_websocket_broadcast_regression.py
|
||||
git commit -m "fix(broadcast): migrate HookServer.broadcast() callers to WebSocketMessage signature
|
||||
|
||||
Phase 5 of any_type_componentization_20260621 changed
|
||||
HookServer.broadcast(channel, payload) -> broadcast(message: WebSocketMessage)
|
||||
but did not update internal callers in app_controller.py, events.py, gui_2.py.
|
||||
This produced worker[queue_fallback] TypeError spam on the GUI thread.
|
||||
|
||||
Fix: wrap each call site with WebSocketMessage(channel=, payload=).
|
||||
Adds tests/test_websocket_broadcast_regression.py with a no-TypeError assertion
|
||||
that code_path_audit_20260607 will reuse."
|
||||
git notes add -m "Phase 6a checkpoint: broadcast() TypeError fixed; 4 regression tests added; tier-1-unit-core passes FULLY" HEAD
|
||||
```
|
||||
|
||||
Update `conductor/tracks/phase2_4_5_call_site_completion_20260621/state.toml` to mark phase_6a status="completed" + checkpointsha.
|
||||
|
||||
---
|
||||
|
||||
## Phase 6b: Complete `_send_grok` / `_send_minimax` / `_send_llama` OpenAICompatibleRequest Migration
|
||||
|
||||
Focus: Migrate the 3 OpenAI-compatible senders in `src/ai_client.py` to construct `OpenAICompatibleRequest(messages=[ChatMessage(...)])` instead of `messages=[{"role": ..., "content": ...}]`.
|
||||
|
||||
### Task 6b.1: Identify existing provider tests
|
||||
|
||||
- [ ] **Step 1: Check for provider-specific test files**
|
||||
|
||||
Run: `Get-ChildItem tests/test_*provider*.py 2>&1 | Select-String -Pattern 'grok|minimax|llama'`
|
||||
Expected: at least one of `tests/test_grok_provider.py`, `tests/test_minimax_provider.py`, `tests/test_llama_provider.py`; if any are missing, add a smoke test (Task 6b.1b).
|
||||
|
||||
- [ ] **Step 1b: (if any missing) Add smoke test**
|
||||
|
||||
For each missing provider, create `tests/test_<provider>_provider.py`:
|
||||
```python
|
||||
"""Smoke tests for the OpenAI-compatible _send_<provider> path."""
|
||||
def test_<provider>_sends_chat_message() -> None:
|
||||
"""Verify _send_<provider> constructs OpenAICompatibleRequest with ChatMessage."""
|
||||
from src.ai_client import _send_<provider>
|
||||
import inspect
|
||||
src = inspect.getsource(_send_<provider>)
|
||||
# Old signature: messages=[{"role": ...
|
||||
# New signature: messages=[ChatMessage(...
|
||||
assert "ChatMessage" in src or 'messages=[ChatMessage' in src, f"_send_<provider} still uses legacy dict shape"
|
||||
```
|
||||
|
||||
### Task 6b.2: Write failing tests for ChatMessage in OpenAICompatibleRequest construction
|
||||
|
||||
**Files:**
|
||||
- Modify: each provider test file
|
||||
|
||||
For each provider, add:
|
||||
```python
|
||||
def test_<provider>_constructs_openai_compatible_request_with_chat_message() -> None:
|
||||
"""_send_<provider> must use ChatMessage, not dict literals."""
|
||||
from src.openai_schemas import OpenAICompatibleRequest, ChatMessage
|
||||
# Mock the underlying API call; just verify the shape
|
||||
# (Actual call is too expensive for a unit test)
|
||||
import inspect
|
||||
src = inspect.getsource(_send_<provider>)
|
||||
# Look for the OpenAICompatibleRequest instantiation
|
||||
assert "OpenAICompatibleRequest" in src
|
||||
# Look for ChatMessage usage (not legacy dict shape)
|
||||
assert "ChatMessage(" in src, f"_send_<provider} still uses legacy dict shape"
|
||||
assert 'messages=[{"role"' not in src, f"_send_<provider} still uses legacy dict shape"
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run tests to verify they fail**
|
||||
|
||||
Run: `uv run pytest tests/test_grok_provider.py tests/test_minimax_provider.py tests/test_llama_provider.py -v`
|
||||
Expected: FAIL (the 3 senders still use `messages=[{"role": ..., "content": ...}]`)
|
||||
|
||||
### Task 6b.3: Migrate `src/ai_client.py:_send_grok` (L2532)
|
||||
|
||||
- [ ] **Step 1: Read the current implementation**
|
||||
|
||||
Run: `Get-Content src/ai_client.py | Select-Object -Skip 2530 -First 80`
|
||||
|
||||
- [ ] **Step 2: Add ChatMessage import + replace dict construction**
|
||||
|
||||
At the top of `_send_grok`:
|
||||
```python
|
||||
from src.openai_schemas import ChatMessage, NormalizedResponse, OpenAICompatibleRequest, UsageStats
|
||||
```
|
||||
|
||||
Replace each `messages=[{"role": ..., "content": ...}]` with `messages=[ChatMessage(role=..., content=...)]`.
|
||||
|
||||
- [ ] **Step 3: Run grok test**
|
||||
|
||||
Run: `uv run pytest tests/test_grok_provider.py -v`
|
||||
|
||||
### Task 6b.4: Migrate `src/ai_client.py:_send_minimax` (L2616)
|
||||
|
||||
Same pattern as Task 6b.3.
|
||||
|
||||
### Task 6b.5: Migrate `src/ai_client.py:_send_llama` (L2856)
|
||||
|
||||
Same pattern as Task 6b.3.
|
||||
|
||||
### Task 6b.6: Run tier-1-unit-core + provider tests FULLY
|
||||
|
||||
- [ ] **Step 1: Run the tests**
|
||||
|
||||
Run: `uv run python scripts/run_tests_batched.py --tier tier-1-unit-core`
|
||||
Expected: all PASS
|
||||
|
||||
Run: `uv run pytest tests/test_grok_provider.py tests/test_minimax_provider.py tests/test_llama_provider.py -v`
|
||||
Expected: all PASS
|
||||
|
||||
### Task 6b.7: Phase 6b checkpoint
|
||||
|
||||
```bash
|
||||
git add src/ai_client.py tests/test_grok_provider.py tests/test_minimax_provider.py tests/test_llama_provider.py
|
||||
git commit -m "refactor(ai_client): migrate _send_grok/_send_minimax/_send_llama to ChatMessage API
|
||||
|
||||
Completes the deferred t2_6 task from any_type_componentization_20260621 Phase 2.
|
||||
The 3 OpenAI-compatible senders now construct OpenAICompatibleRequest with
|
||||
messages=[ChatMessage(role=, content=)] instead of messages=[dict] literals."
|
||||
git notes add -m "Phase 6b checkpoint: 3 senders migrated to ChatMessage API" HEAD
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Phase 6d: Update Those Senders' `NormalizedResponse` Construction
|
||||
|
||||
Focus: Replace `NormalizedResponse(text=..., usage_input_tokens=X, usage_output_tokens=Y, ...)` with `NormalizedResponse(text=..., usage=UsageStats(input_tokens=X, ...))` in the 3 OpenAI-compatible senders.
|
||||
|
||||
### Task 6d.1: Write failing tests for UsageStats in NormalizedResponse
|
||||
|
||||
For each provider test:
|
||||
```python
|
||||
def test_<provider>_constructs_normalized_response_with_usage_stats() -> None:
|
||||
"""_send_<provider> must use UsageStats, not separate int fields."""
|
||||
import inspect
|
||||
src = inspect.getsource(_send_<provider>)
|
||||
# Look for the old kwargs (4 separate int fields)
|
||||
assert "usage_input_tokens=" not in src, f"_send_<provider} still uses legacy usage_XXX fields"
|
||||
# Look for the new UsageStats field
|
||||
assert "usage=UsageStats(" in src or "usage=UsageStats " in src
|
||||
```
|
||||
|
||||
- [ ] **Step 1: Run tests to verify they fail**
|
||||
|
||||
Run: `uv run pytest tests/test_grok_provider.py tests/test_minimax_provider.py tests/test_llama_provider.py -v`
|
||||
Expected: FAIL on the 3 new tests
|
||||
|
||||
### Task 6d.2-6d.4: Migrate each sender's `NormalizedResponse` construction
|
||||
|
||||
For each of `_send_grok`, `_send_minimax`, `_send_llama`:
|
||||
|
||||
- [ ] **Step 1: Find the `NormalizedResponse(...)` construction**
|
||||
|
||||
- [ ] **Step 2: Replace 4 separate int fields with `UsageStats(...)`**
|
||||
|
||||
Old:
|
||||
```python
|
||||
NormalizedResponse(
|
||||
text=text,
|
||||
tool_calls=(),
|
||||
usage_input_tokens=in_tok,
|
||||
usage_output_tokens=out_tok,
|
||||
usage_cache_read_tokens=cache_read,
|
||||
usage_cache_creation_tokens=cache_create,
|
||||
raw_response=raw,
|
||||
)
|
||||
```
|
||||
|
||||
New:
|
||||
```python
|
||||
NormalizedResponse(
|
||||
text=text,
|
||||
tool_calls=(),
|
||||
usage=UsageStats(
|
||||
input_tokens=in_tok,
|
||||
output_tokens=out_tok,
|
||||
cache_read_tokens=cache_read,
|
||||
cache_creation_tokens=cache_create,
|
||||
),
|
||||
raw_response=raw,
|
||||
)
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Run provider test**
|
||||
|
||||
Run: `uv run pytest tests/test_<provider>_provider.py -v`
|
||||
|
||||
### Task 6d.5: Run ALL 11 tiers FULLY per regression protocol
|
||||
|
||||
- [ ] **Step 1: Run the full batched suite**
|
||||
|
||||
Run: `uv run python scripts/run_tests_batched.py`
|
||||
Expected: all 11 tiers PASS (no stop-on-failure per the regression protocol)
|
||||
|
||||
### Task 6d.6: Phase 6d checkpoint
|
||||
|
||||
```bash
|
||||
git add src/ai_client.py tests/test_grok_provider.py tests/test_minimax_provider.py tests/test_llama_provider.py
|
||||
git commit -m "refactor(ai_client): migrate _send_grok/_send_minimax/_send_llama NormalizedResponse to UsageStats
|
||||
|
||||
Completes the NormalizedResponse migration for the 3 OpenAI-compatible senders.
|
||||
They now construct UsageStats(input_tokens=, output_tokens=, cache_read_tokens=,
|
||||
cache_creation_tokens=) instead of 4 separate int fields."
|
||||
git notes add -m "Phase 6d checkpoint: 3 senders use UsageStats; all 11 tiers pass FULLY" HEAD
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Verify + Archive
|
||||
|
||||
### Task V.1: Run all audits
|
||||
|
||||
```bash
|
||||
uv run python scripts/audit_weak_types.py --strict
|
||||
uv run python scripts/audit_dataclass_coverage.py --strict
|
||||
uv run python scripts/generate_type_registry.py --check
|
||||
```
|
||||
Expected: all exit 0
|
||||
|
||||
### Task V.2: Write end-of-track report
|
||||
|
||||
Create `docs/reports/TRACK_COMPLETION_phase2_4_5_call_site_completion_20260621.md` covering:
|
||||
- Executive summary (16 commits; 3 phases; the broadcast() fix; the 3 OpenAI-compatible senders migrated)
|
||||
- The broadcast() TypeError bug (root cause + fix)
|
||||
- The Phase 2 migration completion (3 senders now use ChatMessage + UsageStats)
|
||||
- The regression protocol (run all 11 tiers FULLY; the no-TypeError assertion)
|
||||
- Verification commands + results
|
||||
- What's still deferred (Phase 3 + cross-phase coupling + sandbox fixes)
|
||||
- Follow-up: code_path_audit_20260607 (now unblocked)
|
||||
|
||||
```bash
|
||||
git add docs/reports/TRACK_COMPLETION_phase2_4_5_call_site_completion_20260621.md
|
||||
git commit -m "docs(reports): TRACK_COMPLETION_phase2_4_5_call_site_completion_20260621"
|
||||
```
|
||||
|
||||
### Task V.3: Archive + tracks.md update
|
||||
|
||||
```bash
|
||||
git mv conductor/tracks/phase2_4_5_call_site_completion_20260621 conductor/tracks/archive/
|
||||
```
|
||||
|
||||
Update `conductor/tracks.md` to move the entry to "Recently Completed."
|
||||
|
||||
Update `state.toml` to mark all phases completed.
|
||||
|
||||
```bash
|
||||
git add -A
|
||||
git commit -m "conductor(archive): ship phase2_4_5_call_site_completion_20260621 to archive"
|
||||
git notes add -m "TRACK COMPLETE: phase2_4_5_call_site_completion_20260621. broadcast() TypeError fixed; 3 OpenAI-compatible senders migrated to ChatMessage + UsageStats; test_websocket_broadcast_regression.py added with no-TypeError assertion. Unblocks code_path_audit_20260607." HEAD
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Self-Review
|
||||
|
||||
**1. Spec coverage check:** Every section in `spec.md` maps to a task in this plan.
|
||||
|
||||
| Spec section | Plan coverage |
|
||||
|---|---|
|
||||
| §1 Overview | Background; goal stated at top of plan |
|
||||
| §2 Goals (A/A/B/C/D) | Phase 6a (A: broadcast) + Phase 6b (A: OpenAICompatibleRequest) + Phase 6d (B: NormalizedResponse) + regression protocol across all phases |
|
||||
| §3 Architecture | §3.1-3.3 → Phase 6a (broadcast fix) + Phase 6b-6d (sender migration) |
|
||||
| §4 Per-Phase Plan | Phase 6a (Tasks 6a.1-6a.7) + Phase 6b (Tasks 6b.1-6b.7) + Phase 6d (Tasks 6d.1-6d.6) |
|
||||
| §5 Configuration | No new deps (consistent throughout) |
|
||||
| §6 Testing Strategy | Each Phase has tests; regression protocol task V.5 |
|
||||
| §7 Migration / Rollout | 3 phases × ~5 commits each = ~16 atomic commits |
|
||||
| §8 Risks | Addressed via regression protocol + Tier 1 audit-base verification |
|
||||
| §9 Out of Scope | Phase 3 + cross-phase coupling + sandbox fixes + flake: documented as deferred |
|
||||
| §10 Verification Criteria | All 14 items covered in tasks V.1-V.3 + per-phase tests |
|
||||
|
||||
**2. Placeholder scan:** No "TBD", "TODO", "fill in details" in actionable steps.
|
||||
|
||||
**3. Type consistency:** `WebSocketMessage`, `ChatMessage`, `UsageStats`, `NormalizedResponse`, `OpenAICompatibleRequest` used consistently with the parent track's `src/openai_schemas.py` + `src/api_hooks.py`.
|
||||
|
||||
**4. Ambiguity:** Step descriptions are concrete (specific file:line refs, full code blocks, exact verification commands).
|
||||
|
||||
---
|
||||
|
||||
## Execution Handoff
|
||||
|
||||
Plan complete and saved to `conductor/tracks/phase2_4_5_call_site_completion_20260621/plan.md`.
|
||||
|
||||
**Tier 2 autonomous sandbox command:**
|
||||
```
|
||||
/tier-2-auto-execute phase2_4_5_call_site_completion_20260621
|
||||
```
|
||||
(or `uv run python scripts/mma_exec.py --role tier2-autonomous --track phase2_4_5_call_site_completion_20260621`)
|
||||
|
||||
**Pre-flight:**
|
||||
1. Tier 2 creates `tier2/phase2_4_5_call_site_completion_20260621` branch from `master`
|
||||
2. Phase 6a starts immediately (the broadcast() bug fix is the unblocker for the audit)
|
||||
3. After Phase 6a lands: run `tier-1-unit-core` FULLY per the regression protocol
|
||||
4. After all phases: archive + end-of-track report
|
||||
5. Tier 1 reviews + merges
|
||||
6. After merge: launch `code_path_audit_20260607` (the audit's pre-flight adjustments are committed; it can start)
|
||||
|
||||
**Estimated runtime:** ~3 hours Tier 2 work; ~16 atomic commits; 3 phases with checkpoint commits.
|
||||
@@ -0,0 +1,206 @@
|
||||
# Track: Phase 2/4/5 Call-Site Completion (post `any_type_componentization_20260621`)
|
||||
|
||||
**Status:** Active (spec approved 2026-06-21)
|
||||
**Initialized:** 2026-06-21
|
||||
**Owner:** Tier 2 Tech Lead (autonomous sandbox recommended)
|
||||
**Priority:** A (blocks `code_path_audit_20260607`; runtime TypeError pollutes audit instrumentation)
|
||||
|
||||
---
|
||||
|
||||
## 1. Overview
|
||||
|
||||
The `any_type_componentization_20260621` track shipped 48 of 89 fat-struct promotions across 6 phases but **deferred Phase 3** (41 `ProviderHistory` call sites in `src/ai_client.py`) and **left 1 runtime bug**: the Phase 5 `HookServer.broadcast()` signature change (from `(channel, payload)` → `(message: WebSocketMessage)`) was not propagated to internal callers in `src/app_controller.py` and `src/events.py`. This produces `worker[queue_fallback] error: WebSocketServer.broadcast() takes 2 positional arguments but 3 were given` spam on the GUI thread.
|
||||
|
||||
**Tier 1's decision (per `docs/handoffs/PROMPT_FOR_TIER_1.md`):** **SHINK** the follow-up to **Phases 6a + 6b + 6d** only. Defer Phase 3 (`provider_state` call-site migration) to a separate track after `code_path_audit_20260607` provides runtime cost data.
|
||||
|
||||
**This track does 3 things:**
|
||||
1. **Phase 6a** — Fix the runtime bug: migrate `HookServer.broadcast()` callers to the new `WebSocketMessage` signature. Adds a "no-TypeError-errors-on-any-thread" regression test that `code_path_audit_20260607` will reuse.
|
||||
2. **Phase 6b** — Complete the Phase 2 t2_6 deferred task: migrate `_send_grok` / `_send_minimax` / `_send_llama` to construct `OpenAICompatibleRequest(messages=[ChatMessage(...)], ...)` instead of the legacy `messages=[{"role": ..., "content": ...}]` shape. The 3 OpenAI-compatible providers are currently unprofiled and untyped at the call site.
|
||||
3. **Phase 6d** — Update those 3 senders' `NormalizedResponse(text=..., usage_input_tokens=..., ...)` construction to `NormalizedResponse(text=..., usage=UsageStats(...))` (the dataclass signature change from Phase 2).
|
||||
|
||||
**Phase 6c (full ProviderHistory migration in `ai_client.py`) is explicitly OUT OF SCOPE.** It gets its own track after `code_path_audit_20260607` produces per-action cost data.
|
||||
|
||||
## 2. Goals (Priority Order)
|
||||
|
||||
| Priority | Goal | Why |
|
||||
|---|---|---|
|
||||
| **A (blocker)** | Phase 6a: Fix `HookServer.broadcast()` callers; no TypeError spam | Unblocks `code_path_audit_20260607` (TypeError spam contaminates per-action timing) |
|
||||
| **A (blocker)** | Phase 6b: Complete `_send_grok` / `_send_minimax` / `_send_llama` `OpenAICompatibleRequest` migration | The 3 OpenAI-compatible providers were skipped in Phase 2; they're now the only un-migrated senders |
|
||||
| **B (consistency)** | Phase 6d: Update those 3 senders' `NormalizedResponse` to use `UsageStats` | Mirrors the migration done for `_send_anthropic` and the openai_compatible.py internal functions |
|
||||
| **C (audit-input)** | Establish a regression protocol: after any Phase-style refactor, run the FULL `tier-1-unit-core` tier, not targeted tests | The 10 test failures in `any_type_componentization_20260621` came from running targeted tests instead of the full tier |
|
||||
| **D (audit-input)** | Add a "no-TypeError-errors-on-any-thread" assertion that `code_path_audit_20260607` will reuse | The assertion catches the broadcast() regression in any future Phase-style refactor |
|
||||
|
||||
### 2.1 Non-Goals (this track)
|
||||
|
||||
- **NOT** migrating the 41 `_<provider>_history` call sites in `src/ai_client.py` to `provider_state.get_history('anthropic')`. Phase 3 deferred to a separate track post-audit.
|
||||
- **NOT** the cross-phase coupling fix (`OpenAICompatibleRequest.tools: list[dict[str, Any]]` → `list[ToolSpec]`). Deferred.
|
||||
- **NOT** the `audit_tier2_leaks.py` 3 sandbox-pollution failures. The user's `tier2/` sandbox harness modifies `mcp_paths.toml` + `opencode.json` + `.opencode/*`; the audit script needs an `--allowlist` for these (separate infra track).
|
||||
- **NOT** the pre-existing `test_gui2_custom_callback_hook_works` flake. Pre-existing; not introduced by this track.
|
||||
- **NOT** merging the `tier2/any_type_componentization_20260621` branch. Per Tier 2's recommendation, the branch stays as reconnaissance input; this track cherry-picks only the fixes, not the full branch.
|
||||
|
||||
## 3. Architecture
|
||||
|
||||
### 3.1 The Bug: Phase 5's `broadcast()` signature change
|
||||
|
||||
Phase 5 commit `e9fa69dd` refactored `HookServer.broadcast()`:
|
||||
|
||||
```python
|
||||
# BEFORE Phase 5
|
||||
def broadcast(self, channel: str, payload: dict[str, Any]) -> None:
|
||||
...
|
||||
|
||||
# AFTER Phase 5 (src/api_hooks.py)
|
||||
def broadcast(self, message: WebSocketMessage) -> None:
|
||||
...
|
||||
```
|
||||
|
||||
**Internal callers NOT updated by Phase 5:**
|
||||
- `src/app_controller.py:_run_pending_tasks_once_result` — broadcasts task results to the WebSocket pipeline per pending GUI task
|
||||
- `src/events.py` — broadcasts events emitted by the `AsyncEventQueue`
|
||||
- `src/gui_2.py:_process_pending_gui_tasks` — broadcasts from the GUI thread's pending-task queue
|
||||
|
||||
**Fix:** Replace `broadcast("channel", payload_dict)` with `broadcast(WebSocketMessage(channel="channel", payload=payload_dict))`.
|
||||
|
||||
### 3.2 The Missing Senders: 3 OpenAI-Compatible Providers
|
||||
|
||||
The 3 OpenAI-compatible senders in `src/ai_client.py`:
|
||||
- `_send_grok` (L2532)
|
||||
- `_send_minimax` (L2616)
|
||||
- `_send_llama` (L2856)
|
||||
|
||||
(Plus `_send_llama_native` at L2954, which is a different code path.)
|
||||
|
||||
These senders construct `OpenAICompatibleRequest(messages=[...], model=..., ...)` with the **legacy** shape:
|
||||
```python
|
||||
messages=[{"role": "user", "content": user_content}]
|
||||
```
|
||||
|
||||
After this track:
|
||||
```python
|
||||
messages=[ChatMessage(role="user", content=user_content)]
|
||||
```
|
||||
|
||||
And `NormalizedResponse(text=..., usage_input_tokens=..., usage_output_tokens=...)`:
|
||||
```python
|
||||
NormalizedResponse(text=text, tool_calls=(), usage=UsageStats(input_tokens=t_in, output_tokens=t_out), raw_response=raw)
|
||||
```
|
||||
|
||||
### 3.3 The Regression Protocol
|
||||
|
||||
After this track, the protocol for any Phase-style refactor is:
|
||||
|
||||
1. After implementing each phase, run the FULL `tier-1-unit-core` tier (not targeted tests). Targeted tests miss call sites in helper functions / cross-file consumers.
|
||||
2. After all phases complete, run `tier-1-unit-core` + `tier-1-unit-mma` + `tier-2-mock-app-core` + `tier-3-live_gui` FULLY (no stop-on-failure).
|
||||
3. The "no-TypeError-errors-on-any-thread" assertion in `tests/test_websocket_broadcast_regression.py` is the canonical regression test. `code_path_audit_20260607` will reuse this assertion in its per-action profiling.
|
||||
|
||||
## 4. Per-Phase Plan
|
||||
|
||||
### Phase 6a: Fix `HookServer.broadcast()` Callers
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/app_controller.py:_run_pending_tasks_once_result`
|
||||
- Modify: `src/events.py` (broadcast sites)
|
||||
- Modify: `src/gui_2.py:_process_pending_gui_tasks`
|
||||
- Create: `tests/test_websocket_broadcast_regression.py`
|
||||
|
||||
**Approach:**
|
||||
1. Grep `\.broadcast\(` in `src/` to find all internal callers
|
||||
2. For each: replace `broadcast(channel_str, payload_dict)` with `broadcast(WebSocketMessage(channel=channel_str, payload=payload_dict))`
|
||||
3. Add regression test: simulate a GUI task that triggers broadcast and assert no TypeError in stderr
|
||||
|
||||
**Why this matters for code_path_audit:**
|
||||
The audit's per-action profiling assumes no TypeError spam on the GUI thread. The Phase 6a fix makes the GUI's broadcast pipeline type-safe; the audit can then measure `WebSocketMessage.__init__` overhead per broadcast without TypeError contamination.
|
||||
|
||||
### Phase 6b: Complete `_send_grok` / `_send_minimax` / `_send_llama` `OpenAICompatibleRequest` Migration
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/ai_client.py:_send_grok` (L2532)
|
||||
- Modify: `src/ai_client.py:_send_minimax` (L2616)
|
||||
- Modify: `src/ai_client.py:_send_llama` (L2856)
|
||||
- Modify: `tests/test_grok_provider.py` if it exists
|
||||
- Modify: `tests/test_minimax_provider.py` if it exists
|
||||
- Modify: `tests/test_llama_provider.py` if it exists
|
||||
|
||||
**Approach:**
|
||||
1. In each sender, replace `messages=[{"role": "user", "content": ...}]` with `messages=[ChatMessage(role="user", content=...)]`
|
||||
2. Update `OpenAICompatibleRequest` field-by-field to use `ChatMessage` everywhere
|
||||
3. Run provider tests + integration tests
|
||||
|
||||
### Phase 6d: Update Those Senders' `NormalizedResponse` Construction
|
||||
|
||||
**Files:** Same as 6b.
|
||||
|
||||
**Approach:**
|
||||
1. In each sender, replace `NormalizedResponse(text=..., usage_input_tokens=X, usage_output_tokens=Y, usage_cache_read_tokens=Z, usage_cache_creation_tokens=W, raw_response=R)` with `NormalizedResponse(text=..., tool_calls=(), usage=UsageStats(input_tokens=X, output_tokens=Y, cache_read_tokens=Z, cache_creation_tokens=W), raw_response=R)`
|
||||
2. Add import: `from src.openai_schemas import ChatMessage, NormalizedResponse, OpenAICompatibleRequest, UsageStats`
|
||||
3. Run provider tests + integration tests
|
||||
|
||||
## 5. Configuration
|
||||
|
||||
No new dependencies. No new config files.
|
||||
|
||||
## 6. Testing Strategy
|
||||
|
||||
| Test File | Purpose |
|
||||
|---|---|
|
||||
| `tests/test_websocket_broadcast_regression.py` (NEW) | Verify no TypeError spam on GUI thread after broadcast() callers are fixed |
|
||||
| `tests/test_grok_provider.py` (extend) | Verify `_send_grok` uses ChatMessage + UsageStats |
|
||||
| `tests/test_minimax_provider.py` (extend) | Verify `_send_minimax` uses ChatMessage + UsageStats |
|
||||
| `tests/test_llama_provider.py` (extend) | Verify `_send_llama` uses ChatMessage + UsageStats |
|
||||
|
||||
**Verification protocol (the lesson from `any_type_componentization_20260621`):**
|
||||
- After each Phase, run `uv run python scripts/run_tests_batched.py --tier tier-1-unit-core` FULLY (no stop-on-failure)
|
||||
- After all Phases complete, run all 11 tiers FULLY
|
||||
|
||||
## 7. Migration / Rollout
|
||||
|
||||
| Phase | What | Commits |
|
||||
|---|---|---|
|
||||
| 6a | `HookServer.broadcast()` callers fixed; `test_websocket_broadcast_regression.py` added | ~5-7 |
|
||||
| 6b | `_send_grok/minimax/llama` OpenAICompatibleRequest migration | ~3-5 |
|
||||
| 6d | `_send_grok/minimax/llama` NormalizedResponse migration | ~3-4 |
|
||||
| Total | | ~11-16 |
|
||||
|
||||
Each phase has its own checkpoint commit and git note.
|
||||
|
||||
## 8. Risks & Mitigations
|
||||
|
||||
| Risk | Likelihood | Impact | Mitigation |
|
||||
|---|---|---|---|
|
||||
| Grep misses an internal broadcast() caller | Low | Medium | Also check `tests/` for callers; assert "no TypeError spam" on the full 11-tier run |
|
||||
| `_send_grok/minimax/llama` test coverage is thin | Medium | Low | The 3 providers are exercised in `tests/test_*provider*.py`; if tests don't exist, add a smoke test |
|
||||
| The "no-TypeError" assertion is too strict (false positives) | Low | Low | Wrap in `try/except queue_fallback`; assert "no broadcast() TypeError specifically" |
|
||||
|
||||
## 9. Out of Scope
|
||||
|
||||
- **Phase 3 (`provider_state` call-site migration).** Deferred to a separate track after `code_path_audit_20260607` provides runtime cost data.
|
||||
- **Cross-phase coupling** (`OpenAICompatibleRequest.tools: list[ToolSpec]`). Deferred.
|
||||
- **`audit_tier2_leaks.py` sandbox-pollution failures.** Separate infra track.
|
||||
- **Pre-existing `test_gui2_custom_callback_hook_works` flake.** Separate investigation.
|
||||
- **Merging `tier2/any_type_componentization_20260621` branch.** Per Tier 2's recommendation, the branch stays as reconnaissance; this track cherry-picks only the fixes.
|
||||
|
||||
## 10. Verification Criteria
|
||||
|
||||
- [ ] `src/app_controller.py:_run_pending_tasks_once_result` uses `broadcast(WebSocketMessage(...))`
|
||||
- [ ] `src/events.py` broadcast callers use `WebSocketMessage`
|
||||
- [ ] `src/gui_2.py:_process_pending_gui_tasks` broadcast callers use `WebSocketMessage`
|
||||
- [ ] `tests/test_websocket_broadcast_regression.py` exists; asserts no broadcast() TypeError
|
||||
- [ ] `_send_grok` constructs `OpenAICompatibleRequest(messages=[ChatMessage(...)], ...)`
|
||||
- [ ] `_send_minimax` constructs `OpenAICompatibleRequest(messages=[ChatMessage(...)], ...)`
|
||||
- [ ] `_send_llama` constructs `OpenAICompatibleRequest(messages=[ChatMessage(...)], ...)`
|
||||
- [ ] `_send_grok` constructs `NormalizedResponse(text=..., usage=UsageStats(...), ...)`
|
||||
- [ ] `_send_minimax` constructs `NormalizedResponse(text=..., usage=UsageStats(...), ...)`
|
||||
- [ ] `_send_llama` constructs `NormalizedResponse(text=..., usage=UsageStats(...), ...)`
|
||||
- [ ] All 11-tier batched test run passes (no stop-on-failure)
|
||||
- [ ] `audit_weak_types.py --strict` exits 0
|
||||
- [ ] `audit_dataclass_coverage.py --strict` exits 0
|
||||
- [ ] End-of-track report at `docs/reports/TRACK_COMPLETION_phase2_4_5_call_site_completion_20260621.md`
|
||||
|
||||
## 11. See Also
|
||||
|
||||
- `docs/handoffs/PROMPT_FOR_TIER_1.md` — Tier 1 brief from Tier 2
|
||||
- `docs/handoffs/HANDOFF_FOLLOWUP_TRACK_FROM_any_type_componentization.md` — test failure categorization
|
||||
- `docs/handoffs/HANDOFF_CODE_PATH_AUDIT_FROM_any_type_componentization.md` — runtime cost framing
|
||||
- `conductor/tracks/any_type_componentization_20260621/spec.md` — parent track spec
|
||||
- `conductor/tracks/code_path_audit_20260607/spec.md` — the audit (this track unblocks it)
|
||||
- `docs/reports/PHASE3_HYPOTHETICAL_PROMOTION.md` — the Phase 3 hypothetical analysis (separate doc)
|
||||
@@ -0,0 +1,77 @@
|
||||
# Track state for phase2_4_5_call_site_completion_20260621
|
||||
# Updated by Tier 2 Tech Lead as tasks complete
|
||||
|
||||
[meta]
|
||||
track_id = "phase2_4_5_call_site_completion_20260621"
|
||||
name = "Phase 2/4/5 Call-Site Completion (post any_type_componentization)"
|
||||
status = "active"
|
||||
current_phase = 0
|
||||
last_updated = "2026-06-21"
|
||||
|
||||
[blocked_by]
|
||||
# No blockers; this track unblocks the audit
|
||||
[]
|
||||
|
||||
[blocks]
|
||||
code_path_audit_20260607 = "blocked_until_merge"
|
||||
|
||||
[phases]
|
||||
phase_6a = { status = "pending", checkpointsha = "", name = "Fix HookServer.broadcast() callers" }
|
||||
phase_6b = { status = "pending", checkpointsha = "", name = "Complete OpenAICompatibleRequest migration" }
|
||||
phase_6d = { status = "pending", checkpointsha = "", name = "Update NormalizedResponse construction" }
|
||||
|
||||
[tasks]
|
||||
# Phase 6a: Fix HookServer.broadcast() callers
|
||||
t6a_1 = { status = "pending", commit_sha = "", description = "Grep src/ for all .broadcast( callers; document the list (expect ~5-10 sites)" }
|
||||
t6a_2 = { status = "pending", commit_sha = "", description = "Red: tests/test_websocket_broadcast_regression.py (verify no broadcast() TypeError on GUI thread)" }
|
||||
t6a_3 = { status = "pending", commit_sha = "", description = "Fix src/app_controller.py:_run_pending_tasks_once_result broadcast callers" }
|
||||
t6a_4 = { status = "pending", commit_sha = "", description = "Fix src/events.py broadcast callers" }
|
||||
t6a_5 = { status = "pending", commit_sha = "", description = "Fix src/gui_2.py:_process_pending_gui_tasks broadcast callers" }
|
||||
t6a_6 = { status = "pending", commit_sha = "", description = "Run tier-1-unit-core FULLY (no stop-on-failure) per regression protocol" }
|
||||
t6a_7 = { status = "pending", commit_sha = "", description = "Phase 6a checkpoint commit + git note" }
|
||||
# Phase 6b: OpenAICompatibleRequest migration
|
||||
t6b_1 = { status = "pending", commit_sha = "", description = "Identify tests/test_grok_provider.py + test_minimax_provider.py + test_llama_provider.py; if absent, add smoke tests" }
|
||||
t6b_2 = { status = "pending", commit_sha = "", description = "Red: tests for ChatMessage in OpenAICompatibleRequest construction (grok/minimax/llama senders)" }
|
||||
t6b_3 = { status = "pending", commit_sha = "", description = "Migrate src/ai_client.py:_send_grok messages construction to ChatMessage" }
|
||||
t6b_4 = { status = "pending", commit_sha = "", description = "Migrate src/ai_client.py:_send_minimax messages construction to ChatMessage" }
|
||||
t6b_5 = { status = "pending", commit_sha = "", description = "Migrate src/ai_client.py:_send_llama messages construction to ChatMessage" }
|
||||
t6b_6 = { status = "pending", commit_sha = "", description = "Run tier-1-unit-core + provider tests FULLY" }
|
||||
t6b_7 = { status = "pending", commit_sha = "", description = "Phase 6b checkpoint commit + git note" }
|
||||
# Phase 6d: NormalizedResponse construction
|
||||
t6d_1 = { status = "pending", commit_sha = "", description = "Red: tests for UsageStats in NormalizedResponse construction (grok/minimax/llama senders)" }
|
||||
t6d_2 = { status = "pending", commit_sha = "", description = "Migrate src/ai_client.py:_send_grok NormalizedResponse to use UsageStats" }
|
||||
t6d_3 = { status = "pending", commit_sha = "", description = "Migrate src/ai_client.py:_send_minimax NormalizedResponse to use UsageStats" }
|
||||
t6d_4 = { status = "pending", commit_sha = "", description = "Migrate src/ai_client.py:_send_llama NormalizedResponse to use UsageStats" }
|
||||
t6d_5 = { status = "pending", commit_sha = "", description = "Run tier-1-unit-core + provider tests FULLY" }
|
||||
t6d_6 = { status = "pending", commit_sha = "", description = "All 11 tiers FULLY (no stop-on-failure) per regression protocol" }
|
||||
t6d_7 = { status = "pending", commit_sha = "", description = "Phase 6d checkpoint commit + git note" }
|
||||
# Verify + archive
|
||||
tv_1 = { status = "pending", commit_sha = "", description = "Run audit_weak_types.py --strict + audit_dataclass_coverage.py --strict (both exit 0)" }
|
||||
tv_2 = { status = "pending", commit_sha = "", description = "Run generate_type_registry.py --check (exit 0)" }
|
||||
tv_3 = { status = "pending", commit_sha = "", description = "Write docs/reports/TRACK_COMPLETION_phase2_4_5_call_site_completion_20260621.md" }
|
||||
tv_4 = { status = "pending", commit_sha = "", description = "git mv to conductor/tracks/archive/" }
|
||||
tv_5 = { status = "pending", commit_sha = "", description = "Update conductor/tracks.md" }
|
||||
|
||||
[verification]
|
||||
phase_6a_broadcast_fixed = false
|
||||
phase_6a_regression_test_passes = false
|
||||
phase_6b_openai_compat_migrated = false
|
||||
phase_6d_normalized_response_migrated = false
|
||||
full_11_tier_regression_passes = false
|
||||
audit_weak_types_strict_passes = false
|
||||
audit_dataclass_coverage_strict_passes = false
|
||||
type_registry_check_passes = false
|
||||
track_archived = false
|
||||
|
||||
[broadcast_callers_to_fix]
|
||||
# Filled in t6a_1
|
||||
expected_sites = 8
|
||||
files_affected = ["src/app_controller.py", "src/events.py", "src/gui_2.py"]
|
||||
|
||||
[deferred_from_parent_track]
|
||||
phase_3_provider_state_sites = 112
|
||||
phase_3_deferred_to = "separate track post code_path_audit_20260607"
|
||||
cross_phase_coupling = "OpenAICompatibleRequest.tools: list[dict] -> list[ToolSpec]; deferred"
|
||||
|
||||
[unblocks]
|
||||
code_path_audit_20260607 = "Phase 6a fixes broadcast() TypeError that contaminates audit instrumentation"
|
||||
Reference in New Issue
Block a user