conductor(track): type_alias_unfuck_20260626 spec
This commit is contained in:
@@ -0,0 +1,829 @@
|
||||
# Plan: type_alias_unfuck_20260626 (EXTREME DETAIL)
|
||||
|
||||
> **Tier 1 exhaustive plan — 2026-06-26.** This plan is the EXECUTABLE CONTRACT for Tier 2/Tier 3. Every task has exact file:line refs, exact before/after code, exact test commands, and explicit FIX-IF-FAILS steps. NEVER use `git restore`, `git checkout --`, `git reset`, or `git revert` (per AGENTS.md hard ban). If a phase's count delta doesn't match, MODIFY the migration until it does.
|
||||
>
|
||||
> **Baseline (measured 2026-06-26, master `b4bd772d`):**
|
||||
> - `.get('key', default)` sites in `src/*.py`: **52** (down from 107 — prior Tier 2 attempts migrated ~55)
|
||||
> - `[ 'key' ]` subscript sites in `src/*.py`: **~70** (most are genuinely collapsed-codepath)
|
||||
> - Effective codepaths: **4.014e+22**
|
||||
>
|
||||
> **Acceptance:** `.get()` count drops to < 15 (collapsed-codepath only); effective codepaths drops by ≥ 1 order of magnitude; 7 audit gates pass `--strict`; 10/11 batched test tiers PASS.
|
||||
>
|
||||
> **Tier 2 already migrated (do NOT re-do these):**
|
||||
> - src/ai_client.py:2565,2808,2900: partially migrated (`fi if hasattr(fi, 'path') else models.FileItem(path=fi.get('path', 'attachment'))`)
|
||||
> - src/gui_2.py:5802: `entry['source_tier'] if 'source_tier' in entry else 'main'` (half-measure; needs full migration)
|
||||
> - src/synthesis_formatter.py:24,37: Tier 2 migrated these (no longer in grep output)
|
||||
> - src/app_controller.py:2303,2314,2315: Tier 2 migrated `u = payload['usage']` to `u_stats.input_tokens` direct access (no longer in grep output)
|
||||
|
||||
## §0 Pre-flight (Tier 2 runs before Tier 3 starts)
|
||||
|
||||
```bash
|
||||
# 0.1 Clean working tree on a fresh branch
|
||||
git checkout -b tier2/type_alias_unfuck_20260626
|
||||
git status --short
|
||||
# Expect: no output (clean)
|
||||
|
||||
# 0.2 Capture baseline counts
|
||||
git grep -nE "\.get\('[a-z_]+'," -- 'src/*.py' > /tmp/before_get.txt
|
||||
# count of /tmp/before_get.txt lines: 52
|
||||
git grep -nE "\[[ ]*'[a-z_]+'[ ]*\]" -- 'src/*.py' > /tmp/before_subscript.txt
|
||||
# count of /tmp/before_subscript.txt lines: ~70
|
||||
|
||||
# 0.3 Confirm 7 audit gates pass --strict (note any pre-existing failures)
|
||||
uv run python scripts/audit_weak_types.py --strict
|
||||
uv run python scripts/generate_type_registry.py --check
|
||||
uv run python scripts/audit_main_thread_imports.py
|
||||
uv run python scripts/audit_no_models_config_io.py
|
||||
uv run python scripts/audit_code_path_audit_coverage.py --input-dir docs/reports/code_path_audit/latest --strict
|
||||
uv run python scripts/audit_exception_handling.py --strict
|
||||
uv run python scripts/audit_optional_in_3_files.py --strict
|
||||
# All exit 0; note pre-existing failures separately
|
||||
|
||||
# 0.4 Verify existing dataclasses import
|
||||
uv run python -c "from src.type_aliases import CommsLogEntry, HistoryMessage, ToolDefinition, SessionInsights, DiscussionSettings, CustomSlice, MMAUsageStats, ProviderPayload, UIPanelConfig, PathInfo; from src.openai_schemas import ToolCall, ChatMessage, UsageStats, NormalizedResponse; from src.models import Ticket, FileItem; from src.rag_engine import RAGChunk; from src.mcp_client import ASTNode, SearchResult, MCPToolResult; print('all imports OK')"
|
||||
# Expect: all imports OK
|
||||
```
|
||||
|
||||
**STOP if any pre-existing failure is not documented in the baseline report.**
|
||||
|
||||
## §Phase 1: Ticket consumers (SKIP)
|
||||
|
||||
Already done in `metadata_promotion_20260624/0506c5da`. No work in this phase.
|
||||
|
||||
## §Phase 2: FileItem consumers (3 sites, partial migration completion)
|
||||
|
||||
**WHERE:** `src/ai_client.py:2565,2808,2900`
|
||||
|
||||
**Current state:** Tier 2 partially migrated these. The pattern is:
|
||||
|
||||
```python
|
||||
fi_item = fi if hasattr(fi, 'path') else models.FileItem(path=fi.get('path', 'attachment'))
|
||||
```
|
||||
|
||||
This is a half-measure. The `.get('path', 'attachment')` is still inside the else branch. Tier 2 needs to fix this by ensuring `fi` is a `FileItem` instance before the access, or by using direct attribute access on `fi` if it's already a dataclass.
|
||||
|
||||
**Task 2.1:** Fix the half-measure pattern in `src/ai_client.py:2565,2808,2900`.
|
||||
|
||||
**Read the full context first:**
|
||||
|
||||
```bash
|
||||
manual-slop_get_file_slice --path src/ai_client.py --start_line 2560 --end_line 2570
|
||||
manual-slop_get_file_slice --path src/ai_client.py --start_line 2803 --end_line 2813
|
||||
manual-slop_get_file_slice --path src/ai_client.py --start_line 2895 --end_line 2905
|
||||
```
|
||||
|
||||
**Determine the variable's actual type.** If `fi` arrives from upstream as a `models.FileItem` instance, the migration is `fi.path or 'attachment'`. If `fi` is a dict (from JSON wire), the migration is `models.FileItem.from_dict(fi).path or 'attachment'`.
|
||||
|
||||
**Pattern (decide per-site based on actual type):**
|
||||
|
||||
```python
|
||||
# BEFORE:
|
||||
fi_item = fi if hasattr(fi, 'path') else models.FileItem(path=fi.get('path', 'attachment'))
|
||||
|
||||
# AFTER (if fi is dict at this site):
|
||||
fi_item = models.FileItem.from_dict(fi) if isinstance(fi, dict) else fi
|
||||
|
||||
# AFTER (if fi is dataclass at this site):
|
||||
fi_item = fi
|
||||
```
|
||||
|
||||
Then the downstream `fi_item.path or 'attachment'` works regardless.
|
||||
|
||||
**HOW:** `manual-slop_edit_file` per site. **Anchor on the surrounding context** (read 2 lines above + 2 below) to ensure exact match.
|
||||
|
||||
**SAFETY:**
|
||||
```bash
|
||||
git grep -nE "\.get\('path'," -- 'src/ai_client.py' | wc -l
|
||||
# Expect: 0
|
||||
uv run python -m pytest tests/test_ai_client.py tests/test_file_item_model.py -x --timeout=60
|
||||
# Expect: all pass
|
||||
```
|
||||
|
||||
**MODIFY-IF-FAILS:**
|
||||
- If `git grep` returns non-zero: check whether the `hasattr` pattern is still using `.get`. Read the surrounding code. If `fi` is a `FileItem` dataclass, remove the `hasattr` guard entirely (it's a half-measure defensive pattern).
|
||||
- If pytest fails: STOP. Read the failure mode. Predict whether the migration introduced a regression. If `fi` was a dict before and is now expected to be a `FileItem`, the upstream caller needs to be fixed.
|
||||
|
||||
**COMMIT:** `refactor(ai_client): complete FileItem migration (finish half-measure pattern)`
|
||||
|
||||
**Commit message body MUST include:**
|
||||
```
|
||||
Phase 2: FileItem
|
||||
Before: 3 .get('path',...) sites in src/ai_client.py
|
||||
After: 0 .get('path',...) sites in src/ai_client.py
|
||||
Delta: -3 (expected: -3)
|
||||
```
|
||||
|
||||
**GIT NOTE:** Completed FileItem migration. Tier 2's earlier attempt left a half-measure (`fi if hasattr(fi, 'path') else models.FileItem(path=fi.get('path', 'attachment'))`); this commit removes the `.get('path', 'attachment')` fallback by ensuring `fi` is always a `FileItem` instance via `from_dict()`.
|
||||
|
||||
## §Phase 3: CommsLogEntry consumers (4 sites)
|
||||
|
||||
**WHERE:**
|
||||
- `src/app_controller.py:2278` (inside `entry_obj` dict construction)
|
||||
- `src/app_controller.py:2305,2306,2307,2308` (inside `new_token_history.append` block)
|
||||
- `src/gui_2.py:5802` (render_tool_calls_panel)
|
||||
|
||||
**Task 3.1:** Read the full context of `src/app_controller.py:2270-2320` to understand the data flow.
|
||||
|
||||
**Current code (read first):**
|
||||
|
||||
```python
|
||||
# app_controller.py:2270-2310 (approximate, READ FIRST)
|
||||
if kind == 'tool_call':
|
||||
tid = payload.get('id') or payload.get('call_id')
|
||||
script = payload.get('script') or json.dumps(payload.get('args', {}), indent=1)
|
||||
script = _resolve_log_ref(script, session_dir)
|
||||
entry_obj = {
|
||||
'source_tier': entry.get('source_tier', 'main'), # ← line 2278
|
||||
...
|
||||
}
|
||||
elif kind == 'response' and 'usage' in payload:
|
||||
u = payload['usage']
|
||||
...
|
||||
new_token_history.append({
|
||||
'time': ts,
|
||||
'input': u.get('input_tokens', 0) or 0, # ← line 2305
|
||||
'output': u.get('output_tokens', 0) or 0, # ← line 2306
|
||||
'cache_read': u.get('cache_read_input_tokens', 0) or 0, # ← line 2307
|
||||
'cache_creation': u.get('cache_creation_input_tokens', 0) or 0, # ← line 2308
|
||||
...
|
||||
})
|
||||
```
|
||||
|
||||
**Per-site migration:**
|
||||
|
||||
For `app_controller.py:2278`:
|
||||
- **old_string:** `'source_tier': entry.get('source_tier', 'main'),`
|
||||
- **new_string:** `'source_tier': (entry.source_tier if hasattr(entry, 'source_tier') else CommsLogEntry.from_dict(entry).source_tier),`
|
||||
|
||||
Or, if `entry` is always a dict at this site:
|
||||
- **new_string:** `'source_tier': CommsLogEntry.from_dict(entry).source_tier,`
|
||||
|
||||
(Tier 3 determines the right pattern by reading the surrounding context with `manual-slop_get_file_slice`.)
|
||||
|
||||
For `app_controller.py:2305,2306,2307,2308`:
|
||||
- **old_string:** `'input': u.get('input_tokens', 0) or 0,`
|
||||
- **new_string:** `'input': (UsageStats.from_dict(u).input_tokens if isinstance(u, dict) else u.input_tokens) or 0,`
|
||||
|
||||
(Or simpler, if `u` is always a dict: `'input': UsageStats.from_dict(u).input_tokens or 0,`)
|
||||
|
||||
For `gui_2.py:5802`:
|
||||
- **current:** `entry['source_tier'] if 'source_tier' in entry else 'main'`
|
||||
- **new:** `CommsLogEntry.from_dict(entry).source_tier if isinstance(entry, dict) else entry.source_tier`
|
||||
|
||||
**HOW:** `manual-slop_edit_file` per site. Read the full surrounding context (5 lines above + 5 below) before each edit.
|
||||
|
||||
**SAFETY:**
|
||||
```bash
|
||||
git grep -nE "\.get\('source_tier'," -- 'src/*.py' | wc -l
|
||||
# Expect: 0
|
||||
git grep -nE "\.get\('model'," -- 'src/app_controller.py' | wc -l
|
||||
# Expect: 0 (if Phase 3 also migrates the model get at line 2311)
|
||||
uv run python -m pytest tests/test_session_logger_optimization.py tests/test_session_logger_reset.py tests/test_session_logging.py tests/test_logging_e2e.py tests/test_comms_log_entry.py -x --timeout=60
|
||||
# Expect: all pass
|
||||
```
|
||||
|
||||
**MODIFY-IF-FAILS:**
|
||||
- If grep shows non-zero: search for any `.get('source_tier',` or `.get('model',` you missed. Add them to this phase's commit as additional migrations.
|
||||
- If pytest fails: STOP. Read the failure mode. Likely cause: `entry` is genuinely a dict constructed on-the-fly and the migration to `CommsLogEntry.from_dict(entry)` is correct but the surrounding function doesn't handle the conversion. Re-read the function and find where the entry_obj is built. Add the `from_dict()` call at the top of the function (not at every access site).
|
||||
|
||||
**COMMIT:** `refactor(app_controller,gui_2): migrate CommsLogEntry consumers to direct field access`
|
||||
|
||||
**Commit message body MUST include:**
|
||||
```
|
||||
Phase 3: CommsLogEntry
|
||||
Before: 4 .get('source_tier',...) + .get('model',...) sites
|
||||
After: 0
|
||||
Delta: -4 (expected: -4)
|
||||
```
|
||||
|
||||
## §Phase 4: HistoryMessage consumers (0 sites — already done by Tier 2)
|
||||
|
||||
`src/synthesis_formatter.py:24,37` was migrated by Tier 2. No work in this phase.
|
||||
|
||||
## §Phase 5: ChatMessage into per-vendor send paths (~27 sites)
|
||||
|
||||
**WHERE:** `src/ai_client.py` (8 vendor send methods: `_send_anthropic`, `_send_deepseek`, `_send_gemini`, `_send_gemini_cli`, `_send_minimax`, `_send_qwen`, `_send_llama`, `_send_grok`)
|
||||
|
||||
**Task 5.1:** Read each send method to find the `.get('role', ...)` and `.get('content', ...)` sites.
|
||||
|
||||
```bash
|
||||
git grep -nE "_send_anthropic|_send_deepseek|_send_gemini|_send_gemini_cli|_send_minimax|_send_qwen|_send_llama|_send_grok" -- 'src/ai_client.py'
|
||||
```
|
||||
|
||||
Each send method has its own provider-specific message construction. The pattern is consistent:
|
||||
|
||||
```python
|
||||
# BEFORE (per provider):
|
||||
for msg in anthropic_history:
|
||||
if msg.get("role") == "user":
|
||||
messages.append({"role": "user", "content": msg.get("content", "")})
|
||||
```
|
||||
|
||||
**Pattern (per-site):**
|
||||
|
||||
```python
|
||||
# AFTER:
|
||||
for msg in anthropic_history:
|
||||
cm = msg if isinstance(msg, ChatMessage) else ChatMessage.from_dict(msg)
|
||||
if cm.role == "user":
|
||||
messages.append(cm.to_dict())
|
||||
```
|
||||
|
||||
**HOW:** For each send method, read the full method body with `manual-slop_get_file_slice`. Identify every `.get('role', ...)`, `.get('content', ...)`, `.get('tool_calls', ...)`, etc. Apply the `ChatMessage.from_dict()` pattern.
|
||||
|
||||
**Specific sites to migrate** (read each line first):
|
||||
|
||||
```bash
|
||||
git grep -nE "\.get\('role',|\.get\('content',|\.get\('tool_calls',|\.get\('tool_call_id',|\.get\('name'," -- 'src/ai_client.py'
|
||||
```
|
||||
|
||||
For each hit, apply the `ChatMessage.from_dict()` pattern at the entry to the per-message processing block.
|
||||
|
||||
**SAFETY:**
|
||||
```bash
|
||||
git grep -nE "msg\.get\('role',|msg\.get\('content'," -- 'src/ai_client.py' | wc -l
|
||||
# Expect: 0
|
||||
uv run python -m pytest tests/test_ai_client.py tests/test_anthropic_provider.py tests/test_deepseek_provider.py tests/test_openai_schemas.py tests/test_chat_message.py -x --timeout=120
|
||||
# Expect: all pass
|
||||
```
|
||||
|
||||
**MODIFY-IF-FAILS:**
|
||||
- If grep shows non-zero: check whether the `msg` variable is iterated as a dict vs a ChatMessage instance. If it's a `provider_state.get_history()` return value, the history might already be ChatMessage instances — in which case the migration is `if cm.role == "user"` (no `from_dict()` needed).
|
||||
- If pytest fails: STOP. Likely cause: the `ChatMessage.from_dict()` returns None for missing fields; check whether `cm.role` would AttributeError if `cm` is None.
|
||||
|
||||
**COMMIT:** `refactor(ai_client): wire ChatMessage into per-vendor send paths (Phase 5)`
|
||||
|
||||
**Commit message body MUST include:**
|
||||
```
|
||||
Phase 5: ChatMessage
|
||||
Before: N .get('role',...) + .get('content',...) sites in src/ai_client.py
|
||||
After: 0
|
||||
Delta: -N (expected: ≥10)
|
||||
```
|
||||
|
||||
## §Phase 6: UsageStats into per-call usage aggregation (4 sites)
|
||||
|
||||
**WHERE:**
|
||||
- `src/app_controller.py:2305,2306,2307,2308` (already partially in Phase 3 — migrate the remaining `.get('input_tokens', 0)` style sites)
|
||||
|
||||
Wait — `src/app_controller.py:2305-2308` were already migrated by Tier 2 to use `u_stats.input_tokens` direct attribute access. Let me verify by reading:
|
||||
|
||||
```bash
|
||||
git grep -nE "\.get\('input_tokens',|\.get\('output_tokens',|\.get\('cache_read_input_tokens',|\.get\('cache_creation_input_tokens'," -- 'src/app_controller.py'
|
||||
```
|
||||
|
||||
If 0 sites remain, Phase 6 is DONE. If sites remain, migrate them.
|
||||
|
||||
**Task 6.1:** Verify Phase 6 is done; if not, migrate.
|
||||
|
||||
**Pattern (if migration needed):**
|
||||
|
||||
```python
|
||||
# BEFORE:
|
||||
u = payload['usage'] # dict
|
||||
'input': u.get('input_tokens', 0) or 0,
|
||||
|
||||
# AFTER:
|
||||
u = UsageStats.from_dict(payload['usage'])
|
||||
'input': u.input_tokens or 0,
|
||||
```
|
||||
|
||||
**HOW:** `manual-slop_edit_file` per site.
|
||||
|
||||
**SAFETY:**
|
||||
```bash
|
||||
git grep -nE "\.get\('input_tokens',|\.get\('output_tokens'," -- 'src/app_controller.py' | wc -l
|
||||
# Expect: 0
|
||||
uv run python -m pytest tests/test_token_usage.py tests/test_usage_analytics_popout_sim.py -x --timeout=60
|
||||
# Expect: all pass
|
||||
```
|
||||
|
||||
**COMMIT:** `refactor(app_controller): wire UsageStats into per-call usage (Phase 6)`
|
||||
|
||||
**Commit message body MUST include:**
|
||||
```
|
||||
Phase 6: UsageStats
|
||||
Before: N .get('input_tokens',...) sites in src/app_controller.py
|
||||
After: 0
|
||||
Delta: -N (expected: ≥4)
|
||||
```
|
||||
|
||||
## §Phase 7: ToolCall into tool loop (3 sites)
|
||||
|
||||
**WHERE:**
|
||||
- `src/mcp_client.py:1707,1708,1714`
|
||||
|
||||
**Current code:**
|
||||
```python
|
||||
src/mcp_client.py:1707: for t in result['tools']:
|
||||
src/mcp_client.py:1708: self.tools[t['name']] = t
|
||||
src/mcp_client.py:1714: return '\n'.join([c.get('text', '') for c in result['content'] if c.get('type') == 'text'])
|
||||
```
|
||||
|
||||
**Pattern:**
|
||||
```python
|
||||
# BEFORE:
|
||||
for t in result['tools']:
|
||||
self.tools[t['name']] = t
|
||||
|
||||
# AFTER:
|
||||
mc_result = MCPToolResult.from_dict(result)
|
||||
for t in mc_result.tools:
|
||||
self.tools[t.name] = t
|
||||
```
|
||||
|
||||
For `mcp_client.py:1714`:
|
||||
```python
|
||||
# BEFORE:
|
||||
return '\n'.join([c.get('text', '') for c in result['content'] if c.get('type') == 'text'])
|
||||
|
||||
# AFTER (if result.content is now a tuple of dicts after from_dict):
|
||||
mc_result = MCPToolResult.from_dict(result)
|
||||
return '\n'.join([c.get('text', '') for c in mc_result.content if c.get('type') == 'text'])
|
||||
```
|
||||
|
||||
Wait — `MCPToolResult.content: tuple[Metadata, ...]` per Phase 0 of `metadata_promotion_20260624`. So `mc_result.content` is a tuple of dicts. The `[c.get('text', '') for c in mc_result.content]` still uses `.get()` on each dict. That's correct because each `c` is still a `dict` (not a dataclass). **The migration at this site is `result['content']` → `mc_result.content` (subscript → attribute).** The `.get('text', '')` on each `c` stays because `c` is a dict element, not a dataclass.
|
||||
|
||||
**HOW:** `manual-slop_edit_file` per site. Read the surrounding context first.
|
||||
|
||||
**SAFETY:**
|
||||
```bash
|
||||
git grep -nE "result\['tools'\]|result\['content'\]" -- 'src/mcp_client.py' | wc -l
|
||||
# Expect: 0 (the `result['content']` is replaced by `mc_result.content`)
|
||||
git grep -nE "t\['name'\]" -- 'src/mcp_client.py' | wc -l
|
||||
# Expect: 0
|
||||
uv run python -m pytest tests/test_mcp_client.py tests/test_metadata_dataclass_aux.py -x --timeout=60
|
||||
# Expect: all pass
|
||||
```
|
||||
|
||||
**MODIFY-IF-FAILS:**
|
||||
- If grep shows non-zero: check whether `result` is still used as a dict. If yes, the migration to `MCPToolResult.from_dict(result)` should be done BEFORE the `for t in result['tools']:` line (at the top of the function).
|
||||
- If pytest fails: STOP. `MCPToolResult.from_dict()` may have wrong field names; check whether `content` is a tuple or list.
|
||||
|
||||
**COMMIT:** `refactor(mcp_client): wire MCPToolResult into tool loop (Phase 7)`
|
||||
|
||||
**Commit message body MUST include:**
|
||||
```
|
||||
Phase 7: ToolCall / MCPToolResult
|
||||
Before: 3 .get('tools'/'content'/'name') sites in src/mcp_client.py
|
||||
After: 0
|
||||
Delta: -3 (expected: -3)
|
||||
```
|
||||
|
||||
## §Phase 8: ToolDefinition consumers (3 sites)
|
||||
|
||||
**WHERE:**
|
||||
- `src/mcp_client.py:1970`
|
||||
- `src/gui_2.py:5875,5877`
|
||||
|
||||
**Current code:**
|
||||
```python
|
||||
src/mcp_client.py:1970: 'description': tinfo.get('description', ''),
|
||||
src/gui_2.py:5875: imgui.text(tinfo.get('server', 'unknown')) # ← 'server' is NOT in ToolDefinition
|
||||
src/gui_2.py:5877: imgui.text(tinfo.get('description', ''))
|
||||
```
|
||||
|
||||
**CRITICAL:** `src/gui_2.py:5875` reads `tinfo.get('server', 'unknown')` — but `ToolDefinition` has no `server` field. The fields are `name, description, parameters, auto_start`. **This site cannot be migrated to ToolDefinition.** It must be migrated to a different aggregate (possibly `ToolInfo` which has `server, description`, etc.) OR classified as collapsed-codepath.
|
||||
|
||||
**Task 8.1:** Read the surrounding context for `src/gui_2.py:5875` to determine what `tinfo` actually is.
|
||||
|
||||
```bash
|
||||
manual-slop_get_file_slice --path src/gui_2.py --start_line 5870 --end_line 5880
|
||||
```
|
||||
|
||||
If `tinfo` is a `dict` from MCP server registration, it's NOT a ToolDefinition. Keep as `.get('server', 'unknown')` and classify as collapsed-codepath.
|
||||
|
||||
**For `src/mcp_client.py:1970` and `src/gui_2.py:5877`:**
|
||||
|
||||
```python
|
||||
# BEFORE:
|
||||
'description': tinfo.get('description', ''),
|
||||
|
||||
# AFTER:
|
||||
td = ToolDefinition.from_dict(tinfo) if isinstance(tinfo, dict) else tinfo
|
||||
'description': td.description,
|
||||
```
|
||||
|
||||
**HOW:** `manual-slop_edit_file` per site.
|
||||
|
||||
**SAFETY:**
|
||||
```bash
|
||||
git grep -nE "\.get\('description'," -- 'src/mcp_client.py' 'src/gui_2.py' | wc -l
|
||||
# Expect: 0 (or 1 if 'server' stays as collapsed-codepath)
|
||||
uv run python -m pytest tests/test_mcp_client.py tests/test_tool_definition.py -x --timeout=60
|
||||
# Expect: all pass
|
||||
```
|
||||
|
||||
**MODIFY-IF-FAILS:**
|
||||
- If `tinfo.get('server', 'unknown')` is in collapsed-codepath (because `tinfo` is a server-info dict, not a ToolDefinition), document in the commit: "site 5875 is ToolInfo, not ToolDefinition; classified as collapsed-codepath per FR2."
|
||||
- If pytest fails: STOP. The `ToolDefinition.from_dict()` may fail if `tinfo` has unexpected fields. Read the failure mode.
|
||||
|
||||
**COMMIT:** `refactor(mcp_client,gui_2): migrate ToolDefinition consumers to direct field access`
|
||||
|
||||
**Commit message body MUST include:**
|
||||
```
|
||||
Phase 8: ToolDefinition
|
||||
Before: 3 .get('description',...) sites
|
||||
After: 0 .get('description',...) sites (gui_2.py:5875 'server' field stays as collapsed-codepath per FR2 because tinfo is ToolInfo, not ToolDefinition)
|
||||
Delta: -2 (expected: -2 or -3 depending on ToolInfo classification)
|
||||
```
|
||||
|
||||
## §Phase 9: RAGChunk consumers (3 sites)
|
||||
|
||||
**WHERE:**
|
||||
- `src/aggregate.py:3259`
|
||||
- `src/app_controller.py:251,4162`
|
||||
|
||||
**Current code:**
|
||||
```python
|
||||
src/aggregate.py:3259: context_block += f"### Chunk {i+1} (Source: {path})\n{chunk.get('document', '')}\n\n"
|
||||
src/app_controller.py:251: context_block += f"### Chunk {i+1} (Source: {path})\n{chunk.get('document', '')}\n\n"
|
||||
src/app_controller.py:4162: context_block += f"### Chunk {i+1} (Source: {path})\n{chunk.get('document', '')}\n\n"
|
||||
```
|
||||
|
||||
**CRITICAL:** `RAGChunk` has fields `document, path, score, metadata`. The wire dict from `rag_engine.search()` has `chunk['document']` and `chunk['metadata']['path']` (path nested in metadata). Direct field access requires `chunk.document` (top-level) — but the wire dict has `document` at top-level too, so this might work directly.
|
||||
|
||||
**Task 9.1:** Read the surrounding context to determine what `chunk` actually is at each site.
|
||||
|
||||
```bash
|
||||
manual-slop_get_file_slice --path src/aggregate.py --start_line 3250 --end_line 3270
|
||||
manual-slop_get_file_slice --path src/app_controller.py --start_line 245 --end_line 260
|
||||
manual-slop_get_file_slice --path src/app_controller.py --start_line 4155 --end_line 4170
|
||||
```
|
||||
|
||||
**Pattern (if chunk is a dict):**
|
||||
|
||||
```python
|
||||
# BEFORE:
|
||||
context_block += f"### Chunk {i+1} (Source: {path})\n{chunk.get('document', '')}\n\n"
|
||||
|
||||
# AFTER:
|
||||
rc = RAGChunk.from_dict(chunk) if isinstance(chunk, dict) else chunk
|
||||
context_block += f"### Chunk {i+1} (Source: {path})\n{rc.document}\n\n"
|
||||
```
|
||||
|
||||
**HOW:** `manual-slop_edit_file` per site.
|
||||
|
||||
**SAFETY:**
|
||||
```bash
|
||||
git grep -nE "chunk\.get\('document'," -- 'src/aggregate.py' 'src/app_controller.py' | wc -l
|
||||
# Expect: 0
|
||||
uv run python -m pytest tests/test_rag_engine.py tests/test_rag_phase4_final_verify.py tests/test_rag_chunk.py -x --timeout=120
|
||||
# Expect: all pass
|
||||
```
|
||||
|
||||
**MODIFY-IF-FAILS:**
|
||||
- If `rag_engine.search()` returns `List[Dict]` with `document` nested in `metadata`, then `RAGChunk.from_dict(chunk)` would not find `document` at top level. Fix: extend `RAGChunk.from_dict()` to handle nested metadata (override the classmethod).
|
||||
- If pytest fails: STOP. Read the failure. Likely the chunk document is missing because the wire format has it nested.
|
||||
|
||||
**COMMIT:** `refactor(rag_engine,aggregate,app_controller): migrate RAGChunk consumers to direct field access`
|
||||
|
||||
**Commit message body MUST include:**
|
||||
```
|
||||
Phase 9: RAGChunk
|
||||
Before: 3 .get('document',...) sites
|
||||
After: 0
|
||||
Delta: -3 (expected: -3)
|
||||
```
|
||||
|
||||
## §Phase 10: Small-batch aggregates (33 sites)
|
||||
|
||||
**WHERE:**
|
||||
- SessionInsights: `src/gui_2.py:4926-4931` (6 sites)
|
||||
- DiscussionSettings: `src/gui_2.py:3536` (3 sites: temperature, top_p, max_output_tokens)
|
||||
- CustomSlice: `src/gui_2.py:4049,4055,4091,4092,5952,5958,5979,5980` + subscripts at 4034,4054,4056,5920,5957,5959 (10 sites)
|
||||
- MMAUsageStats: `src/gui_2.py:2200,2201,2202,2217,6609,6784,6785,6786` (8 sites)
|
||||
- ProviderPayload: `src/app_controller.py:2278,2291` (2 sites)
|
||||
- UIPanelConfig: `src/app_controller.py:2070,2071,2072` (3 sites)
|
||||
- PathInfo: `src/app_controller.py:1976,1980,1986,1987` (4 sites)
|
||||
|
||||
**Task 10.1: SessionInsights (6 sites)**
|
||||
|
||||
Read the context first:
|
||||
```bash
|
||||
manual-slop_get_file_slice --path src/gui_2.py --start_line 4920 --end_line 4940
|
||||
```
|
||||
|
||||
```python
|
||||
# BEFORE:
|
||||
imgui.text(f"Total Tokens: {insights.get('total_tokens', 0):,}")
|
||||
imgui.text(f"API Calls: {insights.get('call_count', 0)}")
|
||||
imgui.text(f"Burn Rate: {insights.get('burn_rate', 0):.0f} tokens/min")
|
||||
imgui.text(f"Session Cost: ${insights.get('session_cost', 0):.4f}")
|
||||
completed = insights.get('completed_tickets', 0)
|
||||
efficiency = insights.get('efficiency', 0)
|
||||
|
||||
# AFTER:
|
||||
insights_obj = SessionInsights.from_dict(insights) if isinstance(insights, dict) else insights
|
||||
imgui.text(f"Total Tokens: {insights_obj.total_tokens:,}")
|
||||
imgui.text(f"API Calls: {insights_obj.call_count}")
|
||||
imgui.text(f"Burn Rate: {insights_obj.burn_rate:.0f} tokens/min")
|
||||
imgui.text(f"Session Cost: ${insights_obj.session_cost:.4f}")
|
||||
completed = insights_obj.completed_tickets
|
||||
efficiency = insights_obj.efficiency
|
||||
```
|
||||
|
||||
**Task 10.2: DiscussionSettings (3 sites)**
|
||||
|
||||
```bash
|
||||
manual-slop_get_file_slice --path src/gui_2.py --start_line 3530 --end_line 3545
|
||||
```
|
||||
|
||||
```python
|
||||
# BEFORE:
|
||||
imgui.same_line(); summary = f" (T:{entry.get('temperature', 0.7):.1f}, P:{entry.get('top_p', 1.0):.2f}, M:{entry.get('max_output_tokens', 0)})"
|
||||
|
||||
# AFTER:
|
||||
entry_obj = DiscussionSettings.from_dict(entry) if isinstance(entry, dict) else entry
|
||||
imgui.same_line(); summary = f" (T:{entry_obj.temperature:.1f}, P:{entry_obj.top_p:.2f}, M:{entry_obj.max_output_tokens})"
|
||||
```
|
||||
|
||||
**Task 10.3: CustomSlice (10 sites — note mutation patterns)**
|
||||
|
||||
CustomSlice is `frozen=True`. Mutations like `slc['tag'] = ...` become `slc = dataclasses.replace(slc, tag=...)` + list reassignment.
|
||||
|
||||
```python
|
||||
# BEFORE (read at gui_2.py:4049):
|
||||
current_tag = slc.get('tag', '')
|
||||
imgui.same_line(); imgui.set_next_item_width(-30); changed_comm, new_comm = imgui.input_text("##Note", slc.get('comment', ''))
|
||||
|
||||
# AFTER (per-iteration, at top of loop):
|
||||
cs = CustomSlice.from_dict(slc) if isinstance(slc, dict) else slc
|
||||
current_tag = cs.tag
|
||||
imgui.same_line(); imgui.set_next_item_width(-30); changed_comm, new_comm = imgui.input_text("##Note", cs.comment)
|
||||
```
|
||||
|
||||
For mutations (`slc['tag'] = ...`):
|
||||
```python
|
||||
# BEFORE:
|
||||
if ch_tag: slc['tag'] = tags[new_tag_idx]
|
||||
|
||||
# AFTER:
|
||||
if ch_tag:
|
||||
cs = CustomSlice.from_dict(slc) if isinstance(slc, dict) else slc
|
||||
cs = dataclasses.replace(cs, tag=tags[new_tag_idx])
|
||||
custom_slices[idx] = cs # list reassignment (the variable holding custom_slices)
|
||||
```
|
||||
|
||||
**Task 10.4: MMAUsageStats (8 sites)**
|
||||
|
||||
```bash
|
||||
manual-slop_get_file_slice --path src/gui_2.py --start_line 2195 --end_line 2225
|
||||
manual-slop_get_file_slice --path src/gui_2.py --start_line 6605 --end_line 6615
|
||||
manual-slop_get_file_slice --path src/gui_2.py --start_line 6780 --end_line 6790
|
||||
```
|
||||
|
||||
```python
|
||||
# BEFORE:
|
||||
model = stats.get('model', 'unknown')
|
||||
in_t = stats.get('input', 0)
|
||||
out_t = stats.get('output', 0)
|
||||
|
||||
# AFTER (per loop iteration or at top of function):
|
||||
stats_obj = MMAUsageStats.from_dict(stats) if isinstance(stats, dict) else stats
|
||||
model = stats_obj.model
|
||||
in_t = stats_obj.input
|
||||
out_t = stats_obj.output
|
||||
```
|
||||
|
||||
**Task 10.5: ProviderPayload (2 sites)**
|
||||
|
||||
```bash
|
||||
manual-slop_get_file_slice --path src/app_controller.py --start_line 2272 --end_line 2295
|
||||
```
|
||||
|
||||
```python
|
||||
# BEFORE:
|
||||
script = payload.get('script') or json.dumps(payload.get('args', {}), indent=1)
|
||||
output = payload.get('output', payload.get('content', ''))
|
||||
|
||||
# AFTER:
|
||||
pp = ProviderPayload.from_dict(payload) if isinstance(payload, dict) else payload
|
||||
script = pp.script or json.dumps(pp.args, indent=1)
|
||||
output = pp.output
|
||||
```
|
||||
|
||||
**Task 10.6: UIPanelConfig (3 sites)**
|
||||
|
||||
```bash
|
||||
manual-slop_get_file_slice --path src/app_controller.py --start_line 2065 --end_line 2080
|
||||
```
|
||||
|
||||
```python
|
||||
# BEFORE:
|
||||
self.ui_separate_message_panel = gui_cfg.get('separate_message_panel', False)
|
||||
self.ui_separate_response_panel = gui_cfg.get('separate_response_panel', False)
|
||||
self.ui_separate_tool_calls_panel = gui_cfg.get('separate_tool_calls_panel', False)
|
||||
|
||||
# AFTER:
|
||||
gui = UIPanelConfig.from_dict(gui_cfg) if isinstance(gui_cfg, dict) else gui_cfg
|
||||
self.ui_separate_message_panel = gui.separate_message_panel
|
||||
self.ui_separate_response_panel = gui.separate_response_panel
|
||||
self.ui_separate_tool_calls_panel = gui.separate_tool_calls_panel
|
||||
```
|
||||
|
||||
**Task 10.7: PathInfo (4 sites, includes nested dict access)**
|
||||
|
||||
```bash
|
||||
manual-slop_get_file_slice --path src/app_controller.py --start_line 1970 --end_line 1995
|
||||
```
|
||||
|
||||
```python
|
||||
# BEFORE:
|
||||
lpath = Path(proj_paths['logs_dir'])
|
||||
spath = Path(proj_paths['scripts_dir'])
|
||||
self.ui_logs_dir = str(path_info['logs_dir']['path'])
|
||||
self.ui_scripts_dir = str(path_info['scripts_dir']['path'])
|
||||
|
||||
# AFTER (if proj_paths and path_info are PathInfo dataclasses):
|
||||
lpath = Path(proj_paths.logs_dir)
|
||||
spath = Path(proj_paths.scripts_dir)
|
||||
self.ui_logs_dir = str(path_info.logs_dir.path if hasattr(path_info.logs_dir, 'path') else path_info.logs_dir)
|
||||
self.ui_scripts_dir = str(path_info.scripts_dir.path if hasattr(path_info.scripts_dir, 'path') else path_info.scripts_dir)
|
||||
|
||||
# AFTER (if proj_paths and path_info are dicts):
|
||||
proj_paths = PathInfo.from_dict(proj_paths) if isinstance(proj_paths, dict) else proj_paths
|
||||
path_info = PathInfo.from_dict(path_info) if isinstance(path_info, dict) else path_info
|
||||
lpath = Path(proj_paths.logs_dir)
|
||||
spath = Path(proj_paths.scripts_dir)
|
||||
self.ui_logs_dir = str(path_info.logs_dir if isinstance(path_info.logs_dir, str) else path_info.logs_dir.get('path', ''))
|
||||
self.ui_scripts_dir = str(path_info.scripts_dir if isinstance(path_info.scripts_dir, str) else path_info.scripts_dir.get('path', ''))
|
||||
```
|
||||
|
||||
(Per-site decision: if the dict has nested structure, the migration is partial; document in commit.)
|
||||
|
||||
**HOW:** `manual-slop_edit_file` per task. Read the surrounding context first for each.
|
||||
|
||||
**SAFETY:**
|
||||
```bash
|
||||
git grep -nE "\.get\('total_tokens',|\.get\('burn_rate',|\.get\('session_cost',|\.get\('temperature',|\.get\('top_p',|\.get\('max_output_tokens'," -- 'src/gui_2.py' | wc -l
|
||||
# Expect: 0
|
||||
git grep -nE "\.get\('separate_message_panel',|\.get\('separate_response_panel',|\.get\('separate_tool_calls_panel'," -- 'src/app_controller.py' | wc -l
|
||||
# Expect: 0
|
||||
uv run python -m pytest tests/test_session_insights.py tests/test_discussion_settings.py tests/test_custom_slice.py tests/test_mma_usage_stats.py tests/test_provider_payload.py tests/test_ui_panel_config.py tests/test_path_info.py tests/test_app_controller.py tests/test_gui_2.py -x --timeout=120
|
||||
# Expect: all pass
|
||||
```
|
||||
|
||||
**MODIFY-IF-FAILS:**
|
||||
- If grep shows non-zero: search for any `.get(...)` you missed for each small-batch aggregate. Add additional migrations.
|
||||
- If pytest fails: STOP. Likely cause: the dataclass field names differ from the dict keys. Check `src/type_aliases.py` for the exact field names.
|
||||
|
||||
**COMMIT (per task):** `refactor(gui_2,app_controller): migrate SessionInsights consumers to direct field access` (per aggregate)
|
||||
|
||||
**Each commit message body MUST include:**
|
||||
```
|
||||
Phase 10.N: <aggregate name>
|
||||
Before: N .get('<key>',...) sites
|
||||
After: 0
|
||||
Delta: -N
|
||||
```
|
||||
|
||||
## §Phase 11: Re-measure + verification
|
||||
|
||||
```bash
|
||||
git grep -nE "\.get\('[a-z_]+'," -- 'src/*.py' | wc -l
|
||||
# Expect: < 15 (collapsed-codepath only)
|
||||
|
||||
git grep -nE "\[[ ]*'[a-z_]+'[ ]*\]" -- 'src/*.py' | wc -l
|
||||
# Expect: ~50 (most subscript sites are handler-map / shader_uniforms / project config — genuinely collapsed-codepath)
|
||||
|
||||
uv run python -c "
|
||||
import sys
|
||||
sys.path.insert(0, 'scripts/code_path_audit')
|
||||
sys.path.insert(0, 'src')
|
||||
from code_path_audit import build_pcg
|
||||
from code_path_audit_ssdl import count_branches_in_function
|
||||
pcg = build_pcg('src').data
|
||||
metadata_consumers = pcg.consumers.get('Metadata', [])
|
||||
total = sum(2 ** count_branches_in_function(f, 'src') for f in metadata_consumers)
|
||||
print(f'Post-track effective codepaths: {total:.3e} (baseline 4.014e+22)')
|
||||
"
|
||||
# Expect: < 1e+21
|
||||
|
||||
uv run python scripts/audit_weak_types.py --strict
|
||||
uv run python scripts/generate_type_registry.py --check
|
||||
uv run python scripts/audit_main_thread_imports.py
|
||||
uv run python scripts/audit_no_models_config_io.py
|
||||
uv run python scripts/audit_code_path_audit_coverage.py --input-dir docs/reports/code_path_audit/latest --strict
|
||||
uv run python scripts/audit_exception_handling.py --strict
|
||||
uv run python scripts/audit_optional_in_3_files.py --strict
|
||||
# All exit 0
|
||||
|
||||
uv run python scripts/run_tests_batched.py
|
||||
# Expect: 10/11 PASS (RAG flake acceptable)
|
||||
```
|
||||
|
||||
**MODIFY-IF-FAILS (metric didn't drop):**
|
||||
- If effective codepaths is still 4.014e+22: search for any remaining `.get('key', default)` on known aggregates. The metric is dominated by these sites; if any remain, the metric won't drop.
|
||||
- If 7 audit gates fail: STOP. Read which audit failed. Likely a new dataclass field name diverges from the wire format. Modify the dataclass or the wire format.
|
||||
- If batched tests fail: STOP. Read the failure. Likely a dataclass-from-dict conversion is producing wrong field values.
|
||||
|
||||
**DO NOT just accept "metric didn't drop".** Keep modifying until it drops OR until the only remaining `.get()` sites are documented collapsed-codepath (Phase 12).
|
||||
|
||||
## §Phase 12: Collapsed-codepath audit
|
||||
|
||||
For any remaining `.get()` + subscript sites after Phase 11, write `docs/reports/collapsed_codepath_audit_20260626.md`:
|
||||
|
||||
```bash
|
||||
git grep -nE "\.get\('[a-z_]+'," -- 'src/*.py' > /tmp/remaining_get.txt
|
||||
git grep -nE "\[[ ]*'[a-z_]+'[ ]*\]" -- 'src/*.py' > /tmp/remaining_subscript.txt
|
||||
```
|
||||
|
||||
For each remaining site, classify as:
|
||||
- **collapsed-codepath (TOML config):** `self.project.get('paths', {})`, `self.config.get('ai', {})`, `self.project.get('conductor', {})` etc. — keep as `.get()`.
|
||||
- **collapsed-codepath (handler-map):** `_predefined_callbacks[...]`, `_gettable_fields[...]` — keep as subscript.
|
||||
- **collapsed-codepath (shader-uniforms):** `app.shader_uniforms['crt']` — keep.
|
||||
- **collapsed-codepath (handler map / dispatch):** keep.
|
||||
- **collateral (genuinely dict):** sites where the variable is genuinely a `dict` from JSON wire or external source — keep.
|
||||
|
||||
Write the audit doc with per-site classification + per-site justification + per-site decision (stay vs fix).
|
||||
|
||||
**COMMIT:** `docs(audit): collapsed-codepath audit for remaining access sites`
|
||||
|
||||
## §Acceptance Criteria (Definition of Done)
|
||||
|
||||
| # | Criterion | Verification |
|
||||
|---|---|---|
|
||||
| VC1 | All `.get('key', default)` sites on known aggregates replaced | `git grep -cE "\.get\('[a-z_]+'," -- 'src/*.py'` returns < 15 |
|
||||
| VC2 | All `[ 'key' ]` subscript sites on known aggregates replaced | `git grep -cE "\[[ ]*'[a-z_]+'[ ]*\]" -- 'src/*.py'` returns < 55 (excluding handler-maps + shader_uniforms) |
|
||||
| VC3 | Per-phase guard enforced | Each phase commit message has "Before/After/Delta" |
|
||||
| VC4 | Effective codepaths drops by ≥ 1 order of magnitude | `< 1e+21` |
|
||||
| VC5 | All 7 audit gates pass `--strict` | All exit 0 |
|
||||
| VC6 | 10/11 batched test tiers PASS | `scripts/run_tests_batched.py` → 10/11 |
|
||||
| VC7 | Collapsed-codepath audit written | `docs/reports/collapsed_codepath_audit_20260626.md` exists |
|
||||
| VC8 | No "no-op" classifications | No phase commit message says "no-op per FR2" |
|
||||
| VC9 | No parallel dataclass definitions | All FileItem references resolve to `models.FileItem`; all ToolCall references resolve to `openai_schemas.ToolCall` |
|
||||
| VC10 | Per-site type checks documented | Per-phase commits include "var was dataclass: yes/no; converted via from_dict: yes/no" |
|
||||
|
||||
## §Tier 2 / Tier 3 Hard Rules
|
||||
|
||||
1. **NEVER use `git restore`, `git checkout --`, `git reset`, or `git revert`.** Per AGENTS.md hard ban. If a phase's count delta doesn't match the plan, MODIFY the migration (add more sites, reclassify, fix the wrong sites). Do NOT throw away the work.
|
||||
|
||||
2. **NEVER classify a phase as "no-op per FR2 collapsed-codepath audit."** Each phase has a planned N sites. After the phase, exactly N sites must be migrated. If not, ADD more migrations to make the count match.
|
||||
|
||||
3. **NEVER use `if key in dict else default` as a "migration."** The migration is `var = Aggregate.from_dict(var)` + direct attribute access. The dict-with-`in`-check pattern is a half-measure that does NOT achieve the per-attribute access that the spec requires.
|
||||
|
||||
4. **NEVER batch commits.** One atomic commit per task (or per phase). Per-task commits enable precise rollback via `git revert` (oh wait — don't use git revert). Per-task commits enable precise FIX via additional commits.
|
||||
|
||||
5. **NEVER add comments to source code.** Per AGENTS.md. Documentation lives in `/docs`.
|
||||
|
||||
6. **NEVER use the native `edit` tool on Python files.** Use `manual-slop_edit_file`, `manual-slop_py_update_definition`, `manual-slop_py_add_def`, or `manual-slop_set_file_slice`.
|
||||
|
||||
7. **NEVER create new `src/<thing>.py` files.** Per AGENTS.md. Helpers go in the parent module.
|
||||
|
||||
8. **NEVER add new dataclasses.** Per this track's spec, all dataclasses already exist. Reuse them.
|
||||
|
||||
9. **NEVER modify existing dataclass definitions.** Per this track's spec, dataclass definitions are frozen. If a field type is wrong, that's a separate track.
|
||||
|
||||
10. **NEVER skip a failing test with `@pytest.mark.skip`.** Fix the bug.
|
||||
|
||||
11. **NEVER exceed 5 nesting levels.** Extract to functions.
|
||||
|
||||
12. **NEVER modify `src/code_path_audit*.py`.** The audit infrastructure is correct.
|
||||
|
||||
13. **NEVER promote `Metadata: TypeAlias = dict[str, Any]` to a shared mega-dataclass.** Per the spec FR1 + FR2 (the user explicitly rejected this on 2026-06-25).
|
||||
|
||||
14. **STOP AND ASK if any site's variable type is unclear.** Write a 1-sentence question. Wait for the user. Do not invent a reconciliation.
|
||||
|
||||
15. **If a commit breaks more than 2 tests, STOP.** Read the failures. Identify the root cause. Modify the commit (amend or add a fixup). Do not ship broken state.
|
||||
|
||||
## §Per-Phase Tier 2 Review Checklist
|
||||
|
||||
Before approving each phase, Tier 2 verifies:
|
||||
|
||||
1. The commit message has "Before: N, After: M, Delta: -K" with K matching the planned count.
|
||||
2. The relevant `git grep` count decreased by exactly the planned K.
|
||||
3. The relevant `pytest` files pass.
|
||||
4. No audit gate regressed.
|
||||
5. The batched test suite still passes 10/11 tiers.
|
||||
6. No "no-op" or "REVERT" or "skipped" in the commit message.
|
||||
|
||||
If any check fails: **DO NOT APPROVE.** Tell Tier 3 what to fix. Tier 3 modifies the migration and re-commits.
|
||||
|
||||
## §Anti-Pattern Guard (per AGENTS.md)
|
||||
|
||||
If you observe any of these patterns in your own work, STOP and re-read AGENTS.md:
|
||||
|
||||
1. **The Deduction Loop**: running a test 4+ times in one investigation. STOP after 2 failures.
|
||||
2. **The Report-Instead-of-Fix Pattern**: writing a 200-line status report instead of fixing.
|
||||
3. **The Scope-Creep Track-Doc Pattern**: writing a 5-phase spec for a 1-line fix.
|
||||
4. **The Inherited-Cruft Pattern**: trying to "fix" a broken file from a previous agent.
|
||||
5. **No Diagnostic Noise in Production**: `sys.stderr.write` lines in `src/*.py`.
|
||||
6. **The "I Am Not Going To Attempt Another Fix" Surrender**: only after the 5-step protocol.
|
||||
7. **The Verbose-Commit-Message Pattern**: commit messages > 15 lines.
|
||||
8. **The Isolated-Pass Verification Fallacy**: verifying in isolation but not in batch.
|
||||
9. **The Workspace-Path Drift Pattern**: using `/tmp` or env vars for test paths.
|
||||
10. **The No-Op Classification Shortcut**: marking phases complete without doing the work. (banned by Hard Rule #2)
|
||||
|
||||
## §See also
|
||||
|
||||
- `conductor/tracks/type_alias_unfuck_20260626/spec.md` — the track spec
|
||||
- `conductor/tracks/metadata_promotion_20260624/spec.md` — the previous track (now superseded)
|
||||
- `conductor/tracks/metadata_promotion_20260624/state.toml` — honest state of the previous track
|
||||
- `conductor/code_styleguides/type_aliases.md` §2.5 — the per-aggregate dataclass rule
|
||||
- `conductor/code_styleguides/data_oriented_design.md` — canonical DOD reference
|
||||
- `conductor/AGENTS.md` — hard bans (NEVER use `git restore`, `git checkout --`, `git reset`, `git revert`)
|
||||
- `src/type_aliases.py` — the existing per-aggregate dataclasses (REUSE, do not modify)
|
||||
- `src/openai_schemas.py` — canonical ToolCall, ChatMessage, UsageStats
|
||||
- `src/models.py:533` — canonical FileItem
|
||||
- `src/models.py:302` — canonical Ticket
|
||||
@@ -0,0 +1,460 @@
|
||||
# Track Specification: type_alias_unfuck_20260626
|
||||
|
||||
## Overview
|
||||
|
||||
**This is the MINIMAL track to fix the type-usage problem.** It exists because `metadata_promotion_20260624` became a tar pit. This track is scoped to JUST the consumer migration work (Phases 1-10 of the original plan) with strict per-phase guards that prevent the no-op shortcut.
|
||||
|
||||
**Goal:** Replace the 67 remaining `.get('key', default)` sites and ~80 subscript sites in `src/*.py` with direct field access on existing per-aggregate dataclasses.
|
||||
|
||||
**Scope:** 12 small phases, one per aggregate. Each phase migrates a specific aggregate's consumers. Each phase has a hard guard: `.get()` count for that aggregate must decrease by exactly N (the planned sites). If not, the code is MODIFIED until it does.
|
||||
|
||||
**Non-scope:** No new dataclasses (Phase 0 of `metadata_promotion_20260624` already added them). No metric-driven design changes. No test rewrites unless tests break.
|
||||
|
||||
## Current State Audit (master `b4bd772d`, measured 2026-06-25)
|
||||
|
||||
| Metric | Value | Source |
|
||||
|---|---:|---|
|
||||
| `.get('key', default)` sites in `src/*.py` | **67** | `git grep -cE "\.get\('[a-z_]+'," -- 'src/*.py' \| awk -F: '{s+=$2} END {print s}'` |
|
||||
| Subscript `[ 'key' ]` sites in `src/*.py` | ~80 | `git grep -cE "\[[ ]*'[a-z_]+'[ ]*\]" -- 'src/*.py' \| awk -F: '{s+=$2} END {print s}'` |
|
||||
| Existing per-aggregate dataclasses | **12 in src/type_aliases.py** + 4 reused (Ticket, FileItem, ToolCall, ChatMessage, UsageStats) | `git grep "^class .*dataclass" src/type_aliases.py` |
|
||||
| Effective codepaths | **4.014e+22** | baseline from `metadata_promotion_20260624` |
|
||||
|
||||
### Per-aggregate breakdown of remaining `.get()` sites
|
||||
|
||||
| Aggregate | Sites | Primary files |
|
||||
|---|---:|---|
|
||||
| Ticket | 0 (Phase 1 of metadata_promotion_20260624 done; SKIP this track) | n/a |
|
||||
| FileItem | 4 | `src/ai_client.py:2565,2807,2898`, `src/app_controller.py:3508` |
|
||||
| CommsLogEntry | 5 | `src/app_controller.py:2277,2302,2310`, `src/gui_2.py:5803`, `src/synthesis_formatter.py:24,37` |
|
||||
| HistoryMessage | 2 | `src/synthesis_formatter.py:24,37` (overlaps with CommsLogEntry; classify per-site) |
|
||||
| ChatMessage | 27 | `src/ai_client.py` per-vendor send paths |
|
||||
| UsageStats | 4 | `src/app_controller.py:2304,2305,2308,2309` |
|
||||
| ToolCall | 3 | `src/mcp_client.py:1707,1708,1714` |
|
||||
| ToolDefinition | 4 | `src/mcp_client.py:1970`, `src/gui_2.py:5876,5878` |
|
||||
| RAGChunk | 3 | `src/aggregate.py:3259`, `src/app_controller.py:251,4162` |
|
||||
| SessionInsights | 6 | `src/gui_2.py:4926-4931` |
|
||||
| DiscussionSettings | 3 | `src/gui_2.py:3535` |
|
||||
| CustomSlice | 10 | `src/gui_2.py:4048,4054,4090,5953,5959,5980,4033,5921` |
|
||||
| MMAUsageStats | 6 | `src/gui_2.py:2199-2201,2216,6610` |
|
||||
| ProviderPayload | 4 | `src/app_controller.py:2274,2287` |
|
||||
| UIPanelConfig | 3 | `src/app_controller.py:2068-2070` |
|
||||
| PathInfo | 4 | `src/app_controller.py:1974,1978,1984,1985` |
|
||||
| Other (collapsed-codepath) | unknown until Phase 12 audit | various |
|
||||
|
||||
**Total: ~88 sites** (some overlap between aggregates; exact sites identified per-phase below).
|
||||
|
||||
## Goals
|
||||
|
||||
| ID | Goal | Acceptance |
|
||||
|---|---|---|
|
||||
| G1 | All `.get('key', default)` sites on known aggregates replaced with direct field access | `git grep -nE "\.get\('[a-z_]+'," -- 'src/*.py' \| wc -l` returns 0 (excluding collapsed-codepath sites documented in Phase 12) |
|
||||
| G2 | All `[ 'key' ]` subscript sites on known aggregates replaced with direct field access | `git grep -nE "\[[ ]*'[a-z_]+'[ ]*\]" -- 'src/*.py' \| wc -l` returns 0 (excluding collapsed-codepath sites) |
|
||||
| G3 | Per-phase guard enforced (count decreases by exactly N; if not, modify until it does) | Each phase commit has a "before: N, after: M, delta: D" line in the commit message; if delta ≠ expected, MODIFY the code and recommit |
|
||||
| G4 | Effective codepaths drops by ≥ 1 order of magnitude | `compute_effective_codepaths` returns `< 1e+21` (was 4.014e+22) |
|
||||
| G5 | All 7 audit gates pass `--strict` (no regression) | All exit 0 |
|
||||
| G6 | All existing tests pass (10/11 batched tiers — RAG flake acceptable) | `scripts/run_tests_batched.py` → 10/11 PASS |
|
||||
| G7 | Collapsed-codepath sites documented (Phase 12) | `docs/reports/collapsed_codepath_audit_20260626.md` exists with per-site justification |
|
||||
|
||||
## Non-Goals
|
||||
|
||||
- Modifying dataclass definitions in `src/type_aliases.py` (Phase 0 of `metadata_promotion_20260624` is frozen for this track)
|
||||
- Fixing drifted field types (separate track if needed; this track uses whatever the dataclasses currently define)
|
||||
- Adding new `src/<thing>.py` files
|
||||
- Creating any further followup tracks (this is the minimum; no more layers)
|
||||
|
||||
## Functional Requirements
|
||||
|
||||
### FR1: Per-phase hard guard (THE key rule)
|
||||
|
||||
**Every phase has a specific `.get()` site count to migrate.** If the after-commit count for the phase's aggregate is NOT exactly N sites lower than before, the code is MODIFIED until it matches. NEVER use `git restore`, `git checkout --`, `git reset`, or `git revert` per AGENTS.md hard ban. NEVER blow away the work. FIX IT.
|
||||
|
||||
**Before each phase commit:**
|
||||
```bash
|
||||
git grep -nE "\.get\('[a-z_]+'," -- 'src/*.py' | wc -l
|
||||
```
|
||||
|
||||
**After each phase commit:**
|
||||
```bash
|
||||
git grep -nE "\.get\('[a-z_]+'," -- 'src/*.py' | wc -l
|
||||
```
|
||||
|
||||
**The commit message MUST include:**
|
||||
```
|
||||
Phase N: <aggregate name>
|
||||
Before: <N> .get() sites
|
||||
After: <M> .get() sites
|
||||
Delta: <N-M> (expected: -<planned>)
|
||||
```
|
||||
|
||||
**If delta != -planned:** the migration is incomplete. Look at the remaining `.get()` sites for the aggregate, ADD more migrations until the count matches. Recommit (amend the previous commit or add a fixup commit). DO NOT delete the work.
|
||||
|
||||
### FR2: Use the pattern: `var = Aggregate.from_dict(var)` before access
|
||||
|
||||
For sites where the variable is currently a dict (constructed on-the-fly or from JSON), the migration adds ONE line at the top of the function:
|
||||
|
||||
```python
|
||||
# BEFORE:
|
||||
def _process_entry(entry: Metadata) -> None:
|
||||
tier = entry.get('source_tier', 'main')
|
||||
model = entry.get('model', 'unknown')
|
||||
|
||||
# AFTER:
|
||||
def _process_entry(entry: Metadata) -> None:
|
||||
entry = CommsLogEntry.from_dict(entry) # ← ONE LINE ADDED
|
||||
tier = entry.source_tier
|
||||
model = entry.model
|
||||
```
|
||||
|
||||
This is the FULL migration. NOT `.get()` → `if key in dict else default`. The dataclass is the destination; the dict is the source. Convert once, then use direct access.
|
||||
|
||||
### FR3: No "no-op" shortcuts
|
||||
|
||||
If a phase has 0 actual `.get()` sites to migrate (because the variable is always a dataclass or the sites don't exist), the phase work is different: ADD migration sites from the per-aggregate table above. The table shows N planned sites per aggregate; each must be migrated.
|
||||
|
||||
There is no "Phase 2: no-op per FR2 collapsed-codepath audit" commit allowed in this track.
|
||||
|
||||
## Per-Phase Task List
|
||||
|
||||
### Phase 0: Pre-flight (no commits)
|
||||
|
||||
```bash
|
||||
# Baseline capture
|
||||
git grep -nE "\.get\('[a-z_]+'," -- 'src/*.py' > /tmp/before.txt
|
||||
wc -l /tmp/before.txt
|
||||
# Expect: 67
|
||||
|
||||
git grep -nE "\[[ ]*'[a-z_]+'[ ]*\]" -- 'src/*.py' > /tmp/before_subscript.txt
|
||||
wc -l /tmp/before_subscript.txt
|
||||
# Expect: ~80
|
||||
|
||||
# Confirm 7 audit gates pass --strict (note any pre-existing failures)
|
||||
uv run python scripts/audit_weak_types.py --strict
|
||||
uv run python scripts/generate_type_registry.py --check
|
||||
uv run python scripts/audit_main_thread_imports.py
|
||||
uv run python scripts/audit_no_models_config_io.py
|
||||
uv run python scripts/audit_code_path_audit_coverage.py --input-dir docs/reports/code_path_audit/latest --strict
|
||||
uv run python scripts/audit_exception_handling.py --strict
|
||||
uv run python scripts/audit_optional_in_3_files.py --strict
|
||||
```
|
||||
|
||||
**STOP if any pre-existing failure is not in the baseline report. Report to user.**
|
||||
|
||||
### Phase 1: Ticket consumers (SKIP — already done in metadata_promotion_20260624)
|
||||
|
||||
No work. Move to Phase 2.
|
||||
|
||||
### Phase 2: FileItem consumers (4 sites)
|
||||
|
||||
**WHERE:**
|
||||
- `src/ai_client.py:2565,2807,2898`: `fi.get('path', 'attachment')` × 3
|
||||
- `src/app_controller.py:3508`: `f['path'] for f in file_items` × 1
|
||||
|
||||
**Pattern:**
|
||||
```python
|
||||
# BEFORE:
|
||||
user_content = f"[IMAGE: {fi.get('path', 'attachment')}]\n{user_content}"
|
||||
|
||||
# AFTER (if fi is dataclass):
|
||||
user_content = f"[IMAGE: {fi.path or 'attachment'}]\n{user_content}"
|
||||
|
||||
# AFTER (if fi is dict):
|
||||
fi = FileItem.from_dict(fi) # at top of function
|
||||
user_content = f"[IMAGE: {fi.path or 'attachment'}]\n{user_content}"
|
||||
```
|
||||
|
||||
**Per-site verification:**
|
||||
```bash
|
||||
git grep -nE "\.get\('path'," -- 'src/ai_client.py' | wc -l
|
||||
# Expect: 0
|
||||
```
|
||||
|
||||
**Acceptance:** `.get('path', default)` count in src/ai_client.py + src/app_controller.py decreases by 4.
|
||||
|
||||
### Phase 3: CommsLogEntry consumers (5 sites)
|
||||
|
||||
**WHERE:**
|
||||
- `src/app_controller.py:2277,2302,2310`: `entry.get('source_tier', 'main')`, `entry.get('source_tier', 'main')`, `entry.get('model', 'unknown')` × 3
|
||||
- `src/gui_2.py:5803`: `entry.get('source_tier', 'main')` × 1
|
||||
- `src/synthesis_formatter.py:24,37`: `msg.get('role', 'unknown')`, `msg.get('content', '')` × 4 (these may be HistoryMessage; classify per-site)
|
||||
|
||||
**Pattern:**
|
||||
```python
|
||||
# BEFORE:
|
||||
'source_tier': entry.get('source_tier', 'main'),
|
||||
|
||||
# AFTER:
|
||||
entry = CommsLogEntry.from_dict(entry) # at top of function
|
||||
'source_tier': entry.source_tier,
|
||||
```
|
||||
|
||||
**Per-site verification:**
|
||||
```bash
|
||||
git grep -nE "entry\.get\('source_tier'," -- 'src/app_controller.py' | wc -l
|
||||
# Expect: 0
|
||||
```
|
||||
|
||||
**Acceptance:** `.get('source_tier', default)` + `.get('role', default)` + `.get('content', default)` counts decrease by 5.
|
||||
|
||||
### Phase 4: HistoryMessage consumers (2 sites, if not in Phase 3)
|
||||
|
||||
**WHERE:**
|
||||
- `src/synthesis_formatter.py:24,37` (if classified as HistoryMessage rather than CommsLogEntry in Phase 3)
|
||||
|
||||
**Pattern:**
|
||||
```python
|
||||
# BEFORE:
|
||||
f"{msg.get('role', 'unknown')}: {msg.get('content', '')}"
|
||||
|
||||
# AFTER:
|
||||
msg = HistoryMessage.from_dict(msg)
|
||||
f"{msg.role}: {msg.content or ''}"
|
||||
```
|
||||
|
||||
**Acceptance:** HistoryMessage sites migrated; CommsLogEntry sites classified in Phase 3.
|
||||
|
||||
### Phase 5: ChatMessage into per-vendor send paths (27 sites)
|
||||
|
||||
**WHERE:** `src/ai_client.py` (8 vendor send methods: `_send_anthropic`, `_send_deepseek`, `_send_gemini`, `_send_gemini_cli`, `_send_minimax`, `_send_qwen`, `_send_llama`, `_send_grok`)
|
||||
|
||||
**Pattern:**
|
||||
```python
|
||||
# BEFORE:
|
||||
for msg in anthropic_history:
|
||||
if msg.get("role") == "user":
|
||||
messages.append({"role": "user", "content": msg.get("content", "")})
|
||||
|
||||
# AFTER:
|
||||
for msg in anthropic_history:
|
||||
cm = msg if isinstance(msg, ChatMessage) else ChatMessage.from_dict(msg)
|
||||
if cm.role == "user":
|
||||
messages.append(cm.to_dict())
|
||||
```
|
||||
|
||||
**Per-site verification:** Each send method's `msg.get(` count decreases.
|
||||
|
||||
**Acceptance:** All 8 send methods use ChatMessage; total `.get('role', default)` + `.get('content', default)` sites in src/ai_client.py decrease by 27.
|
||||
|
||||
### Phase 6: UsageStats into per-call usage aggregation (4 sites)
|
||||
|
||||
**WHERE:**
|
||||
- `src/app_controller.py:2304,2305,2308,2309`: `u.get('input_tokens', 0)`, `u.get('output_tokens', 0)`
|
||||
|
||||
**Pattern:**
|
||||
```python
|
||||
# BEFORE:
|
||||
new_mma_usage[tier]['input'] += u.get('input_tokens', 0) or 0
|
||||
|
||||
# AFTER:
|
||||
u = UsageStats.from_dict(u) if isinstance(u, dict) else u
|
||||
new_mma_usage[tier] = dataclasses.replace(
|
||||
new_mma_usage[tier],
|
||||
input=new_mma_usage[tier].input + (u.input_tokens or 0),
|
||||
)
|
||||
```
|
||||
|
||||
**Acceptance:** All `u.get('input_tokens', ...)` + `u.get('output_tokens', ...)` in src/app_controller.py:2299-2311 replaced.
|
||||
|
||||
### Phase 7: ToolCall into tool loop (3 sites)
|
||||
|
||||
**WHERE:**
|
||||
- `src/mcp_client.py:1707,1708,1714`: `result['tools']`, `t['name']`, `c.get('text', '')` × 3
|
||||
|
||||
**Pattern:**
|
||||
```python
|
||||
# BEFORE:
|
||||
for t in result['tools']:
|
||||
self.tools[t['name']] = t
|
||||
|
||||
# AFTER:
|
||||
result = MCPToolResult.from_dict(result)
|
||||
for t in result.tools:
|
||||
self.tools[t.name] = t
|
||||
```
|
||||
|
||||
**Acceptance:** `result['tools']` and `t['name']` replaced with `.tools` and `.name`.
|
||||
|
||||
### Phase 8: ToolDefinition consumers (4 sites)
|
||||
|
||||
**WHERE:**
|
||||
- `src/mcp_client.py:1970`: `tinfo.get('description', '')`
|
||||
- `src/gui_2.py:5876,5878`: `tinfo.get('server', 'unknown')`, `tinfo.get('description', '')`
|
||||
|
||||
**Pattern:**
|
||||
```python
|
||||
# BEFORE:
|
||||
'description': tinfo.get('description', '')
|
||||
|
||||
# AFTER:
|
||||
tinfo = ToolDefinition.from_dict(tinfo) if isinstance(tinfo, dict) else tinfo
|
||||
'description': tinfo.description,
|
||||
```
|
||||
|
||||
**Acceptance:** All `.get('description', default)` on ToolDefinition consumers replaced.
|
||||
|
||||
### Phase 9: RAGChunk consumers (3 sites)
|
||||
|
||||
**WHERE:**
|
||||
- `src/aggregate.py:3259`, `src/app_controller.py:251,4162`: `chunk.get('document', '')`
|
||||
|
||||
**Pattern:**
|
||||
```python
|
||||
# BEFORE:
|
||||
context_block += f"### Chunk {i+1} (Source: {path})\n{chunk.get('document', '')}\n\n"
|
||||
|
||||
# AFTER:
|
||||
chunk = RAGChunk.from_dict(chunk) if isinstance(chunk, dict) else chunk
|
||||
context_block += f"### Chunk {i+1} (Source: {path})\n{chunk.document}\n\n"
|
||||
```
|
||||
|
||||
**Acceptance:** All `chunk.get('document', ...)` replaced.
|
||||
|
||||
### Phase 10: Small-batch aggregates (33 sites)
|
||||
|
||||
**WHERE:**
|
||||
- SessionInsights: `src/gui_2.py:4926-4931` (6 sites)
|
||||
- DiscussionSettings: `src/gui_2.py:3535` (3 sites)
|
||||
- CustomSlice: `src/gui_2.py:4048,4054,4090,5953,5959,5980,4033,5921` (10 sites)
|
||||
- MMAUsageStats: `src/gui_2.py:2199-2201,2216,6610` (6 sites)
|
||||
- ProviderPayload: `src/app_controller.py:2274,2287` (4 sites)
|
||||
- UIPanelConfig: `src/app_controller.py:2068-2070` (3 sites)
|
||||
- PathInfo: `src/app_controller.py:1974,1978,1984,1985` (4 sites, includes nested `path_info['logs_dir']['path']`)
|
||||
|
||||
**Pattern:** Per-aggregate `from_dict()` + direct field access.
|
||||
|
||||
**Note on CustomSlice mutations:** `slc['tag'] = tags[new_tag_idx]` (mutation) becomes:
|
||||
```python
|
||||
slc = CustomSlice.from_dict(slc)
|
||||
slc = dataclasses.replace(slc, tag=tags[new_tag_idx])
|
||||
# Then list reassignment:
|
||||
custom_slices[idx] = slc
|
||||
```
|
||||
|
||||
**Acceptance:** All small-batch `.get()` + subscript sites replaced.
|
||||
|
||||
### Phase 11: Re-measure + verification
|
||||
|
||||
```bash
|
||||
git grep -nE "\.get\('[a-z_]+'," -- 'src/*.py' | wc -l
|
||||
# Expect: 0 (or only collapsed-codepath sites)
|
||||
|
||||
git grep -nE "\[[ ]*'[a-z_]+'[ ]*\]" -- 'src/*.py' | wc -l
|
||||
# Expect: ~0 (or only collapsed-codepath sites)
|
||||
|
||||
uv run python -c "
|
||||
import sys
|
||||
sys.path.insert(0, 'scripts/code_path_audit')
|
||||
sys.path.insert(0, 'src')
|
||||
from code_path_audit import build_pcg
|
||||
from code_path_audit_ssdl import count_branches_in_function
|
||||
pcg = build_pcg('src').data
|
||||
metadata_consumers = pcg.consumers.get('Metadata', [])
|
||||
total = sum(2 ** count_branches_in_function(f, 'src') for f in metadata_consumers)
|
||||
print(f'Post-track effective codepaths: {total:.3e} (baseline 4.014e+22)')
|
||||
"
|
||||
# Expect: < 1e+21 (target: ≥1 order of magnitude drop)
|
||||
|
||||
uv run python scripts/run_tests_batched.py
|
||||
# Expect: 10/11 PASS
|
||||
```
|
||||
|
||||
**Acceptance:** All 10 VCs pass.
|
||||
|
||||
### Phase 12: Collapsed-codepath audit (FR7)
|
||||
|
||||
For any remaining `.get()` + subscript sites after Phase 11, classify as collapsed-codepath with per-site justification:
|
||||
|
||||
```bash
|
||||
git grep -nE "\.get\('[a-z_]+'," -- 'src/*.py' > /tmp/remaining.txt
|
||||
wc -l /tmp/remaining.txt
|
||||
# Expect: ~10-15 (only TOML config, JSON wire, handler-map)
|
||||
```
|
||||
|
||||
Write `docs/reports/collapsed_codepath_audit_20260626.md` with:
|
||||
- Per-site classification (collapsed-codepath vs should-be-migrated)
|
||||
- Per-site justification
|
||||
- Decision on whether each remaining site needs a followup track or stays as-is
|
||||
|
||||
## Acceptance Criteria (Definition of Done)
|
||||
|
||||
| # | Criterion | Verification command |
|
||||
|---|---|---|
|
||||
| VC1 | All `.get('key', default)` sites on known aggregates replaced | `git grep -nE "\.get\('[a-z_]+'," HEAD -- 'src/*.py' \| wc -l` returns < 15 |
|
||||
| VC2 | All `[ 'key' ]` subscript sites on known aggregates replaced | `git grep -nE "\[[ ]*'[a-z_]+'[ ]*\]" HEAD -- 'src/*.py' \| wc -l` returns < 20 |
|
||||
| VC3 | Per-phase guard enforced (each phase decreased the count by exactly N) | Each phase commit message has "Before: N, After: M, Delta: -N" |
|
||||
| VC4 | Effective codepaths drops by ≥ 1 order of magnitude | `compute_effective_codepaths` returns `< 1e+21` |
|
||||
| VC5 | All 7 audit gates pass `--strict` | All exit 0 |
|
||||
| VC6 | 10/11 batched test tiers PASS | `scripts/run_tests_batched.py` → 10/11 |
|
||||
| VC7 | Collapsed-codepath audit written | `docs/reports/collapsed_codepath_audit_20260626.md` exists |
|
||||
| VC8 | No "no-op" classifications | No phase commit message says "no-op per FR2" |
|
||||
| VC9 | No parallel dataclass definitions | All FileItem references resolve to `models.FileItem`; all ToolCall references resolve to `openai_schemas.ToolCall` |
|
||||
| VC10 | Per-site type checks documented | Per-phase commits include "var was dataclass: yes/no; converted via from_dict: yes/no" |
|
||||
|
||||
## Hard Rules
|
||||
|
||||
1. **NO "no-op" classifications.** Each phase has a planned N sites. After the phase, exactly N sites must be migrated. If not, MODIFY the code (add more migrations) until the count matches.
|
||||
2. **NO parallel dataclass definitions.** Reuse the existing dataclasses. Do not add new ones. Do not modify the existing ones.
|
||||
3. **NO metric rationalization.** If `compute_effective_codepaths` doesn't drop after the track, MODIFY the migration (find missed sites, reclassify) until it does. Report progress to the user without rolling back.
|
||||
4. **NO inference decisions.** If a variable's type is unclear at an access site, STOP. Read the surrounding context with `manual-slop_get_file_slice` to determine the type. If still unclear, write a 1-sentence question and wait for the user.
|
||||
5. **NO shortcuts.** `if key in dict else default` is NOT a migration. `var = Aggregate.from_dict(var)` IS the migration. Use the dataclass.
|
||||
6. **NO blowing away work.** Never `git restore`, `git checkout --`, `git reset`, or `git revert` (per AGENTS.md hard ban). When something goes wrong, fix the migration. Add more sites. Reclassify. Amend the commit. Do not throw the work away.
|
||||
|
||||
## Tier 2 Invitation Prompt
|
||||
|
||||
Use this prompt to invoke Tier 2:
|
||||
|
||||
```
|
||||
Track: type_alias_unfuck_20260626 (branch: tier2/type_alias_unfuck_20260626).
|
||||
|
||||
Read the EXHAUSTIVE spec at conductor/tracks/type_alias_unfuck_20260626/spec.md (this track).
|
||||
This is the MINIMAL track to fix the type-usage problem. The previous track (metadata_promotion_20260624) became a tar pit because Tier 2 took the no-op shortcut.
|
||||
|
||||
HARD RULES (NON-NEGOTIABLE):
|
||||
1. NO "no-op" classifications. Each phase has a planned N sites. After the phase, exactly N sites must be migrated. If not, MODIFY the code (add more migrations) until the count matches.
|
||||
2. NO parallel dataclass definitions. Reuse existing dataclasses (src/type_aliases.py for type-system aggregates; src/models.py for FileItem, Ticket; src/openai_schemas.py for ToolCall, ChatMessage, UsageStats).
|
||||
3. NO metric rationalization. If compute_effective_codepaths doesn't drop after the track, MODIFY the migration. Don't blow it away.
|
||||
4. NO inference decisions. If variable type is unclear, STOP and ask.
|
||||
5. NO shortcuts. `if key in dict else default` is NOT a migration. `var = Aggregate.from_dict(var)` IS the migration.
|
||||
6. NO blowing away work. NEVER use `git restore`, `git checkout --`, `git reset`, or `git revert`. When something goes wrong, fix it. Add more sites. Reclassify. Amend the commit. Do not throw the work away.
|
||||
|
||||
PER-PHASE HARD GUARD:
|
||||
Each phase commit message MUST include:
|
||||
Phase N: <aggregate name>
|
||||
Before: <N> .get() sites (in the relevant file(s))
|
||||
After: <M> .get() sites
|
||||
Delta: <N-M> (expected: -<planned>)
|
||||
|
||||
If delta != -planned, FIX the migration. Add more sites. Reclassify. Recommit.
|
||||
|
||||
START:
|
||||
git log --oneline -10
|
||||
# Confirm you're on tier2/type_alias_unfuck_20260626
|
||||
|
||||
# Read the spec
|
||||
cat conductor/tracks/type_alias_unfuck_20260626/spec.md
|
||||
|
||||
# Run pre-flight
|
||||
git grep -nE "\.get\('[a-z_]+'," -- 'src/*.py' | wc -l
|
||||
# Expect: 67
|
||||
|
||||
# Execute Phase 0 pre-flight (baseline capture)
|
||||
# Then Phase 2 (FileItem)
|
||||
# Then Phase 3 (CommsLogEntry)
|
||||
# ... etc.
|
||||
|
||||
STOP AND ASK if any site's variable type is unclear.
|
||||
FIX (don't blow away) if any phase's count doesn't match the plan.
|
||||
DO NOT classify anything as no-op.
|
||||
```
|
||||
|
||||
## See also
|
||||
|
||||
- `conductor/tracks/metadata_promotion_20260624/spec.md` — the previous track that this one supersedes
|
||||
- `conductor/tracks/metadata_promotion_20260624/state.toml` — the (now honest) state of the previous track
|
||||
- `docs/reports/TIER1_REVIEW_metadata_promotion_20260624_20260625.md` — the Tier 1 review (planned)
|
||||
- `conductor/code_styleguides/type_aliases.md` §2.5 — the per-aggregate dataclass rule
|
||||
- `conductor/code_styleguides/data_oriented_design.md` — canonical DOD reference
|
||||
- `src/type_aliases.py` — the existing per-aggregate dataclasses (REUSE, do not modify)
|
||||
- `src/openai_schemas.py` — canonical ToolCall, ChatMessage, UsageStats
|
||||
- `src/models.py:533` — canonical FileItem
|
||||
- `src/models.py:302` — canonical Ticket
|
||||
- `conductor/AGENTS.md` — hard bans on `git restore`, `git checkout --`, `git reset`, `git revert` (NEVER use these)
|
||||
Reference in New Issue
Block a user