ed/manual_slop
Private
Public Access
0
0
Fork 0
Code Activity

docs(reports): planning correction for metadata_promotion_20260624

Browse Source
This commit is contained in:
Edward R. Gonzalez
2026-06-25 14:33:21 -04:00
parent c6748634a8
commit 51833f9d4d
1 changed files with 328 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/reports/PLANNING_CORRECTION_metadata_promotion_20260625.md
+328
View File
@@ -0,0 +1,328 @@
# Planning Correction: metadata_promotion_20260624
**Date:** 2026-06-25
**Author:** Tier 1 (post-audit correction)
**Status:** SPEC + PLAN + METADATA.JSON corrected; styleguide clarified; awaiting commit
**Scope:** Removes the bad inference from the `metadata_promotion_20260624` track (the proposal to share one mega-dataclass across all 5 sub-aggregates) and replaces it with the per-aggregate dataclass design that the 2026-06-06 `data_structure_strengthening` spec originally anticipated.
## TL;DR
The original `metadata_promotion_20260624` track (committed `e50bebdd` on 2026-06-25) proposed:
```python
@dataclass(frozen=True, slots=True)
class Metadata:
role: str = ""
content: Any = None
tool_calls: Any = None
tool_call_id: str = ""
name: str = ""
args: Any = None
source_tier: str = "main"
model: str = "unknown"
id: str = ""
ts: str = ""
role_: str = "" # For dicts that used 'role' as a key
description: str = ""
depends_on: tuple[str, ...] = ()
status: str = ""
manual_block: bool = False
completed_tickets: int = 0
auto_start: bool = False
command: str = ""
script: str = ""
output: Any = None
error: str = ""
tier: str = ""
path: str = ""
full_path: str = ""
filename: str = ""
mtime: float = 0.0
size: int = 0
# ... ~200 fields total, all Optional or with sensible defaults ...
CommsLogEntry: TypeAlias = Metadata # BAD
CommsLog: TypeAlias = list[CommsLogEntry]
HistoryMessage: TypeAlias = Metadata # BAD
History: TypeAlias = list[HistoryMessage]
FileItem: TypeAlias = Metadata # BAD
FileItems: TypeAlias = list[FileItem]
ToolDefinition: TypeAlias = Metadata # BAD
ToolCall: TypeAlias = Metadata # BAD
```
This is **wrong**. The 5 sub-aggregates (`CommsLogEntry`, `HistoryMessage`, `FileItem`, `ToolDefinition`, `ToolCall`) are distinct concepts with distinct field sets. Lifting them into one mega-dataclass:
1. **Hides the type information that direct field access is supposed to reveal.** A consumer that has a `Ticket` can read `.source_tier` (a `CommsLogEntry` field) and silently get the empty default.
2. **Is "less defined" than the current `dict[str, Any]` state.** Today, reading `.source_tier` on a `Ticket` raises `AttributeError` immediately. After the mega-dataclass, it silently returns `""`.
3. **Reverses the original 2026-06-06 design intent.** The `data_structure_strengthening_20260606` spec §3.3 explicitly anticipated per-concept promotion: *"Phase 2 can convert `Metadata` to a `TypedDict` (or split into per-concept `TypedDict`s) and the aliases continue to work without breaking changes. The aliases are STABLE NAMES; the underlying type can evolve."*
The corrected design promotes each known sub-aggregate to its OWN dataclass with its OWN fields. `Metadata: TypeAlias = dict[str, Any]` is preserved as the catch-all for **truly collapsed codepaths** (TOML project config, generic JSON parsing, polymorphic log dumping) only.
## What was bad about the original inference
### 1. The original spec proposed a single mega-dataclass with ~200 fields
The original `metadata_promotion_20260624/spec.md` §FR1 defined:
```python
@dataclass(frozen=True, slots=True)
class Metadata:
role: str = ""
content: Any = None
tool_calls: Any = None
tool_call_id: str = ""
name: str = ""
args: Any = None
source_tier: str = "main"
model: str = "unknown"
id: str = ""
ts: str = ""
role_: str = "" # For dicts that used 'role' as a key
description: str = ""
depends_on: tuple[str, ...] = ()
status: str = ""
manual_block: bool = False
completed_tickets: int = 0
auto_start: bool = False
command: str = ""
script: str = ""
output: Any = None
error: str = ""
tier: str = ""
path: str = ""
full_path: str = ""
filename: str = ""
mtime: float = 0.0
size: int = 0
# ... ~200 fields total, all Optional or with sensible defaults ...
CommsLogEntry: TypeAlias = Metadata
CommsLog: TypeAlias = list[CommsLogEntry]
HistoryMessage: TypeAlias = Metadata
History: TypeAlias = list[HistoryMessage]
FileItem: TypeAlias = Metadata
FileItems: TypeAlias = list[FileItem]
ToolDefinition: TypeAlias = Metadata
ToolCall: TypeAlias = Metadata
```
This is the bad inference. The user complaint:
> "If we have known sub-types they should be their own data class if they're not already, this doesn't make sense to lift them into a less defined moshpit, even with the data-oriented setup."
The 200-field mega-dataclass IS the "less defined moshpit." It mashes 12+ distinct aggregates into one polymorphic type.
### 2. The original spec's G3 explicitly mandated the bad pattern
The original `metadata_promotion_20260624/spec.md` Goal G3:
> "**G3**: All 5 sub-aggregates share the same dataclass (per type_aliases.py chain)."
And the Out of Scope:
> "The 5 sub-aggregates (CommsLogEntry, HistoryMessage, FileItem, ToolDefinition, ToolCall) becoming separate dataclasses each (overkill; they share the same Metadata base)"
The user complaint:
> "All 5 sub-aggregates share the same dataclass (per type_aliases.py chain) Is not a good thing todo."
The original spec's G3 + Out of Scope are direct contradictions of the user's intent. Both are rewritten in the corrected spec.
### 3. The original spec's 213 access sites actually span 12+ distinct aggregates
A sampling of the actual access patterns in `src/` (from `git grep -E "\.get\('[a-z_]+',"`):
| Access pattern | Aggregate it actually represents |
|---|---|
| `item.get('custom_slices', [])`, `item.get('content', '')` | **FileItem** |
| `fi.get('path', 'attachment')` | **FileItem** |
| `chunk.get('document', '')` | **RAGChunk** |
| `entry.get('source_tier', 'main')`, `entry.get('model', 'unknown')` | **CommsLogEntry** |
| `u.get('input_tokens', 0)`, `u.get('output_tokens', 0)` | **UsageStats** |
| `t.get('id', '')`, `t.get('depends_on', [])`, `t.get('manual_block', False)`, `t.get('status')` | **Ticket** |
| `stats.get('model', 'unknown')`, `stats.get('input', 0)`, `stats.get('output', 0)` | **MMAUsageStats** |
| `insights.get('total_tokens', 0)`, `insights.get('call_count', 0)`, `insights.get('burn_rate', 0)`, `insights.get('session_cost', 0)`, `insights.get('completed_tickets', 0)`, `insights.get('efficiency', 0)` | **SessionInsights** |
| `entry.get('temperature', 0.7)`, `entry.get('top_p', 1.0)`, `entry.get('max_output_tokens', 0)` | **DiscussionSettings** |
| `slc.get('tag', '')`, `slc.get('comment', '')` | **CustomSlice** |
| `preset.get('files', [])`, `preset.get('screenshots', [])` | **ContextPreset** |
| `payload.get('script')`, `payload.get('args', {})`, `payload.get('output', '')`, `payload.get('content', '')` | **ProviderPayload** |
| `self.project.get('paths', {})`, `self.project.get('conductor', {})`, `self.project.get('context_presets', {})` | **ProjectConfig** (TRULY collapsed codepath) |
| `gui_cfg.get('separate_message_panel', False)`, `gui_cfg.get('separate_response_panel', False)`, `gui_cfg.get('separate_tool_calls_panel', False)` | **UIPanelConfig** |
| `self.project.get('discussion', {}).get('discussions', {})` | **DiscussionStore** |
| `path_info['logs_dir']['path']` | **PathInfo** (nested) |
There is no single "Metadata" shape. The 107 `.get()` sites access ~12 distinct aggregates. The original spec's mega-dataclass tried to force them all into one type — that IS the "less defined moshpit."
### 4. The corrected design follows the canonical pattern already in production
`src/openai_schemas.py` defines **5 separate frozen dataclasses**:
- `ToolCallFunction` (2 fields: `name, arguments`)
- `ToolCall` (3 fields: `id, function, type`)
- `ChatMessage` (5 fields: `role, content, tool_calls, tool_call_id, name`)
- `UsageStats` (4 fields: `input_tokens, output_tokens, cache_read_tokens, cache_creation_tokens`)
- `NormalizedResponse` (4 fields: `text, tool_calls, usage, raw_response`)
`src/models.py` defines **4 more separate frozen dataclasses**:
- `Ticket` (15 fields: `id, description, target_symbols, context_requirements, depends_on, status, assigned_to, priority, target_file, blocked_reason, step_mode, retry_count, manual_block, model_override, persona_id`)
- `FileItem` (10 fields: `path, auto_aggregate, force_full, view_mode, selected, ast_signatures, ast_definitions, ast_mask, custom_slices, injected_at`) with paired `to_dict()` / `from_dict()`
- `Track` (3 fields: `id, description, tickets`)
- `TrackState` (3 fields: `metadata, discussion, tasks`)
These are the **canonical reference pattern**. They are not shared mega-dataclasses; they are per-aggregate frozen dataclasses with their own fields. The corrected `metadata_promotion_20260624` spec continues in this direction.
## What the corrected design is
### Per-aggregate dataclasses (each its own type with its own fields)
| Class | Module | Fields | Reused vs NEW |
|---|---|---:|---|
| `Ticket` | `src/models.py:302` | 15 | REUSED |
| `FileItem` | `src/models.py:533` | 10 | REUSED |
| `ContextPreset` | `src/models.py:932` (extended) | 3+ | REUSED + EXTENDED |
| `ToolCall` | `src/openai_schemas.py:32` | 3 | REUSED |
| `ToolCallFunction` | `src/openai_schemas.py:26` | 2 | REUSED |
| `ChatMessage` | `src/openai_schemas.py:48` | 5 | REUSED |
| `UsageStats` | `src/openai_schemas.py:68` | 4 | REUSED |
| `NormalizedResponse` | `src/openai_schemas.py:78` | 4 | REUSED |
| `CommsLogEntry` | `src/type_aliases.py` (NEW) | 8 | NEW |
| `HistoryMessage` | `src/type_aliases.py` (NEW) | 6 | NEW |
| `ToolDefinition` | `src/type_aliases.py` (NEW) | 4 | NEW |
| `SessionInsights` | `src/type_aliases.py` (NEW) | 6 | NEW |
| `DiscussionSettings` | `src/type_aliases.py` (NEW) | 3 | NEW |
| `CustomSlice` | `src/type_aliases.py` (NEW) | 4 | NEW |
| `MMAUsageStats` | `src/type_aliases.py` (NEW) | 3 | NEW |
| `ProviderPayload` | `src/type_aliases.py` (NEW) | 4 | NEW |
| `UIPanelConfig` | `src/type_aliases.py` (NEW) | 3 | NEW |
| `PathInfo` | `src/type_aliases.py` (NEW) | 3 | NEW |
| `RAGChunk` | `src/rag_engine.py` (NEW) | 4 | NEW |
Each new dataclass has a paired `to_dict()` / `from_dict()` round-trip (the canonical pattern from `src/openai_schemas.py` and `src/models.py:533`).
### `Metadata: TypeAlias = dict[str, Any]` — preserved as the catch-all
`Metadata` is **unchanged**. It is the catch-all for the truly collapsed codepaths:
- `manual_slop.toml` project config loading (`self.project.get('paths', {})`, `self.project.get('conductor', {})`, `self.project.get('context_presets', {})`, `self.project.get('discussion', {})`)
- Generic JSON parsing at the wire boundary (REST API payloads, WebSocket messages)
- Polymorphic log dumping (a function that serializes a list of mixed-aggregate entries to JSON without caring about their individual types)
These sites keep `Metadata` and `.get('key', default)` because there is no per-aggregate type to promote to. The classification (per-site: "promoted" or "collapsed-codepath with justification") is auditable in the Phase 11 commit message.
### 13 phases (1 per aggregate + audit + verification)
The corrected plan has 13 phases:
- Phase 0: Design the new dataclasses + add regression-guard tests (5 tasks)
- Phase 1: Migrate `Ticket` consumers (3 tasks; remove legacy `get()` method)
- Phase 2: Migrate `FileItem` consumers (2 tasks)
- Phase 3: Migrate `CommsLogEntry` consumers (4 tasks; new dataclass)
- Phase 4: Migrate `HistoryMessage` consumers (2 tasks; new dataclass)
- Phase 5: Wire `ChatMessage` into per-vendor send paths (4 tasks)
- Phase 6: Wire `UsageStats` into per-call usage aggregation (1 task)
- Phase 7: Wire `ToolCall` into tool loop section (2 tasks)
- Phase 8: Migrate `ToolDefinition` consumers (2 tasks; new dataclass)
- Phase 9: Migrate `RAGChunk` consumers (1 task; new dataclass)
- Phase 10: Migrate small-batch aggregates (2 tasks; 8 small aggregates)
- Phase 11: `Metadata` collapsed-codepath audit (1 task; classification per FR6)
- Phase 12: Verification + end-of-track (1 task; 3 commits)
Estimated 29+ atomic commits.
## What was changed in the corrected artifacts
### `conductor/tracks/metadata_promotion_20260624/spec.md`
Rewrote:
- **Overview**: rewrote to emphasize per-aggregate dataclasses (not a shared mega-dataclass) and added the "CORRECTED 2026-06-25" status banner
- **Current State Audit**: added a 16-row table mapping each access pattern to its actual aggregate (the evidence that 12+ aggregates exist)
- **Goals**: rewrote G3 from "All 5 sub-aggregates share the same dataclass" to "Each known sub-aggregate is its OWN `@dataclass(frozen=True, slots=True)`"
- **Goals**: added G2 explicitly: "`Metadata: TypeAlias = dict[str, Any]` is preserved as the catch-all; NOT promoted to a shared mega-dataclass"
- **Goals**: added G8: classification rule for the remaining `.get()` sites
- **Functional Requirements**: rewrote FR1 with per-aggregate dataclass tables (existing reused + NEW dataclasses) and a "Why per-aggregate, not mega-dataclass" section
- **Out of Scope**: removed the "5 sub-aggregates becoming separate dataclasses each is overkill" line; added an explicit "Promoting `Metadata` to a shared mega-dataclass is the original spec's bad inference; rejected 2026-06-25" line
- **Non-Goals**: rewrote to reference the per-aggregate design
- **Risks**: rewrote R1 to reference the canonical pattern from `src/openai_schemas.py` / `src/models.py:533`; added R7 for name collisions
### `conductor/tracks/metadata_promotion_20260624/plan.md`
Rewrote:
- **Header**: added "CORRECTED 2026-06-25" status banner
- **Phase 0**: expanded to 5 tasks (was 2); now includes RAGChunk (in `src/rag_engine.py`), ContextPreset schema completion (in `src/models.py`), per-aggregate test files (split into 12 files, not 1), and the styleguide clarification
- **Phases 1-10**: renamed to per-aggregate phases (Ticket, FileItem, CommsLogEntry, HistoryMessage, ChatMessage, UsageStats, ToolCall, ToolDefinition, RAGChunk, small-batch aggregates)
- **Phase 11**: NEW — the `Metadata` collapsed-codepath classification audit
- **Phase 12**: renamed from "Phase 6" — verification + end-of-track
- **Commit log**: expanded from 19-21 commits to 29+ commits
- **Verification commands**: updated to reflect the per-aggregate design (VC1: Metadata unchanged; VC2: each new dataclass exists; VC6: 60+ tests across 12 test files)
### `conductor/tracks/metadata_promotion_20260624/metadata.json`
Rewrote:
- **`name`**: changed from "Metadata Promotion: dict[str, Any] -> @dataclass(frozen=True, slots=True)" to "Metadata Promotion: per-aggregate dataclasses + direct field access (NOT a shared mega-dataclass)"
- **`corrected`**: added field with date and correction note
- **`blocked_by`**: updated to reflect `code_path_audit_phase_3_provider_state_20260624` SHIPPED status
- **`scope.new_files`**: replaced single `tests/test_metadata_dataclass.py` with 12 per-aggregate test files
- **`scope.modified_files`**: replaced `src/type_aliases.py` alone with the 12 modified files (the type_aliases.py + the 9 consumer files + the styleguide + ContextPreset in models.py + RAGChunk in rag_engine.py)
- **`scope.new_dataclasses`**: NEW field — the 11 new dataclasses to add
- **`scope.reused_existing_dataclasses`**: NEW field — the 8 existing dataclasses to reuse unchanged
- **`scope.deprecated`**: NEW field — the 4 things this track removes (the alias chain, the legacy `Ticket.get()` method)
- **`verification_criteria`**: replaced "All 5 sub-aggregate TypeAliases (CommsLogEntry, HistoryMessage, FileItem, ToolDefinition, ToolCall) point to the new Metadata" with the per-aggregate criteria; added "Planning correction report exists"
- **`estimated_effort.scope`**: updated to reflect 29+ commits across 13 phases
- **`risk_register`**: rewrote R1-R7 to reference the per-aggregate design; added R7 (name collisions) and R8 (legacy `Ticket.get()` removal)
- **`out_of_scope`**: added "Promoting Metadata: TypeAlias = dict[str, Any] itself to a shared mega-dataclass (the original spec's bad inference; rejected 2026-06-25)"
### `conductor/code_styleguides/type_aliases.md`
Added §2.5 (after §2) — "When the role has stable distinct fields, promote it to its OWN dataclass":
- The rule (per-aggregate dataclasses, not mega-dataclass)
- The when-NOT-to-promote rule (collapsed codepaths keep `Metadata`)
- A worked example from `src/openai_schemas.py` and `src/models.py:533`
- A reference back to the 2026-06-06 `data_structure_strengthening_20260606` spec §3.3 design intent
- A note that the `metadata_promotion_20260624` track was corrected on 2026-06-25 to continue in the per-concept promotion direction
## Why this happened (the Tier 1 failure pattern)
The original `metadata_promotion_20260624` author (me, on 2026-06-25) cited the `data_structure_strengthening_20260606` spec §3.3 design intent as evidence that the aliases could be promoted:
> "Phase 2 can convert `Metadata` to a `TypedDict` (or split into per-concept `TypedDict`s) and the aliases continue to work without breaking changes. The aliases are STABLE NAMES; the underlying type can evolve."
But then the author chose the wrong direction: instead of splitting into per-concept TypedDicts/dataclasses (the "(or split into per-concept `TypedDict`s)" option), the author consolidated all 5 sub-aggregates into one mega-dataclass. The author treated the 5 sub-aggregates as "all the same thing, just labeled differently" — the exact opposite of what the 2026-06-06 spec anticipated.
The user feedback (2026-06-25):
> "I don't know where the previous tier 1 got the idea that this would be ok. It just makes a mess for no reason. Downstream codepaths that are going to utilize a specific data class should just... fucking use them."
The Tier 1 failure pattern:
1. **Cited the spec without reading the actual code.** The author should have run `git grep -E "\.get\('[a-z_]+',"` to see the actual access patterns. The 12+ distinct aggregates are evident from the access patterns.
2. **Did not check the existing per-aggregate dataclasses.** `src/openai_schemas.py` and `src/models.py` already define 9 separate frozen dataclasses — each with its own fields. The pattern was already in production; the author should have followed it.
3. **Conflated "names for shapes" with "same shape."** The `data_structure_strengthening_20260606` convention is "names for shapes" (the aliases document semantic role), but the underlying types were all `dict[str, Any]` because the codebase didn't have per-aggregate dataclasses yet. The promotion step is to GIVE each aggregate its OWN dataclass, not to MERGE them into one mega-dataclass.
## Lessons learned (for future Tier 1s)
1. **Read the actual code before designing.** The 12+ aggregates are evident from a `git grep` of the access patterns. Don't infer from type aliases alone.
2. **Check for existing per-aggregate dataclasses.** `src/openai_schemas.py` and `src/models.py` already define 9 separate frozen dataclasses. The pattern is canonical; follow it.
3. **Read the original spec's design intent.** `data_structure_strengthening_20260606` §3.3 anticipated per-concept promotion. The corrected design continues in that direction.
4. **"Names for shapes" ≠ "same shape."** Aliases document semantic role, but the underlying types can (and should) diverge into per-aggregate dataclasses as the codebase matures.
5. **The user said: "If we have known sub-types they should be their own data class if they're not already."** This is the rule. The original spec violated it; the corrected spec follows it.
## See also
- `conductor/tracks/metadata_promotion_20260624/spec.md` (corrected 2026-06-25)
- `conductor/tracks/metadata_promotion_20260624/plan.md` (corrected 2026-06-25)
- `conductor/tracks/metadata_promotion_20260624/metadata.json` (corrected 2026-06-25)
- `conductor/code_styleguides/type_aliases.md` §2.5 (added 2026-06-25)
- `conductor/code_styleguides/data_oriented_design.md` — canonical DOD reference
- `conductor/code_styleguides/error_handling.md` — `Result[T]` convention
- `conductor/tracks/data_structure_strengthening_20260606/spec.md` §3.3 — original 2026-06-06 design intent
- `conductor/tracks/any_type_componentization_20260621/spec.md` — grandparent track (89 sites promoted to dataclasses)
- `src/openai_schemas.py` — canonical per-aggregate dataclass pattern
- `src/models.py:533` — `FileItem` with `to_dict()` / `from_dict()` round-trip
- `src/models.py:302` — `Ticket` with 15 typed fields
- `docs/reports/SSDL_CAMPAIGN_ABORTED_20260624.md` — the post-mortem that established the type-dispatch-as-bug thesis