Private
Public Access
conductor(archive): move legacy superpowers specs/plans from docs/superpowers/ to conductor/archive/superpowers/
Resolves HIGH-priority recommendation #3 from superpowers_review_20260619 §16.1 (dual-convention). 41 files moved (21 specs + 20 plans + 2 subdirs + root): - docs/superpowers/specs/*.md -> conductor/archive/superpowers/specs/*.md (21 files) - docs/superpowers/plans/*.md -> conductor/archive/superpowers/plans/*.md (20 files) Original taxonomy preserved (specs/ and plans/ subdirectories intact). The superpowers-plugin source itself remains at C:/Users/Ed/.cache/opencode/.../superpowers/skills/. Future spec/plan artifacts must use the conductor convention: conductor/tracks/<id>/spec.md + plan.md.
This commit is contained in:
+253
@@ -0,0 +1,253 @@
|
||||
# Context Composition Redesign Specification
|
||||
|
||||
## Overview
|
||||
|
||||
Redesign Context Composition in Manual Slop to support sophisticated, layered context management for MMA epics and 1:1 agent discussions. The goal is explicit user control over what context an agent receives at varying granularities.
|
||||
|
||||
## Current State
|
||||
|
||||
- Files & Media and Context Composition are coupled (flags sync visually)
|
||||
- Context Composition inherits from Files & Media automatically
|
||||
- No view presets, limited slice visualization
|
||||
- No context preview before sending
|
||||
|
||||
## Design Goals
|
||||
|
||||
1. **Decouple** Files & Media from Context Composition
|
||||
2. **Context Composition** = curated selection from project whitelist with view modes
|
||||
3. **File View Presets** = named combinations of sig/def/full/custom for per-file selection
|
||||
4. **Directory grouping** for compact, scrollable file lists
|
||||
5. **Slice visualization** with annotation support
|
||||
6. **Context Presets** = save/load file+view+slices compositions
|
||||
7. **Context Preview** = show exactly what will be sent to agent
|
||||
|
||||
---
|
||||
|
||||
## Phase 1: Decoupling + File Stats + View Selection
|
||||
|
||||
### 1.1 Files & Media = Project Whitelist
|
||||
|
||||
**Purpose:** Define what files exist and belong to the project codebase.
|
||||
|
||||
**Behavior:**
|
||||
- Raw file list with wildcards (existing)
|
||||
- Does NOT affect Discussion Context directly
|
||||
- RAG indexing pulls from this list
|
||||
- Tool access pulls from this list (via tooling permissions)
|
||||
|
||||
**UI:**
|
||||
- File listing with path display
|
||||
- Add/remove files
|
||||
- Wildcard patterns for bulk add
|
||||
- Rescan project for new files
|
||||
|
||||
### 1.2 Context Composition = Curated Selection
|
||||
|
||||
**Purpose:** Select which files to include in THIS discussion/epic and HOW.
|
||||
|
||||
**Behavior:**
|
||||
- Independent from Files & Media (decoupled)
|
||||
- User manually adds files FROM the project whitelist
|
||||
- Or user adds ALL from whitelist then removes unwanted
|
||||
- Each file has view mode and optional custom slices
|
||||
|
||||
**View Modes per file:**
|
||||
- `full` - raw text (no AST pruning)
|
||||
- `sig` - signature-only outline (basic)
|
||||
- `def` - definition outline (includes forwards, full AST)
|
||||
- `custom` - sig/def + user-defined slices with annotations
|
||||
|
||||
**UI Structure:**
|
||||
```
|
||||
[Preset Selector: ▼ Default ▼] [+ New] [💾 Save] [🗑 Delete]
|
||||
|
||||
[Search/Filter files...]
|
||||
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
|
||||
📁 base/auxiliary/
|
||||
☑ builder.cpp [View: def ▼] [Inspect] [Slices]
|
||||
☑ allocator.cpp [View: sig ▼] [Inspect] [Slices]
|
||||
📁 core/
|
||||
☑ engine.h [View: full ▼] [Inspect] [Slices]
|
||||
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
|
||||
|
||||
[Stats Panel]
|
||||
Files: 3 | Lines: 1,247 | AST Elements: 89
|
||||
```
|
||||
|
||||
### 1.3 Directory Grouping
|
||||
|
||||
**Compact display:**
|
||||
- Files grouped by common relative directory prefix
|
||||
- Directory header collapsible
|
||||
- Reduces scrolling, eliminates horizontal scroll
|
||||
- Format: `📁 relative/path/`
|
||||
|
||||
### 1.4 File Stats
|
||||
|
||||
**Per-file stats display:**
|
||||
- Line count
|
||||
- AST element count (functions, classes, structs, etc.)
|
||||
- View mode indicator
|
||||
|
||||
**Aggregate stats for selection:**
|
||||
- Total files, total lines, total AST elements
|
||||
- Helps user decide: "should I just inject full text or curate?"
|
||||
|
||||
---
|
||||
|
||||
## Phase 2: Slice Visualization + Annotations
|
||||
|
||||
### 2.1 Slice Inspector (replaces Inspect button)
|
||||
|
||||
**Purpose:** Visualize and edit slices for a file.
|
||||
|
||||
**Opens as popup/modal showing:**
|
||||
- Full file content with line numbers
|
||||
- AST-derived slices highlighted (sig elements, def elements)
|
||||
- User custom slices with annotations
|
||||
- Line range indicators for each slice
|
||||
|
||||
**Slice types:**
|
||||
- `sig` - AST signature elements
|
||||
- `def` - AST definition elements
|
||||
- `custom` - user-defined line ranges with optional tag/comment
|
||||
|
||||
**Annotation support:**
|
||||
- Each custom slice can have:
|
||||
- `tag` - category label (e.g., "performance", "bug", "api")
|
||||
- `comment` - free-text explanation for agent
|
||||
|
||||
### 2.2 View Presets
|
||||
|
||||
**Named combinations of view settings:**
|
||||
- Preset defines default view mode + default slices for a file type
|
||||
- Example: "Debug View" = full text + custom slice at error-prone lines
|
||||
- Example: "API Surface" = sig + custom slices for public API functions
|
||||
|
||||
**Behavior:**
|
||||
- Select preset for a file → auto-fills view mode + common slices
|
||||
- User can override after selecting preset
|
||||
- Presets are project-scoped (saved in project config)
|
||||
|
||||
---
|
||||
|
||||
## Phase 3: Context Presets
|
||||
|
||||
### 3.1 Save/Load Context Presets
|
||||
|
||||
**Context Preset contains:**
|
||||
- List of files with:
|
||||
- Relative path
|
||||
- View mode (full/sig/def/custom)
|
||||
- Custom slices with line ranges, tags, comments
|
||||
- Preset name
|
||||
- Preset description (optional)
|
||||
|
||||
**Save behavior:**
|
||||
- Validate all files exist in project
|
||||
- If file missing: warn user, offer to:
|
||||
- Save without missing files
|
||||
- Cancel and resolve manually
|
||||
- Save to project config (project_context_presets.toml)
|
||||
|
||||
**Load behavior:**
|
||||
- On preset select, Context Composition populates with saved state
|
||||
- If file from preset is missing: highlight in red, warn
|
||||
- User can remove missing or attempt to re-path
|
||||
|
||||
### 3.2 Context Preview
|
||||
|
||||
**Before sending to agent:**
|
||||
- Show exactly what will be composed
|
||||
- For each file, show:
|
||||
- Which view mode applied
|
||||
- Line ranges included
|
||||
- Tags/comments visible to agent
|
||||
- Total token estimate
|
||||
|
||||
**Preview modes:**
|
||||
- Collapsed (just file list + view modes)
|
||||
- Expanded (show actual text/slices that will be sent)
|
||||
|
||||
---
|
||||
|
||||
## Data Models
|
||||
|
||||
### FileViewPreset
|
||||
```python
|
||||
@dataclass
|
||||
class FileViewPreset:
|
||||
name: str
|
||||
description: str
|
||||
view_mode: str # "full" | "sig" | "def" | "custom"
|
||||
default_slices: list[dict] # [{start_line, end_line, tag, comment}]
|
||||
```
|
||||
|
||||
### ContextPreset
|
||||
```python
|
||||
@dataclass
|
||||
class ContextPreset:
|
||||
name: str
|
||||
description: str
|
||||
files: list[ContextFileEntry]
|
||||
|
||||
@dataclass
|
||||
class ContextFileEntry:
|
||||
relative_path: str # relative to project root
|
||||
view_mode: str
|
||||
custom_slices: list[dict] # [{start_line, end_line, tag, comment}]
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## UX Flows
|
||||
|
||||
### Adding Files to Context Composition
|
||||
|
||||
1. User clicks "Add Files" in Context Composition
|
||||
2. File picker shows project whitelist files, grouped by directory
|
||||
3. User selects files (multi-select supported)
|
||||
4. Files added with default view mode (configurable)
|
||||
|
||||
### Creating Custom Slice
|
||||
|
||||
1. User clicks [Slices] on a file
|
||||
2. Slice editor opens showing file content + AST slices
|
||||
3. User selects line range via click-drag or input
|
||||
4. User adds tag and/or comment
|
||||
5. Slice saved to file entry in Context Composition
|
||||
|
||||
### Saving Context Preset
|
||||
|
||||
1. User curates Context Composition
|
||||
2. Clicks [💾 Save]
|
||||
3. Dialog: enter preset name + optional description
|
||||
4. Validation runs - warns if files missing
|
||||
5. Preset saved to project
|
||||
|
||||
### Loading Context Preset
|
||||
|
||||
1. User selects preset from dropdown
|
||||
2. Context Composition cleared
|
||||
3. Files populated from preset
|
||||
4. Missing files highlighted with warning
|
||||
5. User resolves or proceeds
|
||||
|
||||
---
|
||||
|
||||
## Technical Notes
|
||||
|
||||
- All file paths in presets stored as relative to project root
|
||||
- FileItem model extended with view_mode, custom_slices
|
||||
- Context Composition panel completely rewired to be independent of Files & Media
|
||||
- Aggregate functions updated to respect view modes and slices
|
||||
- Tool access still uses Files & Media whitelist
|
||||
|
||||
---
|
||||
|
||||
## Out of Scope
|
||||
|
||||
- RAG configuration changes (handled separately)
|
||||
- Tool preset changes (already exists)
|
||||
- MMA track creation (handled by track system)
|
||||
- Agent behavior when given incomplete views (assumes tooling access)
|
||||
@@ -0,0 +1,96 @@
|
||||
# ImGui Context Manager Suite Design
|
||||
|
||||
## Status
|
||||
|
||||
Proposed — 2026-05-11
|
||||
|
||||
## Context
|
||||
|
||||
The `src/gui_2.py` file contains ~50 `imgui.begin()` / `imgui.end()` pairs scattered across 5900+ lines. Dear PyGui (and ImGui) uses a procedural API where every `begin_X()` requires a matching `end_X()`. This creates two problems:
|
||||
|
||||
1. **Error-prone for AI agents** — forgetting an `end()` call causes UI nesting bugs that are hard to trace
|
||||
2. **Visual sprawl** — the begin/end pairing is separated by potentially hundreds of lines, making scope boundaries unclear
|
||||
|
||||
## Decision
|
||||
|
||||
Implement a context manager suite that pairs `begin` and `end` calls on the same line using Python's `with` statement.
|
||||
|
||||
## Implementation
|
||||
|
||||
### Core Base Class
|
||||
|
||||
```python
|
||||
class ImGuiScope:
|
||||
def __init__(self, begin_fn, end_fn, *args, **kwargs):
|
||||
self._begin_fn = begin_fn
|
||||
self._end_fn = end_fn
|
||||
self._args = args
|
||||
self._kwargs = kwargs
|
||||
self._opened = None
|
||||
self._entered = False
|
||||
|
||||
def __enter__(self):
|
||||
result = self._begin_fn(*self._args, **self._kwargs)
|
||||
if isinstance(result, tuple):
|
||||
self._opened = result[0]
|
||||
else:
|
||||
self._opened = result
|
||||
self._entered = bool(self._opened)
|
||||
return self._opened
|
||||
|
||||
def __exit__(self, *args):
|
||||
if self._entered:
|
||||
self._end_fn()
|
||||
return False
|
||||
```
|
||||
|
||||
### Scope Helpers
|
||||
|
||||
| Function | Wraps | Signature |
|
||||
|----------|-------|-----------|
|
||||
| `imgui_window(name, visible=True, flags=0)` | `imgui.begin` | `__enter__` returns `opened` bool |
|
||||
| `imgui_table(name, columns, flags=0)` | `imgui.begin_table` | `__enter__` returns `opened` bool |
|
||||
| `imgui_menu_bar()` | `imgui.begin_menu_bar` | No args |
|
||||
| `imgui_menu(label)` | `imgui.begin_menu` | `__enter__` returns `opened` bool |
|
||||
| `imgui_child(id, width=0, height=0, flags=0)` | `imgui.begin_child` | `__enter__` returns `opened` bool |
|
||||
| `imgui_group()` | `imgui.begin_group` | No args |
|
||||
| `imgui_popup(id)` | `imgui.begin_popup` | `__enter__` returns `opened` bool |
|
||||
| `imgui_tooltip()` | `imgui.begin_tooltip` | No args |
|
||||
| `imgui_clipper(count)` | `clipper.begin` | `__enter__` returns `opened` count |
|
||||
| `node_editor_scope(name)` | `ed.begin` | `__enter__` returns `opened` bool |
|
||||
|
||||
### Usage Pattern
|
||||
|
||||
```python
|
||||
# Before
|
||||
exp, opened = imgui.begin("AI Settings", self.show_windows["AI Settings"])
|
||||
if exp:
|
||||
imgui.text("Settings")
|
||||
imgui.separator()
|
||||
# ... 50 more lines ...
|
||||
imgui.end()
|
||||
|
||||
# After
|
||||
if imgui_window("AI Settings", self.show_windows["AI Settings"], flags):
|
||||
imgui.text("Settings")
|
||||
imgui.separator()
|
||||
# ... 50 more lines ...
|
||||
# imgui.end() called automatically
|
||||
```
|
||||
|
||||
## File Location
|
||||
|
||||
`src/imgui_scopes.py`
|
||||
|
||||
## Migration Strategy
|
||||
|
||||
1. New code MUST use context managers
|
||||
2. Existing code migrates incrementally during bug fixes and feature work
|
||||
3. No强制性 — old `begin()`/`end()` calls remain valid
|
||||
|
||||
## Consequences
|
||||
|
||||
- **Reduced** scope pairing errors
|
||||
- **Improved** code legibility for AI agents
|
||||
- **Slight** line count reduction (one `imgui.end()` line saved per scope)
|
||||
- **No** runtime performance impact (imperceptible nanoseconds vs C++ binding calls)
|
||||
@@ -0,0 +1,101 @@
|
||||
# ai_client.py Style Convention Curation
|
||||
|
||||
## Overview
|
||||
|
||||
Refactor `src/ai_client.py` (2522 lines) to align with `conductor/code_styleguides/python.md` conventions, following the pattern established in the recent `gui_2.py` refactor.
|
||||
|
||||
## Current State Audit (as of UNCOMMITTED)
|
||||
|
||||
### Already Implemented
|
||||
- 1-space indentation throughout
|
||||
- Type annotations on all functions
|
||||
- Thread-local helpers (`get_current_tier`, `set_current_tier`) as module-level functions
|
||||
- SDM tags on docstrings
|
||||
|
||||
### Gaps to Fill
|
||||
- `ProviderError` defined inside module (should be at module level per convention)
|
||||
- No `#region` blocks despite 2522-line size (violates python.md Section 14)
|
||||
- No logical section organization for 80+ functions
|
||||
- Error classifier functions (`_classify_*_error`) could use vertical alignment
|
||||
|
||||
## Goals
|
||||
|
||||
1. Move `ProviderError` to module level (proper exception class, justified)
|
||||
2. Add `#region` blocks to organize functions into logical sections
|
||||
3. Apply vertical compaction alignment to dense conditionals
|
||||
4. Preserve all call sites and SDM tags
|
||||
|
||||
## Changes
|
||||
|
||||
### 1. Module-Level `ProviderError`
|
||||
|
||||
Move from inline class definition to module level, before top-level functions:
|
||||
|
||||
```python
|
||||
class ProviderError(Exception):
|
||||
def __init__(self, provider: str, message: str, code: str | None = None):
|
||||
self.provider = provider
|
||||
self.message = message
|
||||
self.code = code
|
||||
|
||||
def ui_message(self) -> str:
|
||||
return f"[{self.provider.upper()}] {self.message}"
|
||||
```
|
||||
|
||||
Location: After imports, before first function (~line 300).
|
||||
|
||||
### 2. Region Block Organization
|
||||
|
||||
Add `#region: Section Name` / `#endregion: Section Name` blocks:
|
||||
|
||||
| Region | Functions |
|
||||
|--------|-----------|
|
||||
| `#region: Provider Configuration` | `set_provider`, `get_provider`, `set_model_params`, `list_models`, `_list_gemini_cli_models`, `_list_gemini_models`, `_list_anthropic_models`, `_list_deepseek_models`, `_list_minimax_models` |
|
||||
| `#region: Credentials & Setup` | `_get_proxy`, `get_credentials_path`, `_load_credentials`, `cleanup` |
|
||||
| `#region: System Prompt Management` | `set_custom_system_prompt`, `set_base_system_prompt`, `set_use_default_base_prompt`, `set_project_context_marker`, `_get_context_marker`, `_get_combined_system_prompt`, `get_combined_system_prompt` |
|
||||
| `#region: Comms Log` | `get_comms_log_callback`, `set_comms_log_callback`, `_append_comms`, `get_comms_log`, `clear_comms_log` |
|
||||
| `#region: Error Classification` | `_classify_anthropic_error`, `_classify_gemini_error`, `_classify_deepseek_error`, `_classify_minimax_error` |
|
||||
| `#region: Tool Configuration` | `set_agent_tools`, `set_tool_preset`, `set_bias_profile`, `get_bias_profile`, `_build_anthropic_tools`, `_get_anthropic_tools`, `_gemini_tool_declaration`, `_build_deepseek_tools`, `_get_deepseek_tools`, `_content_block_to_dict` |
|
||||
| `#region: Tool Execution` | `_execute_tool_calls_concurrently`, `_execute_single_tool_call_async`, `_run_script`, `_truncate_tool_output` |
|
||||
| `#region: File Context Building` | `_reread_file_items`, `_build_file_context_text`, `_build_file_diff_text` |
|
||||
| `#region: Token Estimation` | `_estimate_message_tokens`, `_invalidate_token_estimate`, `_estimate_prompt_tokens`, `_strip_stale_file_refreshes` |
|
||||
| `#region: History Management` | `_trim_anthropic_history`, `_repair_anthropic_history`, `_repair_deepseek_history`, `_add_history_cache_breakpoint`, `_strip_cache_controls` |
|
||||
| `#region: Gemini Provider` | `_ensure_gemini_client`, `_get_gemini_history_list`, `_send_gemini`, `_send_gemini_cli`, `get_gemini_cache_stats` |
|
||||
| `#region: Anthropic Provider` | `_ensure_anthropic_client`, `_chunk_text`, `_build_chunked_context_blocks`, `_send_anthropic` |
|
||||
| `#region: DeepSeek Provider` | `_ensure_deepseek_client`, `_send_deepseek` |
|
||||
| `#region: MiniMax Provider` | `_ensure_minimax_client`, `_send_minimax` |
|
||||
| `#region: Tier 4 Analysis` | `run_tier4_analysis`, `run_tier4_patch_callback`, `run_tier4_patch_generation` |
|
||||
| `#region: Session & Public API` | `reset_session`, `get_token_stats`, `send`, `_add_bleed_derived`, `run_subagent_summarization` |
|
||||
|
||||
### 3. Vertical Compaction
|
||||
|
||||
Apply alignment to error classifier functions:
|
||||
|
||||
```python
|
||||
# Before
|
||||
if status == 'running': col = (0.0, 1.0, 0.0, 1.0)
|
||||
elif status == 'starting': col = (1.0, 1.0, 0.0, 1.0)
|
||||
elif status == 'error': col = (1.0, 0.0, 0.0, 1.0)
|
||||
|
||||
# Apply to _classify_*_error functions with similar patterns
|
||||
```
|
||||
|
||||
## Non-Functional Requirements
|
||||
|
||||
- No logic changes — purely organizational
|
||||
- All existing `[C: ...]` SDM tags preserved
|
||||
- All call sites continue to work (50+ references)
|
||||
- 1-space indentation maintained
|
||||
|
||||
## Out of Scope
|
||||
|
||||
- Extraction of class methods to module-level functions
|
||||
- Indentation changes
|
||||
- Logic modifications
|
||||
- Test changes (unless regression detected)
|
||||
|
||||
## Architecture Reference
|
||||
|
||||
- `conductor/code_styleguides/python.md` — Style conventions
|
||||
- `src/gui_2.py` — Reference for region block pattern (5000+ line refactor)
|
||||
- `docs/guide_architecture.md` — AI client threading model context
|
||||
@@ -0,0 +1,110 @@
|
||||
# AI Server IPC Design
|
||||
|
||||
## Overview
|
||||
|
||||
Decouple heavy AI SDK imports (google.genai, anthropic) from the GUI process via a subprocess command queue. GUI starts instantly (~0.5s) while AI server loads in background (~1.2s one-time cost).
|
||||
|
||||
## Architecture
|
||||
|
||||
```
|
||||
GUI Process AI Server Process
|
||||
+-----------+ +------------------+
|
||||
| Command |----pipe/json--->| AI processing |
|
||||
| Queue | | (google.genai, |
|
||||
+-----------+ | anthropic) |
|
||||
| Response |<---pipe/json-----|------------------+
|
||||
| Queue | | |
|
||||
+-----------+ +------------------+
|
||||
```
|
||||
|
||||
## Command Queue (Input to AI Server)
|
||||
|
||||
Format: JSON lines on stdin
|
||||
```json
|
||||
{"id": "uuid", "method": "send", "params": {...}}
|
||||
{"id": "uuid", "method": "list_models", "params": {}}
|
||||
```
|
||||
|
||||
## Response Queue (Output from AI Server)
|
||||
|
||||
Format: JSON lines on stdout
|
||||
```json
|
||||
{"id": "uuid", "result": {...}}
|
||||
{"id": "uuid", "error": "message"}
|
||||
```
|
||||
|
||||
## Commands
|
||||
|
||||
| Method | Params | Description |
|
||||
|--------|--------|-------------|
|
||||
| `send` | `{history, model, provider, tools}` | Send AI request |
|
||||
| `list_models` | `{provider}` | List available models |
|
||||
| `cleanup` | `{}` | Cleanup sessions |
|
||||
| `reset_session` | `{}` | Reset conversation history |
|
||||
| `set_provider` | `{provider, model}` | Switch provider |
|
||||
| `set_credentials` | `{creds}` | Set API credentials |
|
||||
|
||||
## AI Server Lifecycle
|
||||
|
||||
1. **Spawn**: `subprocess.Popen(["python", "-m", "src.ai_server"])`
|
||||
2. **Startup**: Load google.genai, anthropic SDKs (~1.2s)
|
||||
3. **Ready**: Send `{"type": "ready"}` to GUI
|
||||
4. **Process**: Read commands, write responses
|
||||
5. **Shutdown**: On GUI exit or disconnect
|
||||
|
||||
## GUI Response
|
||||
|
||||
- **Immediate return** from command queue operations
|
||||
- **Background thread reads** response queue
|
||||
- **No polling** - blocking read with timeout
|
||||
- **Lock-free** queue operations
|
||||
|
||||
## Status Indicator
|
||||
|
||||
GUI tracks AI server state:
|
||||
- `init` - Server starting
|
||||
- `ready` - Server loaded, accepting requests
|
||||
- `busy` - Processing request
|
||||
- `error` - Server error state
|
||||
|
||||
Panels that need AI show "Initializing..." tint when `status != ready`.
|
||||
|
||||
## Implementation
|
||||
|
||||
### Files
|
||||
|
||||
- `src/ai_server.py` - Subprocess AI server (new)
|
||||
- `src/ai_client_proxy.py` - Queue client for GUI (new)
|
||||
- Modify `src/ai_client.py` - Route via proxy when AI server enabled
|
||||
|
||||
### ai_server.py
|
||||
|
||||
```
|
||||
- stdin reader loop
|
||||
- Command dispatcher
|
||||
- Provider wrappers (google.genai, anthropic)
|
||||
- stdout writer
|
||||
```
|
||||
|
||||
### ai_client_proxy.py
|
||||
|
||||
```
|
||||
- Command queue (subprocess.stdin)
|
||||
- Response queue (subprocess.stdout reader thread)
|
||||
- Request/response matching by ID
|
||||
- Timeout handling
|
||||
```
|
||||
|
||||
## Error Handling
|
||||
|
||||
- **Server crash**: GUI detects via broken pipe, auto-restart server
|
||||
- **Timeout**: Requests timeout after 60s, return error
|
||||
- **Queue full**: Backpressure, return busy status
|
||||
|
||||
## Startup Sequence
|
||||
|
||||
1. GUI starts, shows immediate (~0.5s)
|
||||
2. Spawn ai_server subprocess
|
||||
3. Server loads SDKs (~1.2s)
|
||||
4. Server sends `{"type": "ready"}`
|
||||
5. GUI enables AI panels
|
||||
@@ -0,0 +1,192 @@
|
||||
# Hot Reloader Design Spec
|
||||
|
||||
**Date:** 2026-05-14
|
||||
**Author:** Tier 2 Tech Lead
|
||||
**Status:** Draft
|
||||
|
||||
## Overview
|
||||
|
||||
Implement a selective, state-preserving hot-reload system for the Manual Slop `./src` Python codebase. This follows the "data in stable memory, code in reloadable modules" pattern pioneered by Casey's Handmade Hero hot-reload in C.
|
||||
|
||||
## Goals
|
||||
|
||||
- Enable hot-reloading of `src/gui_2.py` and `src/app_controller.py` initially
|
||||
- Preserve App instance state across reloads (scroll position, form inputs, open windows)
|
||||
- Extensible architecture — future hot modules (e.g., `ai_client.py`) can be added via registry
|
||||
- Manual trigger only (Ctrl+Alt+R keyboard shortcut + GUI button)
|
||||
- Silent fallback on failure with visual error tint
|
||||
|
||||
## Architecture
|
||||
|
||||
### Delegation Pattern
|
||||
|
||||
Code is separated into **delegators** (thin wrappers in stable modules) and **delegation targets** (actual logic in reloadable modules).
|
||||
|
||||
```python
|
||||
# src/gui_2.py (reloadable)
|
||||
def render_main_interface(app: App) -> None:
|
||||
if app.perf_profiling_enabled:
|
||||
app.perf_monitor.start_component("_render_main_interface")
|
||||
app._render_window_if_open("Project Settings", app._render_project_settings_hub)
|
||||
# ... all render logic here
|
||||
|
||||
# src/app_controller.py (stable) — App class stays stable
|
||||
class App:
|
||||
def _render_main_interface(self) -> None:
|
||||
import src.gui_2 as gui2
|
||||
gui2.render_main_interface(self)
|
||||
```
|
||||
|
||||
### HotModule Registry Entry
|
||||
|
||||
```python
|
||||
@dataclass
|
||||
class HotModule:
|
||||
name: str # "src.gui_2"
|
||||
file_path: str # Absolute path to .py
|
||||
state_keys: list[str] # App attrs to preserve during reload
|
||||
delegation_targets: list[str] # Method names on App that delegate here
|
||||
```
|
||||
|
||||
### HotReloader Core API
|
||||
|
||||
```python
|
||||
class HotReloader:
|
||||
HOT_MODULES: dict[str, HotModule] # Module registry
|
||||
|
||||
@classmethod
|
||||
def register(cls, module: HotModule) -> None: ...
|
||||
|
||||
@classmethod
|
||||
def reload(cls, module_name: str, app: App) -> bool:
|
||||
"""Reload a single module. Returns True on success, False on failure."""
|
||||
|
||||
@classmethod
|
||||
def reload_all(cls, app: App) -> bool:
|
||||
"""Reload all registered modules in dependency order."""
|
||||
|
||||
@classmethod
|
||||
def capture_state(cls, app: App, state_keys: list[str]) -> dict: ...
|
||||
|
||||
@classmethod
|
||||
def restore_state(cls, app: App, state: dict) -> None: ...
|
||||
```
|
||||
|
||||
### State Capture/Restore
|
||||
|
||||
Before reload, `HotReloader.capture_state()` serializes App attributes listed in `state_keys`. After reload (success or failure), `restore_state()` writes them back.
|
||||
|
||||
State is captured as a dict of `{attr_name: copy.deepcopy(value)}`. Restoration uses `setattr` with re-imported module reference.
|
||||
|
||||
### Error Handling Flow
|
||||
|
||||
```
|
||||
Reload triggered
|
||||
↓
|
||||
capture_state(app)
|
||||
↓
|
||||
attempt importlib.reload(module)
|
||||
↓
|
||||
on Exception:
|
||||
restore_state(app) # revert to pre-reload state
|
||||
cls.last_error = traceback
|
||||
cls.is_error_state = True
|
||||
tint GUI red
|
||||
return False
|
||||
↓
|
||||
on success:
|
||||
clear last_error
|
||||
clear error tint
|
||||
return True
|
||||
```
|
||||
|
||||
### Trigger Mechanism
|
||||
|
||||
- `Ctrl+Alt+R` keyboard shortcut captured in main input loop
|
||||
- GUI button in MMA Dashboard: "Hot Reload" with icon
|
||||
- Both call `HotReloader.reload_all(self)` on the App instance
|
||||
|
||||
### Visual Error Tint
|
||||
|
||||
When `HotReloader.is_error_state` is True:
|
||||
- If NERV theme active: overlay with NERV red (rgba 255, 72, 64, alpha)
|
||||
- Else: overlay with red tint
|
||||
- Tint cleared on next successful reload
|
||||
|
||||
## Module Registry (Initial)
|
||||
|
||||
```python
|
||||
HOT_MODULES = {
|
||||
"src.gui_2": HotModule(
|
||||
name="src.gui_2",
|
||||
file_path=str(Path(__file__).parent / "gui_2.py"),
|
||||
state_keys=[
|
||||
"_active_discussion", "_disc_entries", "_disc_roles",
|
||||
"show_windows", "ui_discussion_split_h", "active_tickets",
|
||||
# ... more keys TBD during implementation
|
||||
],
|
||||
delegation_targets=[
|
||||
"_render_main_interface", "_render_discussion_hub",
|
||||
"_render_discussion_panel", "_render_discussion_selector",
|
||||
# ... more targets TBD during implementation
|
||||
],
|
||||
),
|
||||
}
|
||||
```
|
||||
|
||||
## Delegation Refactoring Phases
|
||||
|
||||
### Phase 1: GUI Methods
|
||||
Extract render methods from `App` in `app_controller.py` into delegation targets in `gui_2.py`.
|
||||
|
||||
### Phase 2: State Keys Inventory
|
||||
Catalog all `App` instance attributes that need preservation.
|
||||
|
||||
### Phase 3: HotReloader Implementation
|
||||
Implement `src/hot_reloader.py` with capture/restore, registry, and error handling.
|
||||
|
||||
### Phase 4: Trigger Integration
|
||||
Add Ctrl+Alt+R handler and GUI button.
|
||||
|
||||
### Phase 5: Visual Tint
|
||||
Implement error tint overlay.
|
||||
|
||||
## Extensibility: Adding Future Hot Modules
|
||||
|
||||
```python
|
||||
# Add ai_client as hot module:
|
||||
HotReloader.register(HotModule(
|
||||
name="src.ai_client",
|
||||
file_path=str(Path(__file__).parent / "ai_client.py"),
|
||||
state_keys=["_pending_requests", "_api_key"],
|
||||
delegation_targets=["_send_request", "_stream_response"],
|
||||
))
|
||||
|
||||
# Or via decorator on the delegation target:
|
||||
@hot_module(state_keys=["_pending_requests"])
|
||||
def send_request(app: App) -> None:
|
||||
...
|
||||
```
|
||||
|
||||
## Files Affected
|
||||
|
||||
| File | Change |
|
||||
|------|--------|
|
||||
| `src/hot_reloader.py` | New — HotReloader class |
|
||||
| `src/gui_2.py` | Refactor render methods to module-level functions |
|
||||
| `src/app_controller.py` | Refactor App methods to delegation wrappers |
|
||||
| `src/imgui_scopes.py` | Unchanged (still used by refactored code) |
|
||||
|
||||
## Success Criteria
|
||||
|
||||
1. Pressing Ctrl+Alt+R reloads `src.gui_2` without losing App state
|
||||
2. GUI button triggers same reload
|
||||
3. Failed reload shows error tint, preserves last-good state
|
||||
4. New hot modules can be added via `HotReloader.register()` without modifying core
|
||||
5. No performance impact when not reloading (< 1ms overhead per frame)
|
||||
|
||||
## Out of Scope
|
||||
|
||||
- Automatic file watching (manual trigger only per design)
|
||||
- Hot-reloading of C extensions or native code
|
||||
- Cross-platform reload support (Windows focus for now)
|
||||
@@ -0,0 +1,274 @@
|
||||
# Performance Profiling System Design Spec
|
||||
|
||||
**Date:** 2026-05-15
|
||||
**Author:** Tier 2 Tech Lead
|
||||
**Status:** Draft
|
||||
|
||||
## Overview
|
||||
|
||||
Implement a layered performance profiling system for Manual Slop's `./src` codebase:
|
||||
|
||||
- **Phase 1:** Enhanced Diagnostics Panel — sorted component timings, expandable detail rows
|
||||
- **Phase 2:** Tracy integration via `pytracy` — real-time flamegraph streaming to Tracy GUI
|
||||
- **Phase 3 (future):** Custom `sys.settrace` sampler + imgui flamegraph as fallback
|
||||
|
||||
## Goals
|
||||
|
||||
- Surface hot code paths with zero friction (always-on real-time highlighting)
|
||||
- Enable deep dive profiling on demand via industry-standard tools
|
||||
- Preserve existing PerformanceMonitor infrastructure
|
||||
- Self-contained — minimal external dependencies beyond Tracy
|
||||
|
||||
## Phase 1: Enhanced Diagnostics Panel
|
||||
|
||||
### What's Already There
|
||||
|
||||
The Diagnostics Panel (`_render_diagnostics_panel` in `gui_2.py`) already displays:
|
||||
- FPS, Frame Time, CPU %, Input Lag with live values
|
||||
- Optional per-metric graphs (toggle checkbox)
|
||||
- Detailed Component Timings table: Avg, Count, Max, Min per component
|
||||
- RED highlighting for components with avg > 10ms
|
||||
- Performance Graphs section with rolling history plots
|
||||
- Diagnostic Log table
|
||||
|
||||
### What's Missing (for D)
|
||||
|
||||
1. **No sorting** — components iterate in dict hash order, not worst-first
|
||||
2. **No expandability** — cannot click a row to see full stats breakdown
|
||||
|
||||
### Implementation
|
||||
|
||||
**Sort by worst avg:**
|
||||
```python
|
||||
sorted_components = sorted(
|
||||
[(k, v) for k, v in metrics.items() if k.startswith("time_") and k.endswith("_ms")],
|
||||
key=lambda x: metrics.get(f"{x[0]}_avg", x[1]),
|
||||
reverse=True
|
||||
)
|
||||
```
|
||||
|
||||
**Expandable rows:**
|
||||
```python
|
||||
for key, val in sorted_components:
|
||||
# Render collapsed row with summary
|
||||
expanded = imgui.tree_node(comp_name)
|
||||
if expanded:
|
||||
# Show: last_value, avg, count, max, min, peak frames, stddev
|
||||
imgui.text(f"Last: {val:.2f}ms")
|
||||
imgui.text(f"StdDev: {stddev:.2f}ms")
|
||||
imgui.text(f"Peak frame: {peak_val:.2f}ms at frame {peak_frame}")
|
||||
imgui.tree_pop()
|
||||
```
|
||||
|
||||
### Files Affected
|
||||
|
||||
| File | Change |
|
||||
|------|--------|
|
||||
| `src/gui_2.py` | `_render_diagnostics_panel` — sort + expandable rows |
|
||||
|
||||
### Success Criteria
|
||||
|
||||
1. Component timings table sorted by worst avg descending
|
||||
2. Click a row → expands to show last, stddev, peak info
|
||||
3. Existing red highlighting (>10ms) preserved
|
||||
4. No regression to other diagnostics panel features
|
||||
|
||||
## Phase 2: Tracy Integration
|
||||
|
||||
### Tracy Overview
|
||||
|
||||
Tracy is a real-time, nanosecond-resolution, frame-based profiler for game devs and high-performance applications. It streams profiling data to a dedicated GUI client over a TCP connection. Features:
|
||||
|
||||
- Live CPU profiling with call stacks
|
||||
- Memory profiling (allocations, leaks)
|
||||
- Lock contention visualization
|
||||
- Frame capture (good for your Dear PyGui render loop)
|
||||
- Very low overhead (~1-2%)
|
||||
|
||||
### pytracy Binding
|
||||
|
||||
`pytracy` is a Python binding on PyPI:
|
||||
```bash
|
||||
uv add pytracy
|
||||
```
|
||||
|
||||
Basic usage:
|
||||
```python
|
||||
import pytracy
|
||||
pytracy.setproctitle("manual_slop")
|
||||
|
||||
# Zone annotations (instrumentation)
|
||||
def long_running_function():
|
||||
pytracy.begin("my_zone")
|
||||
# ... work ...
|
||||
pytracy.end("my_zone")
|
||||
```
|
||||
|
||||
### Integration Points in Manual Slop
|
||||
|
||||
**1. Process naming:**
|
||||
```python
|
||||
# In App.__init__ or gui_2.py:
|
||||
pytracy.setproctitle("manual_slop")
|
||||
```
|
||||
|
||||
**2. Zone instrumentation for render components:**
|
||||
```python
|
||||
# In each _render_* method:
|
||||
with pytracy.ctx_zone(name="_render_discussion_panel"):
|
||||
# ... render logic ...
|
||||
```
|
||||
|
||||
**3. Memory tracking (optional):**
|
||||
```python
|
||||
pytracy.allocator_hook_enable()
|
||||
```
|
||||
|
||||
**4. Connection handling:**
|
||||
Tracy GUI must be running and listening before manual_slop starts. The app connects to `localhost:8086` by default (configurable).
|
||||
|
||||
### Tracy GUI
|
||||
|
||||
- Tracy has its own cross-platform UI (Windows/macOS/Linux)
|
||||
- Download from https://github.com/wolfpld/tracy/releases or build from source
|
||||
- Once connected, you get live flamegraphs, frame time charts, memory graphs
|
||||
- Can save trace files for later analysis
|
||||
|
||||
### Graceful Degradation
|
||||
|
||||
If Tracy is not running or `pytracy` import fails:
|
||||
- App continues normally — profiling is opt-in
|
||||
- Existing PerformanceMonitor keeps working
|
||||
- Log a warning on startup: "Tracy not connected — profiling unavailable"
|
||||
|
||||
### Implementation
|
||||
|
||||
```python
|
||||
# src/profiling/tracy_integration.py (new file)
|
||||
from __future__ import annotations
|
||||
import sys
|
||||
from typing import Optional
|
||||
|
||||
_tracy_available = False
|
||||
_tracy = None
|
||||
|
||||
def init_tracy() -> bool:
|
||||
"""Try to initialize pytracy connection. Returns True on success."""
|
||||
global _tracy_available, _tracy
|
||||
try:
|
||||
import pytracy
|
||||
_tracy = pytracy
|
||||
pytracy.setproctitle("manual_slop")
|
||||
_tracy_available = True
|
||||
return True
|
||||
except Exception:
|
||||
_tracy_available = False
|
||||
return False
|
||||
|
||||
def is_tracy_available() -> bool:
|
||||
return _tracy_available
|
||||
|
||||
class TracyZone:
|
||||
"""Context manager for Tracy zones."""
|
||||
def __init__(self, name: str) -> None:
|
||||
self.name = name
|
||||
self.active = False
|
||||
|
||||
def __enter__(self):
|
||||
if _tracy_available and _tracy:
|
||||
_tracy.enter(self.name)
|
||||
self.active = True
|
||||
return self
|
||||
|
||||
def __exit__(self, *args):
|
||||
if self.active:
|
||||
_tracy.leave(self.name)
|
||||
return False
|
||||
|
||||
# Convenience decorator
|
||||
def tracy_zone(name: str):
|
||||
"""Decorator to wrap a function in a Tracy zone."""
|
||||
def decorator(func):
|
||||
def wrapper(*args, **kwargs):
|
||||
with TracyZone(name):
|
||||
return func(*args, **kwargs)
|
||||
return wrapper
|
||||
return decorator
|
||||
```
|
||||
|
||||
### GUI Button for Tracy Status
|
||||
|
||||
Add to Diagnostics Panel:
|
||||
```python
|
||||
if imgui.button("Open Tracy GUI"):
|
||||
import subprocess
|
||||
subprocess.Popen(["tracy"]) # Or path to Tracy executable
|
||||
imgui.same_line()
|
||||
imgui.text(f"Tracy: {'Connected' if tracy_available else 'Not connected'}")
|
||||
```
|
||||
|
||||
### Files Affected
|
||||
|
||||
| File | Change |
|
||||
|------|--------|
|
||||
| `src/profiling/tracy_integration.py` | New — Tracy integration module |
|
||||
| `src/gui_2.py` | Add Tracy zone wrappers around render methods, button in Diagnostics |
|
||||
| `src/app_controller.py` | Optionally add Tracy init in startup |
|
||||
|
||||
### Success Criteria
|
||||
|
||||
1. `pytracy` is a declared dependency in pyproject.toml
|
||||
2. Tracy zones wrap every `_render_*` component in `gui_2.py`
|
||||
3. App starts without error if Tracy GUI is not running (graceful degradation)
|
||||
4. When Tracy GUI is running, live flamegraph appears for running app
|
||||
5. "Open Tracy GUI" button launches Tracy if installed
|
||||
|
||||
## Phase 3 (Future): Custom Sampler Fallback
|
||||
|
||||
Out of scope for initial spec. Would implement `sys.settrace` based sampler if:
|
||||
- Tracy is unavailable/unwanted
|
||||
- User wants self-contained flamegraph rendered in imgui directly
|
||||
|
||||
## Architecture
|
||||
|
||||
```
|
||||
Diagnostics Panel (gui_2.py)
|
||||
|
|
||||
├── PerformanceMonitor (component timings, always-on)
|
||||
| └── O(1) rolling averages, per-component ms tracking
|
||||
|
|
||||
└── Tracy Integration (profiling, on-demand)
|
||||
└── pytracy → Tracy GUI (live flamegraph + memory + locks)
|
||||
```
|
||||
|
||||
Both run independently. PerformanceMonitor gives per-frame glanceable data. Tracy gives deep dive on demand.
|
||||
|
||||
## Dependencies
|
||||
|
||||
| Dependency | Purpose | Notes |
|
||||
|------------|---------|-------|
|
||||
| `pytracy` | Tracy Python binding | Phase 2 only, graceful degradation if unavailable |
|
||||
|
||||
## Files
|
||||
|
||||
| File | Action |
|
||||
|------|--------|
|
||||
| `src/profiling/tracy_integration.py` | Create — Tracy wrapper with graceful degradation |
|
||||
| `src/profiling/__init__.py` | Create — Package init |
|
||||
| `src/gui_2.py` | Modify — Sort + expand Diagnostics, wrap render zones |
|
||||
| `src/app_controller.py` | Optional — Tracy init in startup |
|
||||
| `pyproject.toml` | Modify — Add `pytracy` dependency |
|
||||
|
||||
## Open Questions
|
||||
|
||||
1. **Tracy connection params** — default `localhost:8086`, should be configurable via `config.toml`?
|
||||
2. **Which render methods to zone** — All `_render_*` or only top-level ones?
|
||||
3. **Memory profiling** — Enable allocator hook by default or opt-in only?
|
||||
|
||||
## Success Criteria Summary
|
||||
|
||||
- [ ] Phase 1: Diagnostics panel sorts by worst avg, rows expandable
|
||||
- [ ] Phase 2: pytracy integrated with graceful degradation
|
||||
- [ ] Phase 2: Every `_render_*` method in gui_2.py has Tracy zone
|
||||
- [ ] Phase 2: Tracy GUI shows live flamegraph when connected
|
||||
- [ ] Phase 3: Future extension point clear for custom sampler
|
||||
@@ -0,0 +1,150 @@
|
||||
# OpenCode Agent Definition Fix - Design Spec
|
||||
|
||||
## Overview
|
||||
|
||||
Fix OpenCode agent definitions so subagents spawned via `@mention` (when using superpowers) properly enforce the conductor workflow. Currently subagents fail to follow 1-space indentation, TDD protocols, and delegation rules even when explicitly instructed.
|
||||
|
||||
## Current State Audit
|
||||
|
||||
### Files Involved
|
||||
- `.opencode/agents/tier3-worker.md` - Tier 3 Worker agent definition
|
||||
- `.opencode/agents/tier4-qa.md` - Tier 4 QA agent definition
|
||||
- `.opencode/agents/tier2-tech-lead.md` - Tier 2 Tech Lead agent definition
|
||||
- `.opencode/agents/tier1-orchestrator.md` - Tier 1 Orchestrator agent definition
|
||||
|
||||
### Problems Identified
|
||||
|
||||
1. **Wrong MCP Tool Naming**
|
||||
- Current: `discovered_tool_py_get_code_outline`, `discovered_tool_run_powershell`
|
||||
- Correct: `manual-slop_py_get_code_outline`, `manual-slop_run_powershell`
|
||||
- Result: Subagents cannot find/locate MCP tools
|
||||
|
||||
2. **Insufficient Code Style Enforcement**
|
||||
- "1-space indentation" mentioned but not CRITICAL
|
||||
- No explicit warning about native edit tool destroying indentation
|
||||
- Subagents default to 4-space on unknown files
|
||||
|
||||
3. **Missing Conductor Workflow Structure**
|
||||
- No pre-delegation checkpoint protocol (`git add .`)
|
||||
- No atomic commit per-task rules
|
||||
- No TDD Red→Green→Refactor phase enforcement
|
||||
- No mandatory task state tracking in plan.md
|
||||
|
||||
4. **Context Amnesia Not Addressed**
|
||||
- Subagents lose all workflow context between tasks
|
||||
- No reminder about stateless operation
|
||||
- No checkpoint/commit reminder system
|
||||
|
||||
5. **Tier 3 Worker Mode Incorrect**
|
||||
- `mode: subagent` may cause OpenCode to strip context
|
||||
- May need `mode: primary` for full workflow access
|
||||
|
||||
## Goals
|
||||
|
||||
1. Subagents spawned via `@mention` follow conductor workflow without deviation
|
||||
2. 1-space indentation enforced as CRITICAL for all Python code
|
||||
3. TDD phases (Red→Green→Refactor) enforced mandatorily
|
||||
4. MCP tool names correct and functional
|
||||
5. Pre-delegation checkpoints prevent work loss
|
||||
6. Atomic commits per task maintained
|
||||
|
||||
## Functional Requirements
|
||||
|
||||
### 1. Agent Definition Rewrite
|
||||
|
||||
All tier agent files (`.opencode/agents/*.md`) must include:
|
||||
|
||||
- **CRITICAL Indentation Block** at top:
|
||||
```
|
||||
## CRITICAL: 1-Space Indentation for Python
|
||||
|
||||
ALL Python code MUST use exactly 1 (ONE) space for indentation.
|
||||
VIOLATION: Using 4 spaces or tabs will corrupt the codebase.
|
||||
```
|
||||
|
||||
- **MCP Tool Mapping Table** (correct names):
|
||||
```
|
||||
| Native Tool | MCP Tool |
|
||||
|-------------|----------|
|
||||
| read | manual-slop_read_file |
|
||||
| edit | manual-slop_edit_file |
|
||||
| bash | manual-slop_run_powershell |
|
||||
| glob | manual-slop_search_files |
|
||||
```
|
||||
|
||||
- **Pre-Delegation Checkpoint Protocol**:
|
||||
```
|
||||
## Pre-Delegation Checkpoint (MANDATORY)
|
||||
|
||||
Before delegating ANY change:
|
||||
1. Run: git add .
|
||||
2. Reason: Prevents work loss if subagent fails
|
||||
```
|
||||
|
||||
- **TDD Phase Enforcement**:
|
||||
```
|
||||
## TDD Protocol
|
||||
|
||||
1. RED: Write failing test, confirm failure
|
||||
2. GREEN: Implement to pass, confirm pass
|
||||
3. REFACTOR: Optional, with passing tests
|
||||
NEVER skip phases.
|
||||
```
|
||||
|
||||
- **Atomic Commit Rules**:
|
||||
```
|
||||
## Commit Protocol
|
||||
|
||||
After each task: git add . && git commit
|
||||
Do NOT batch commits.
|
||||
```
|
||||
|
||||
### 2. Tier 3 Worker Specific Fixes
|
||||
|
||||
- Verify `mode: subagent` is correct for OpenCode's @mention system
|
||||
- Add WHERE/WHAT/HOW/SAFETY task structure reminder
|
||||
- Add BLOCKED protocol for unresolvable tasks
|
||||
- Include Context Amnesia reminder (fresh context each task)
|
||||
|
||||
### 3. Tier 4 QA Specific Fixes
|
||||
|
||||
- Add DO NOT FIX warning - analysis only
|
||||
- Include root cause tracing instructions
|
||||
- Add data flow tracing from docs/guide_architecture.md
|
||||
|
||||
### 4. Tier 2 Tech Lead Specific Fixes
|
||||
|
||||
- Add persistent memory reminder
|
||||
- Add surgical prompt structure (WHERE/WHAT/HOW/SAFETY)
|
||||
- Add delegation via Task tool instructions
|
||||
- Add phase completion verification protocol
|
||||
|
||||
### 5. Tier 1 Orchestrator Specific Fixes
|
||||
|
||||
- Verify mode: primary for main agent sessions
|
||||
- Add track initialization workflow
|
||||
- Add product alignment check
|
||||
|
||||
## Architecture Reference
|
||||
|
||||
Based on:
|
||||
- `conductor/workflow.md` - TDD protocol, commit rules
|
||||
- `conductor/product-guidelines.md` - Code style (1-space indentation)
|
||||
- `opencode.json` - MCP tool definitions (manual-slop_*)
|
||||
- superpowers docs - @mention subagent spawning
|
||||
|
||||
## Out of Scope
|
||||
|
||||
- Changes to superpowers plugin itself
|
||||
- Changes to OpenCode core behavior
|
||||
- Changes to mma_exec.py (not used in OpenCode flow)
|
||||
- Changes to Gemini CLI workflow (separate system)
|
||||
|
||||
## Success Criteria
|
||||
|
||||
1. All Python files in project use 1-space indentation
|
||||
2. Subagents follow TDD protocol without skipping phases
|
||||
3. Pre-delegation checkpoints executed before risky operations
|
||||
4. MCP tools findable by subagents
|
||||
5. Atomic commits maintained per task
|
||||
6. Conductor workflow followed without deviation even after corrections
|
||||
@@ -0,0 +1,195 @@
|
||||
# Clean Install Test
|
||||
|
||||
**Date:** 2026-06-02
|
||||
**Status:** Draft (pending review)
|
||||
|
||||
---
|
||||
|
||||
## Context & Motivation
|
||||
|
||||
The user wants a "clean install" test that verifies Manual Slop works correctly when installed from scratch in an isolated environment. The test should:
|
||||
|
||||
1. Clone the repo to a temp directory (no shared state with the source)
|
||||
2. Install dependencies via `uv sync`
|
||||
3. Launch `sloppy.py --enable-test-hooks`
|
||||
4. Verify the Hook API responds (smoke test that the app is functional)
|
||||
|
||||
This is a defense against:
|
||||
- Repository changes that only work on the developer's machine
|
||||
- Dependency drift (works on dev, fails on fresh install)
|
||||
- Build/launch issues that only appear in clean environments
|
||||
|
||||
The target is the user's private Gitea server: `https://git.cozyair.dev/ed/manual_slop`. This is intentionally NOT a public GitHub URL — the test must work in the user's private infrastructure.
|
||||
|
||||
---
|
||||
|
||||
## Scope
|
||||
|
||||
### In Scope
|
||||
|
||||
- `tests/test_clean_install.py` — Opt-in pytest test
|
||||
- `pyproject.toml` update: add `clean_install` marker
|
||||
- Gating via `RUN_CLEAN_INSTALL_TEST=1` env var
|
||||
|
||||
### Out of Scope
|
||||
|
||||
- Auto-clone in CI (the user can opt in via a future CI workflow)
|
||||
- Continuous monitoring
|
||||
- Cloning from a specific branch/tag (uses HEAD of main by default)
|
||||
|
||||
---
|
||||
|
||||
## Design
|
||||
|
||||
### Test File Structure
|
||||
|
||||
```python
|
||||
# tests/test_clean_install.py
|
||||
import os
|
||||
import shutil
|
||||
import subprocess
|
||||
import time
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
import requests
|
||||
|
||||
|
||||
REPO_URL = "https://git.cozyair.dev/ed/manual_slop"
|
||||
STARTUP_TIMEOUT_SECONDS = 30
|
||||
READINESS_POLL_INTERVAL = 0.5
|
||||
|
||||
|
||||
@pytest.mark.clean_install
|
||||
def test_clean_install_runs_with_hooks(tmp_path):
|
||||
"""Clone the repo, install deps, launch sloppy.py, verify Hook API."""
|
||||
if os.environ.get("RUN_CLEAN_INSTALL_TEST") != "1":
|
||||
pytest.skip("Set RUN_CLEAN_INSTALL_TEST=1 to enable")
|
||||
|
||||
clone_dir = tmp_path / "manual_slop"
|
||||
|
||||
# 1. Clone
|
||||
result = subprocess.run(
|
||||
["git", "clone", REPO_URL, str(clone_dir)],
|
||||
capture_output=True, text=True, timeout=60,
|
||||
)
|
||||
assert result.returncode == 0, f"Clone failed: {result.stderr}"
|
||||
|
||||
# 2. Install deps
|
||||
result = subprocess.run(
|
||||
["uv", "sync"],
|
||||
cwd=str(clone_dir),
|
||||
capture_output=True, text=True, timeout=180,
|
||||
)
|
||||
assert result.returncode == 0, f"uv sync failed: {result.stderr}"
|
||||
|
||||
# 3. Launch sloppy.py with hooks
|
||||
process = subprocess.Popen(
|
||||
["uv", "run", "sloppy.py", "--enable-test-hooks"],
|
||||
cwd=str(clone_dir),
|
||||
stdout=subprocess.PIPE, stderr=subprocess.PIPE,
|
||||
creationflags=subprocess.CREATE_NEW_PROCESS_GROUP if os.name == 'nt' else 0,
|
||||
)
|
||||
|
||||
try:
|
||||
# 4. Poll /status endpoint
|
||||
start = time.time()
|
||||
ready = False
|
||||
while time.time() - start < STARTUP_TIMEOUT_SECONDS:
|
||||
if process.poll() is not None:
|
||||
pytest.fail(f"Process exited early. stderr: {process.stderr.read()[:2000]}")
|
||||
try:
|
||||
response = requests.get(
|
||||
"http://127.0.0.1:8999/status",
|
||||
timeout=1.0,
|
||||
)
|
||||
if response.status_code == 200:
|
||||
payload = response.json()
|
||||
if payload.get("status") == "running":
|
||||
ready = True
|
||||
break
|
||||
except (requests.ConnectionError, requests.Timeout):
|
||||
pass
|
||||
time.sleep(READINESS_POLL_INTERVAL)
|
||||
|
||||
assert ready, f"Hook server did not respond within {STARTUP_TIMEOUT_SECONDS}s"
|
||||
|
||||
# 5. Test a write hook (any POST endpoint that should respond)
|
||||
response = requests.get(
|
||||
"http://127.0.0.1:8999/api/gui/mma_status",
|
||||
timeout=5.0,
|
||||
)
|
||||
assert response.status_code == 200
|
||||
# The mma_status endpoint returns a dict; verify it has expected keys
|
||||
data = response.json()
|
||||
assert "status" in data or "mma_state" in data
|
||||
|
||||
finally:
|
||||
# 6. Cleanup
|
||||
if os.name == 'nt':
|
||||
subprocess.run(
|
||||
["taskkill", "/F", "/T", "/PID", str(process.pid)],
|
||||
capture_output=True,
|
||||
)
|
||||
else:
|
||||
process.terminate()
|
||||
try:
|
||||
process.wait(timeout=5)
|
||||
except subprocess.TimeoutExpired:
|
||||
process.kill()
|
||||
```
|
||||
|
||||
### `pyproject.toml` Update
|
||||
|
||||
```toml
|
||||
[tool.pytest.ini_options]
|
||||
markers = [
|
||||
"integration: integration tests requiring live GUI",
|
||||
"strict: tests that require strict mode",
|
||||
"clean_install: clean install verification (opt-in via RUN_CLEAN_INSTALL_TEST=1)",
|
||||
]
|
||||
```
|
||||
|
||||
### Running the Test
|
||||
|
||||
**Default (skip):**
|
||||
```bash
|
||||
uv run pytest tests/test_clean_install.py -v
|
||||
# SKIPPED: Set RUN_CLEAN_INSTALL_TEST=1 to enable
|
||||
```
|
||||
|
||||
**Opt-in:**
|
||||
```bash
|
||||
RUN_CLEAN_INSTALL_TEST=1 uv run pytest tests/test_clean_install.py -v
|
||||
```
|
||||
|
||||
**Just the clean_install marker:**
|
||||
```bash
|
||||
RUN_CLEAN_INSTALL_TEST=1 uv run pytest -m clean_install -v
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## File Structure
|
||||
|
||||
- `tests/test_clean_install.py` — NEW
|
||||
- `pyproject.toml` — MODIFY: add `clean_install` marker
|
||||
|
||||
---
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- `RUN_CLEAN_INSTALL_TEST=1 uv run pytest tests/test_clean_install.py -v` passes when run in an environment with network access to `git.cozyair.dev`
|
||||
- Without the env var, the test skips (no network access required)
|
||||
- The test takes 30-90 seconds to run (clone + install + launch)
|
||||
- A failure in any step (clone, sync, launch, hook response) results in a clear error message
|
||||
- Process cleanup is robust (no orphaned processes on Windows or Unix)
|
||||
|
||||
---
|
||||
|
||||
## Risks
|
||||
|
||||
1. **Network dependency:** The test requires network access to `git.cozyair.dev`. In CI environments without that access, the test will fail. Mitigation: the env var gating makes this opt-in.
|
||||
2. **Clone target is private:** Unlike GitHub, the URL is on a private Gitea server. Test failure on a public CI would leak the existence of the private repo. Mitigation: only run on private infrastructure; the test is opt-in.
|
||||
3. **Port conflicts:** The test uses port 8999. If another process is using it, the test will fail. Mitigation: the polling loop detects early exit and reports the port-in-use error.
|
||||
4. **`uv sync` is slow:** On a fresh machine, `uv sync` can take 30-60 seconds. The test budget is 180 seconds which should be sufficient.
|
||||
@@ -0,0 +1,222 @@
|
||||
# Command Palette Implementation & Tests
|
||||
|
||||
**Date:** 2026-06-02
|
||||
**Status:** Draft (pending review)
|
||||
**Parent Track:** `command_palette_and_performance_20260602` (continuing Phase 2 + adding Phase 3)
|
||||
**Spec:** `conductor/tracks/command_palette_and_performance_20260602/spec.md` (existing)
|
||||
|
||||
---
|
||||
|
||||
## Context & Motivation
|
||||
|
||||
A `command_palette_and_performance_20260602` track was started in early June 2026. **Phase 1 (Async Context Preview)** is complete. **Phase 2 (Command Palette)** is unstarted — no `src/command_palette.py`, no `src/commands.py`, no test files. The user reports the palette doesn't pop up on `Ctrl+Shift+P`.
|
||||
|
||||
The existing spec says `Ctrl+P`; this design uses `Ctrl+Shift+P` (per the user's expectation and VSCode convention documented in `docs/guide_command_palette.md`).
|
||||
|
||||
This design finishes Phase 2 of the existing track and adds Phase 3 (tests).
|
||||
|
||||
---
|
||||
|
||||
## Scope
|
||||
|
||||
### In Scope
|
||||
|
||||
- `src/command_palette.py` — Module-level: `Command` dataclass, `CommandRegistry`, `fuzzy_match()`, `render_palette_modal(app)`, `render_everything_modal(app)`
|
||||
- `src/commands.py` — Static command definitions (~30-50 commands across categories)
|
||||
- `src/gui_2.py` — `self.show_command_palette: bool` in `App.__init__`, `_render_command_palette(self)` thin wrapper, `Ctrl+Shift+P` keyboard handler
|
||||
- `tests/test_command_palette.py` — Unit tests for fuzzy matcher, command registry, mode detection
|
||||
- `tests/test_command_palette_sim.py` — Integration tests via `live_gui`
|
||||
|
||||
### Out of Scope
|
||||
|
||||
- Async context preview (Phase 1; already complete)
|
||||
- The "Everything" mode async search worker (mentioned in the existing spec; defer to a follow-up track)
|
||||
- Visual theming of the palette (use existing ImGui style)
|
||||
|
||||
---
|
||||
|
||||
## Design
|
||||
|
||||
### Data Model: `Command`
|
||||
|
||||
```python
|
||||
@dataclass
|
||||
class Command:
|
||||
id: str # Unique identifier
|
||||
title: str # Display name
|
||||
category: str # Category for grouping
|
||||
shortcut: Optional[str] # Optional default shortcut (e.g., "Ctrl+S")
|
||||
description: str = "" # Optional help text
|
||||
enabled_when: Optional[str] = None # Optional condition expression
|
||||
action: Callable = None # Function to execute when selected
|
||||
```
|
||||
|
||||
### Command Registry
|
||||
|
||||
```python
|
||||
# src/commands.py
|
||||
from src.command_palette import Command, CommandRegistry
|
||||
|
||||
registry = CommandRegistry()
|
||||
|
||||
@registry.register
|
||||
def save_file(app: App) -> None:
|
||||
"""Save File — File category, Ctrl+S"""
|
||||
# ... call app's save logic
|
||||
```
|
||||
|
||||
**Registration patterns:**
|
||||
- Decorator: `@registry.register` for top-level functions
|
||||
- Explicit: `registry.register(Command(id=..., title=..., action=...))` for closures or classes
|
||||
|
||||
### Fuzzy Matcher
|
||||
|
||||
Implemented in `src/command_palette.py` as a pure function:
|
||||
|
||||
```python
|
||||
def fuzzy_match(query: str, candidates: List[Command], top_n: int = 20) -> List[ScoredCommand]:
|
||||
"""
|
||||
Returns the top_n candidates matching query, ranked by score.
|
||||
|
||||
Algorithm:
|
||||
1. Subsequence check: query chars must appear in title, in order
|
||||
2. Score calculation:
|
||||
- Exact prefix match: +1.0
|
||||
- Word boundary match: +0.5
|
||||
- Contiguous match: +0.3
|
||||
- Character distance penalty: -0.1 per gap
|
||||
3. Sort by score descending
|
||||
4. Return top_n
|
||||
"""
|
||||
```
|
||||
|
||||
### Modal Rendering
|
||||
|
||||
The palette is a centered ImGui modal. Module-level function (per delegation pattern):
|
||||
|
||||
```python
|
||||
# src/command_palette.py
|
||||
def render_palette_modal(app: App) -> None:
|
||||
"""Render the Command Palette modal. Called from gui_2.py when app.show_command_palette is True."""
|
||||
if not app.show_command_palette:
|
||||
return
|
||||
imgui.set_next_window_position(...) # Centered
|
||||
imgui.set_next_window_size(...)
|
||||
if imgui.begin("Command Palette##palette", closable=True):
|
||||
# Search input
|
||||
# Fuzzy-matched results list
|
||||
# Keyboard navigation
|
||||
imgui.end()
|
||||
```
|
||||
|
||||
### Keyboard Handler
|
||||
|
||||
In `gui_2.py`'s main event loop:
|
||||
|
||||
```python
|
||||
io = imgui.get_io()
|
||||
if io.key_ctrl and io.key_shift and imgui.is_key_pressed(imgui.Key.p):
|
||||
app.show_command_palette = not app.show_command_palette
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## File Structure
|
||||
|
||||
- `src/command_palette.py` — NEW: Command, CommandRegistry, fuzzy_match, render_palette_modal
|
||||
- `src/commands.py` — NEW: Static command definitions and registry
|
||||
- `src/gui_2.py` — MODIFY: add `self.show_command_palette`, add `_render_command_palette` wrapper, add Ctrl+Shift+P handler, register the palette module
|
||||
- `tests/test_command_palette.py` — NEW: Unit tests
|
||||
- `tests/test_command_palette_sim.py` — NEW: Integration tests via live_gui
|
||||
|
||||
---
|
||||
|
||||
## Tests
|
||||
|
||||
### Unit Tests (`tests/test_command_palette.py`)
|
||||
|
||||
```python
|
||||
def test_fuzzy_match_prefix_ranks_first():
|
||||
from src.command_palette import fuzzy_match
|
||||
candidates = [
|
||||
Command(id="find", title="Find in Selection"),
|
||||
Command(id="fold", title="Fold All"),
|
||||
Command(id="config", title="Configure Settings"),
|
||||
]
|
||||
results = fuzzy_match("fin", candidates)
|
||||
assert results[0].command.id == "find"
|
||||
assert results[0].score > 0.5
|
||||
|
||||
def test_fuzzy_match_rejects_no_match():
|
||||
from src.command_palette import fuzzy_match
|
||||
candidates = [Command(id="x", title="foo bar")]
|
||||
results = fuzzy_match("xyz", candidates)
|
||||
assert len(results) == 0
|
||||
|
||||
def test_command_registry_register_and_list():
|
||||
from src.command_palette import CommandRegistry
|
||||
from src.commands import registry
|
||||
assert "save_file" in registry.all()
|
||||
# All commands have id, title, category
|
||||
for cmd in registry.all():
|
||||
assert cmd.id and cmd.title and cmd.category
|
||||
|
||||
def test_command_registry_duplicate_raises():
|
||||
from src.command_palette import CommandRegistry, Command
|
||||
reg = CommandRegistry()
|
||||
reg.register(Command(id="x", title="X", category="test"))
|
||||
with pytest.raises(ValueError):
|
||||
reg.register(Command(id="x", title="X", category="test"))
|
||||
```
|
||||
|
||||
### Integration Tests (`tests/test_command_palette_sim.py`)
|
||||
|
||||
```python
|
||||
def test_ctrl_shift_p_opens_palette(live_gui):
|
||||
client = live_gui[1]
|
||||
# Press Ctrl+Shift+P
|
||||
client.press_key_combo("Ctrl+Shift+P")
|
||||
# Verify the palette is visible
|
||||
state = client.get_window_state("command_palette")
|
||||
assert state["visible"] == True
|
||||
|
||||
def test_palette_filters_as_user_types(live_gui):
|
||||
client = live_gui[1]
|
||||
client.press_key_combo("Ctrl+Shift+P")
|
||||
client.type_in_palette("save")
|
||||
results = client.get_palette_results()
|
||||
assert any("Save" in r.title for r in results)
|
||||
# Other commands not shown
|
||||
assert not any("Compress" in r.title for r in results)
|
||||
|
||||
def test_palette_executes_command_on_enter(live_gui):
|
||||
client = live_gui[1]
|
||||
client.press_key_combo("Ctrl+Shift+P")
|
||||
client.type_in_palette("Reset")
|
||||
client.press_key("Down")
|
||||
client.press_key("Enter")
|
||||
# Verify the reset command was executed (check via Hook API)
|
||||
state = client.get_session_state()
|
||||
assert state.get("discussion_history", []) == []
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- `Ctrl+Shift+P` opens the palette (verified via `live_gui` test)
|
||||
- Typing in the palette filters results via fuzzy match
|
||||
- Selecting a command (Enter key) executes it and closes the palette
|
||||
- Escape closes the palette without executing
|
||||
- All unit tests pass
|
||||
- All integration tests pass
|
||||
- The palette respects the existing theme (dark/light/nerv)
|
||||
- No new lint errors
|
||||
|
||||
---
|
||||
|
||||
## Risks
|
||||
|
||||
1. **Keyboard handler conflicts:** The Ctrl+Shift+P combo might be intercepted by other subsystems. Mitigation: check for other handlers in the codebase first; if conflicts, document them.
|
||||
2. **Pyodide build dependencies:** The image_bundle web backend (for Track 4) has a different architecture than this track. The two are independent but should be aware of each other.
|
||||
3. **Test flakiness:** `live_gui` tests can be flaky if the GUI doesn't initialize in time. Mitigation: the standard 15-second readiness polling is sufficient.
|
||||
+385
@@ -0,0 +1,385 @@
|
||||
# Comprehensive Documentation Refresh
|
||||
|
||||
**Date:** 2026-06-02
|
||||
**Status:** Draft (pending review)
|
||||
**Parent Track:** `documentation_refresh_comprehensive_20260602`
|
||||
**Sub-Tracks:** 3 (sequential with one parallel pair)
|
||||
|
||||
---
|
||||
|
||||
## 1. Context & Motivation
|
||||
|
||||
Manual Slop is a local GUI orchestrator for LLM-driven coding sessions. Since the last documentation refresh track (`documentation_refresh_20260224`, completed February 2026), the codebase has grown substantially:
|
||||
|
||||
- **New subsystems** without dedicated guides: Beads mode (Dolt-backed issue tracking), RAG (ChromaDB + multi-source retrieval), Hot Reload (state-preserving module reloading), Discussion Metrics & Compression, Command Palette, Structural File Editor (unified AST inspector + slice editor)
|
||||
- **New language support** via tree-sitter: C, C++ (already in docs), plus planned Lua, GDScript, C# (in backlog)
|
||||
- **Two new guide files** that exist but are not linked from the docs index: `docs/guide_context_curation.md`, `docs/guide_shaders_and_window.md`
|
||||
- **Drift in agent config files** (`AGENTS.md`, `CLAUDE.md`, `GEMINI.md`): the three files overlap on Session Startup, Conductor System, MMA tiers, and anti-patterns; no single source of truth
|
||||
- **CLAUDE.md is largely vestigial** — Claude Code is no longer the primary toolchain, but the file still receives content updates
|
||||
- **AGENTS.md contains rich guidance content** that should live in `mma-orchestrator/SKILL.md` or `conductor/workflow.md`, not as a surface-level orientation file
|
||||
|
||||
The user has explicitly stated two goals for this refresh:
|
||||
|
||||
1. **Human-facing docs (`./docs/`, READMEs)** must reach **textbook / Microsoft "purple-tomb" SDK documentation** fidelity. Every detail: state machines, public APIs, threading constraints, algorithms. No hand-waving, no "left to the reader." A human should be able to use the tool AND maintain/extend the codebase from the docs alone.
|
||||
2. **Agent-facing docs** (`AGENTS.md`, `GEMINI.md`, skill files in `.agents/`, `.gemini/`) must follow an **explicit divergence model** with no content duplication. `AGENTS.md` becomes a thin pointer; `CLAUDE.md` is deprecated; `GEMINI.md` stays Gemini-CLI-specific.
|
||||
|
||||
This refresh is **documentation-only** (no new code, no new tooling, no new scripts). Drift control after the refresh is manual via the track's verification step.
|
||||
|
||||
---
|
||||
|
||||
## 2. Scope Boundaries
|
||||
|
||||
### In Scope
|
||||
|
||||
- Top-level `Readme.md`
|
||||
- `docs/Readme.md` and all `docs/guide_*.md` files
|
||||
- `conductor/product.md`, `conductor/product-guidelines.md`, `conductor/tech-stack.md`, `conductor/workflow.md`, `conductor/index.md`
|
||||
- `AGENTS.md`, `CLAUDE.md`, `GEMINI.md`
|
||||
- Light verification pass (read each, confirm role accuracy, fix obvious drift; do NOT rewrite wholesale) on `.agents/skills/*/SKILL.md`, `mma-orchestrator/SKILL.md`, `.gemini/...` mirrors
|
||||
- Per-file atomic git commits with git notes attached as rationale trails
|
||||
|
||||
### Out of Scope
|
||||
|
||||
- Source code changes (a separate agent is making code modifications)
|
||||
- New lint/check scripts (drift control is manual)
|
||||
- New `docs/` files beyond what's required to document new subsystems
|
||||
- Re-architecting the conductor system itself
|
||||
- Removing CLAUDE.md (kept as a stub for compatibility)
|
||||
- Visual diagrams (ASCII is fine; no image generation)
|
||||
- Translating the docs to other languages
|
||||
|
||||
---
|
||||
|
||||
## 3. Track Architecture
|
||||
|
||||
The parent track `documentation_refresh_comprehensive_20260602` is a coordinator: it registers the three sub-tracks in `conductor/tracks.md`, sequences them, and produces a final verification report. Each sub-track is an independent conductor track with its own `spec.md`, `plan.md`, and `metadata.json`.
|
||||
|
||||
### Dependency Graph
|
||||
|
||||
```
|
||||
Sub-Track 1: docs_layer_refresh
|
||||
│
|
||||
├──> Sub-Track 2: conductor_docs_refresh
|
||||
│
|
||||
└──> Sub-Track 3: agent_config_refresh
|
||||
│
|
||||
└──> Parent verification
|
||||
```
|
||||
|
||||
Sub-Tracks 2 and 3 can run in parallel after Sub-Track 1 establishes the human-facing doc layer. The parent track does the final verification and creates the checkpoint commit.
|
||||
|
||||
### Execution Constraints (apply to all 3 sub-tracks)
|
||||
|
||||
- **No subagents.** Each sub-track is executed by a single agent (the user or the Tier 2 Tech Lead session) sequentially. No Tier 3 worker delegation, no Tier 4 QA delegation, no parallel worker execution.
|
||||
- **Per-file atomic commits.** One file = one commit. The git note attached to each commit captures the rationale and the source files cross-referenced during the rewrite.
|
||||
- **Pre-edit `git add .` checkpoint** before each file edit (protects against `git restore` mishaps).
|
||||
- **Style baseline:** `conductor/product-guidelines.md` — VEFontCache-Odin pattern (Philosophy → Architectural Boundaries → Implementation Logic → Verification), high information density, structural parity between doc symbols and source code symbols.
|
||||
- **Symbol parity:** Every class, function, method, and event named in the docs must match the source exactly. If the source uses `AsyncEventQueue`, the doc must use `AsyncEventQueue`, not "the event queue."
|
||||
- **Link validation:** Every `[text](./relative/path)` link is manually verified via `manual-slop_search_files` to confirm the target exists. Cross-doc links must point to files that exist in this or a future sub-track.
|
||||
- **No comments added to source code.** Documentation lives in `./docs/`, not inline.
|
||||
|
||||
---
|
||||
|
||||
## 4. Sub-Track 1: `docs_layer_refresh_20260602`
|
||||
|
||||
**Priority:** HIGHEST. This is the dominant work.
|
||||
|
||||
### File List
|
||||
|
||||
| File | Current Size | Notes |
|
||||
|---|---|---|
|
||||
| `Readme.md` | 15.5K | Main project README |
|
||||
| `docs/Readme.md` | ~3K | Documentation index |
|
||||
| `docs/guide_architecture.md` | 34.5K | Threading, events, AI client, HITL |
|
||||
| `docs/guide_mma.md` | unknown | 4-tier hierarchy, DAG, worker lifecycle |
|
||||
| `docs/guide_tools.md` | 22.9K | MCP tools, Hook API, shell runner |
|
||||
| `docs/guide_simulations.md` | unknown | live_gui fixture, Puppeteer, mock provider |
|
||||
| `docs/guide_context_curation.md` | 8K | Skeleton/curated/targeted views, fuzzy anchors |
|
||||
| `docs/guide_shaders_and_window.md` | 3.4K | Shader pipeline, custom window frame |
|
||||
| `docs/guide_meta_boundary.md` | 4.6K | Application vs Meta-Tooling domain |
|
||||
| `docs/MMA_Support/*` | varies | Legacy MMA reference docs — review for archival or merge |
|
||||
|
||||
### Tasks (Phases)
|
||||
|
||||
**Phase 1.1: Inventory and Gap Analysis**
|
||||
- Read each existing guide, summarize what subsystems it covers
|
||||
- Identify subsystems in `conductor/product.md` that have NO corresponding guide
|
||||
- Identify subsystems that DO have a guide but the guide is stale (e.g., does not cover features added since Feb 2026)
|
||||
- Produce a gap table: subsystem → current guide → needed changes (rewrite / add / link from index / remove)
|
||||
|
||||
**Phase 1.2: Update the Documentation Index**
|
||||
- Update `docs/Readme.md` so every `docs/guide_*.md` appears in the Guides table
|
||||
- Currently unlinked: `docs/guide_context_curation.md`, `docs/guide_shaders_and_window.md`
|
||||
- Decide fate of `docs/MMA_Support/*` (legacy archive or merge into `docs/guide_mma.md`)
|
||||
- Commit: `docs(index): link guide_context_curation and guide_shaders_and_window`
|
||||
|
||||
**Phase 1.3: Rewrite Each Guide (one file at a time, per-file atomic commits)**
|
||||
- For each guide, follow the per-file workflow:
|
||||
1. Read current source code (use `manual-slop_get_file_summary` and `manual-slop_py_get_skeleton`; do NOT read full files >50 lines)
|
||||
2. Read current `docs/MMA_Support/*` and `conductor/tracks/*/plan.md` for context on subsystems that have evolved
|
||||
3. Cross-reference `conductor/product.md` and `conductor/tech-stack.md` for the canonical feature/tech statement
|
||||
4. Identify what changed since the guide was last meaningfully updated
|
||||
5. Rewrite affected sections, preserving the VEFontCache-Odin style (Philosophy → Architectural Boundaries → Implementation Logic → Verification)
|
||||
6. Validate every internal `[link](./relative/path)` resolves
|
||||
7. Commit per-file with `docs(<scope>): <description>` message
|
||||
8. Attach git note with: source files cross-referenced, subsystems updated, any decisions made
|
||||
|
||||
**Phase 1.4: Update `Readme.md`**
|
||||
- Add a "Module by Domain" reference table that links to each guide for deep dives
|
||||
- Add a "Subsystem Index" section cross-referencing each major feature to its dedicated guide
|
||||
- Update the screenshot/gallery references if needed
|
||||
- Update setup instructions to reflect any new prerequisites (e.g., Beads CLI, Dolt)
|
||||
- Commit per-section: `docs(readme): add module-by-domain reference table`, `docs(readme): add subsystem index`, `docs(readme): update setup prerequisites`, etc.
|
||||
|
||||
**Phase 1.5: Subsystems Without Guides (write new guides as needed)**
|
||||
For each subsystem identified in 1.1 that lacks a guide, write a new `docs/guide_<name>.md` using the same per-file workflow. Likely candidates based on the gap analysis:
|
||||
- RAG (`docs/guide_rag.md`) — vector store, chunking, multi-provider search
|
||||
- Beads (`docs/guide_beads.md`) — Dolt integration, toolset, context compaction
|
||||
- Hot Reload (`docs/guide_hot_reload.md`) — `HotReloader` lifecycle, state preservation, UI delegation pattern
|
||||
- Discussion Metrics & Compression (`docs/guide_discussion_metrics.md`) — per-response token tracking, history compression strategy
|
||||
- Command Palette (`docs/guide_command_palette.md`) — async context preview, fuzzy command resolution
|
||||
- Structural File Editor (`docs/guide_structural_editor.md`) — unified AST inspector + slice editor
|
||||
- NERV Theme (`docs/guide_nerv_theme.md`) — black void palette, CRT-style effects, status flickering
|
||||
|
||||
Each new guide must follow the same VEFontCache-Odin style and link from `docs/Readme.md` (which means the index update in 1.2 must accommodate them).
|
||||
|
||||
**Phase 1.6: Verification**
|
||||
- `grep` for every internal link in every rewritten guide to confirm targets exist
|
||||
- `grep` for stale feature references (e.g., "MMA 4-tier" without the recent persona, RAG, Beads integrations mentioned in `conductor/product.md`)
|
||||
- Spot-check 3 random sections per guide against the corresponding source code
|
||||
- Manual user review (per workflow.md Phase Completion Verification protocol)
|
||||
|
||||
### Acceptance Criteria
|
||||
|
||||
- Every `docs/guide_*.md` is referenced from `docs/Readme.md`
|
||||
- Every internal cross-doc link resolves
|
||||
- Every subsystem in `conductor/product.md` has a corresponding guide (either existing or newly written)
|
||||
- Every public class, function, and event named in any guide matches the source code symbol exactly
|
||||
- Every guide follows the VEFontCache-Odin pattern (Philosophy → Architectural Boundaries → Implementation Logic → Verification) with all four sections present where applicable
|
||||
- "Textbook / purple-tomb fidelity" interpreted concretely: for every public class/function/event mentioned, the doc states its signature, threading constraints, side effects, and at least one usage example. Internal algorithms are explained step-by-step, not summarized. State machines include the full transition table
|
||||
- Per-file commits exist for every change, with git notes attached
|
||||
|
||||
---
|
||||
|
||||
## 5. Sub-Track 2: `conductor_docs_refresh_20260602`
|
||||
|
||||
**Priority:** Medium. Sync the source-of-truth conductor docs to current state.
|
||||
|
||||
### File List
|
||||
|
||||
| File | Current Size | Notes |
|
||||
|---|---|---|
|
||||
| `conductor/product.md` | 23.1K | Product vision, feature list, use cases |
|
||||
| `conductor/product-guidelines.md` | 6.6K | Style and process guidelines |
|
||||
| `conductor/tech-stack.md` | unknown | Technology stack constraints |
|
||||
| `conductor/workflow.md` | 25.2K | Task lifecycle, TDD protocol |
|
||||
| `conductor/index.md` | 334 bytes | Conductor directory index |
|
||||
|
||||
### Tasks (Phases)
|
||||
|
||||
**Phase 2.1: Sync `conductor/product.md`**
|
||||
- Cross-reference every feature claim in `product.md` against the actual codebase
|
||||
- Add features that exist in code but are missing from `product.md` (Beads, RAG, Hot Reload, Discussion Metrics/Compression, Command Palette, Structural File Editor, NERV theme, persona editor, workspace profiles, etc.)
|
||||
- Remove features that have been culled or replaced
|
||||
- Update the "Primary Use Cases" section to reflect current usage patterns
|
||||
- Commit per-section: `docs(product): sync MMA dashboard section`, `docs(product): add RAG feature`, etc.
|
||||
|
||||
**Phase 2.2: Sync `conductor/tech-stack.md`**
|
||||
- Verify every entry corresponds to an actual Python dep (check `pyproject.toml` / `requirements.txt`) or an actual src/ module
|
||||
- Add new dependencies: chromadb (RAG), dolt (Beads), any new ones
|
||||
- Add new src/ modules: rag_engine.py, beads_client.py, hot_reloader.py, history.py, workspace_manager.py, etc.
|
||||
- Commit per-section.
|
||||
|
||||
**Phase 2.3: Sync `conductor/workflow.md`**
|
||||
- Verify the TDD protocol is followed by recent tracks (spot-check 3 recent tracks)
|
||||
- Verify the Phase Completion Verification protocol is followed
|
||||
- Verify the checkpointing protocol is followed
|
||||
- Update any sections that have drifted from actual practice
|
||||
- Commit per-section.
|
||||
|
||||
**Phase 2.4: Sync `conductor/product-guidelines.md`**
|
||||
- Verify the AI-Optimized Python Style (1-space indent, no comments, type hints) is actually followed by recent code
|
||||
- Verify the "Modular Controller Pattern" and "UI Delegation Pattern" are followed in `src/app_controller.py` and `src/gui_2.py`
|
||||
- Verify the "Mandatory ImGui Verification" linter (`scripts/check_imgui_scopes.py`) is still the source of truth
|
||||
- Update any sections that have drifted
|
||||
- Commit per-section.
|
||||
|
||||
**Phase 2.5: Update `conductor/index.md`**
|
||||
- If any new top-level conductor docs were added (e.g., `conductor/code_styleguides/` is referenced but not verified), link them
|
||||
- Commit.
|
||||
|
||||
**Phase 2.6: Verification**
|
||||
- `grep` for references to features that no longer exist
|
||||
- `grep` for missing dependencies (every dep in `pyproject.toml` should appear in `tech-stack.md`)
|
||||
- Spot-check 3 random claims per file
|
||||
- Manual user review
|
||||
|
||||
### Acceptance Criteria
|
||||
|
||||
- Every feature in `conductor/product.md` corresponds to a real subsystem in `src/` or `tests/`
|
||||
- Every entry in `conductor/tech-stack.md` is an actual dep or module
|
||||
- `conductor/workflow.md` matches the actual workflow used by recent completed tracks
|
||||
- `conductor/product-guidelines.md` matches the actual coding style used in `src/`
|
||||
- `conductor/index.md` links to all relevant conductor files
|
||||
- Per-file commits with git notes
|
||||
|
||||
---
|
||||
|
||||
## 6. Sub-Track 3: `agent_config_refresh_20260602`
|
||||
|
||||
**Priority:** Lower. Apply the explicit divergence model.
|
||||
|
||||
### File List
|
||||
|
||||
| File | Current Size | Notes |
|
||||
|---|---|---|
|
||||
| `AGENTS.md` | 5.4K | Universal agent orientation — target: thin pointer |
|
||||
| `CLAUDE.md` | 6.7K | DEPRECATE to stub |
|
||||
| `GEMINI.md` | 3.8K | Keep Gemini-CLI-specific content |
|
||||
| `.agents/skills/mma-tier1-orchestrator/SKILL.md` | 2.4K | Light verification only |
|
||||
| `.agents/skills/mma-tier2-tech-lead/SKILL.md` | 3.8K | Light verification only |
|
||||
| `.agents/skills/mma-tier3-worker/SKILL.md` | unknown | Light verification only |
|
||||
| `.agents/skills/mma-tier4-qa/SKILL.md` | 813 bytes | Light verification only |
|
||||
| `.agents/skills/mma-orchestrator/SKILL.md` | 9.7K | Light verification only |
|
||||
| `mma-orchestrator/SKILL.md` (root copy) | 9.3K | Light verification only |
|
||||
| `.gemini/skills/*/SKILL.md` | varies | Light verification only (mirrors) |
|
||||
| `.gemini/agents/*.md` | varies | Light verification only (mirrors) |
|
||||
| `.claude/commands/*.md` | 9 files | Light verification only (legacy) |
|
||||
|
||||
### Tasks (Phases)
|
||||
|
||||
**Phase 3.1: Rewrite `AGENTS.md` to a thin pointer**
|
||||
|
||||
Target structure (estimated ~1K):
|
||||
|
||||
```markdown
|
||||
# AGENTS.md
|
||||
|
||||
## What This Is
|
||||
Manual Slop is a local GUI orchestrator for LLM-driven coding sessions. It bridges high-latency AI reasoning with a low-latency ImGui render loop via a thread-safe async pipeline; every AI-generated payload passes through a human-auditable gate before execution.
|
||||
|
||||
## Guidance for AI Agents
|
||||
Detailed agent guidance lives in the following locations — read these directly, do not duplicate content here:
|
||||
|
||||
- **Operational workflow:** `conductor/workflow.md`
|
||||
- **Code style and process:** `conductor/product-guidelines.md`
|
||||
- **Tech stack and constraints:** `conductor/tech-stack.md`
|
||||
- **Product context:** `conductor/product.md`
|
||||
- **MMA orchestrator role:** `mma-orchestrator/SKILL.md`
|
||||
- **Tier 1 (Orchestrator):** `.agents/skills/mma-tier1-orchestrator/SKILL.md`
|
||||
- **Tier 2 (Tech Lead):** `.agents/skills/mma-tier2-tech-lead/SKILL.md`
|
||||
- **Tier 3 (Worker):** `.agents/skills/mma-tier3-worker/SKILL.md`
|
||||
- **Tier 4 (QA):** `.agents/skills/mma-tier4-qa/SKILL.md`
|
||||
|
||||
## Human-Facing Documentation
|
||||
For understanding, using, and maintaining the tool, see `docs/Readme.md` and the guides it indexes.
|
||||
|
||||
## Critical Anti-Patterns
|
||||
- Do not read full files >50 lines without first using `py_get_skeleton` or `get_file_summary`
|
||||
- Do not modify the tech stack without updating `conductor/tech-stack.md` first
|
||||
- Do not implement code directly as Tier 2 — delegate to Tier 3 workers
|
||||
- Do not skip TDD — write failing tests before implementation
|
||||
- Do not batch commits — commit per-task for atomic rollback
|
||||
```
|
||||
|
||||
**Phase 3.2: Replace `CLAUDE.md` with a 1-line stub**
|
||||
|
||||
Target content:
|
||||
|
||||
```markdown
|
||||
# CLAUDE.md
|
||||
|
||||
This project is no longer actively used with Claude Code. For project context, see `AGENTS.md`. The conductor system in `./conductor/` is the cross-tool abstraction and works with any agent toolchain.
|
||||
```
|
||||
|
||||
(Keep the file as a stub to avoid breaking any external tooling that may still reference it.)
|
||||
|
||||
**Phase 3.3: Lightly refresh `GEMINI.md`**
|
||||
|
||||
- Verify it covers only Gemini-CLI-specific content (no shared content that should live in `AGENTS.md`)
|
||||
- Update any stale references to features, models, or paths
|
||||
- If any content was previously shared with `AGENTS.md` and has been relocated, update `GEMINI.md` to point to `AGENTS.md` instead
|
||||
- Commit: `docs(gemini): refresh Gemini CLI orientation`
|
||||
|
||||
**Phase 3.4: Verify `.agents/skills/*/SKILL.md` and `.gemini/...` mirrors for coherence**
|
||||
|
||||
For each skill file:
|
||||
- Read it; confirm it covers the role accurately
|
||||
- Compare with the canonical role description in `mma-orchestrator/SKILL.md`
|
||||
- If drift is found, fix it (small surgical edits)
|
||||
- If the file is outdated, flag for a separate track — do not rewrite wholesale in this track
|
||||
- Commit per file: `docs(skills): fix drift in tier1 orchestrator skill`
|
||||
|
||||
**Phase 3.5: Verify `.claude/commands/*.md` (legacy)**
|
||||
|
||||
- These exist for backward compatibility
|
||||
- Lightly verify they still work and don't reference removed features
|
||||
- Do not rewrite — they are legacy
|
||||
- If a file is broken, flag for separate track
|
||||
|
||||
**Phase 3.6: Verification**
|
||||
- `grep` for the section headers that should have been relocated (Session Startup, Conductor System, MMA 4-Tier, Anti-Patterns) across `AGENTS.md`, `CLAUDE.md`, `GEMINI.md`
|
||||
- Confirm each appears in at most one file (or in multiple files only as short pointers)
|
||||
- Spot-check 2 random skills files against their canonical description
|
||||
- Manual user review
|
||||
|
||||
### Acceptance Criteria
|
||||
|
||||
- `AGENTS.md` is ≤ 1.5K and contains only project orientation + pointers
|
||||
- `CLAUDE.md` is ≤ 200 bytes and contains only the deprecation stub
|
||||
- `GEMINI.md` contains only Gemini-CLI-specific content
|
||||
- No section header appears in 2+ of these files unless it's a short pointer (≤3 lines)
|
||||
- `.agents/skills/*/SKILL.md` and `.gemini/...` mirrors are verified for coherence
|
||||
- `.claude/commands/*.md` legacy files are verified to not reference removed features
|
||||
- Per-file commits with git notes
|
||||
|
||||
---
|
||||
|
||||
## 7. Parent Track: Final Verification
|
||||
|
||||
After all 3 sub-tracks complete, the parent track produces a final verification report:
|
||||
|
||||
1. **Cross-track link audit:** For every markdown file modified in any sub-track, run `grep` for `[text](./relative/path)` patterns and verify each target exists
|
||||
2. **Symbol parity audit:** Spot-check 5 random symbols named in any doc against the source — confirm exact match
|
||||
3. **Drift audit:** `grep` for the relocated section headers (`Session Startup`, `Conductor System`, `MMA 4-Tier`, `Anti-Patterns`, etc.) across `AGENTS.md`, `CLAUDE.md`, `GEMINI.md` — confirm explicit divergence holds
|
||||
4. **Subsystem coverage audit:** For every feature in `conductor/product.md`, confirm a corresponding guide exists in `docs/`
|
||||
5. **User review gate:** Present the final verification report and request user sign-off per workflow.md Phase Completion Verification protocol
|
||||
|
||||
The parent track then creates a single checkpoint commit: `conductor(checkpoint): Comprehensive documentation refresh complete` and attaches the verification report as a git note.
|
||||
|
||||
---
|
||||
|
||||
## 8. Risks & Mitigations
|
||||
|
||||
| Risk | Mitigation |
|
||||
|---|---|
|
||||
| A source code change by the other agent makes a doc claim stale mid-refresh | Per-file atomic commits + pre-edit `git add .` checkpoint; if drift is found during a later file's rewrite, fix the earlier file in a follow-up commit |
|
||||
| Scope creep — adding new guides balloons the track | The gap analysis in 1.1 is the gating step; new guides are written only for subsystems explicitly listed in `conductor/product.md` |
|
||||
| The 3 sub-tracks drift in style consistency | Style baseline is `conductor/product-guidelines.md` (single source); reviewed by user per sub-track |
|
||||
| Manual link validation misses broken links | Per-file commit immediately after each rewrite; a broken link is caught in the next file's cross-reference work |
|
||||
| User reviews take long enough that the codebase evolves underneath | Sub-tracks are independent; each can resume from its last commit if interrupted |
|
||||
| The legacy `CLAUDE.md` deprecation breaks external tooling | The file is kept as a 1-line stub, not removed; any external tooling that reads it will see a graceful pointer to `AGENTS.md` |
|
||||
|
||||
---
|
||||
|
||||
## 9. Out of Scope (Reminder)
|
||||
|
||||
- Source code changes
|
||||
- New lint scripts (drift control is manual)
|
||||
- Image/diagram generation
|
||||
- Translations
|
||||
- Removing CLAUDE.md
|
||||
- Visual layout of the docs site (Manual Slop is a GUI app, not a docs site)
|
||||
|
||||
---
|
||||
|
||||
## 10. Success Criteria
|
||||
|
||||
The track is complete when:
|
||||
|
||||
1. All 3 sub-tracks have completed all their phases with per-file atomic commits and git notes
|
||||
2. The parent track's verification report shows zero broken cross-doc links, zero symbol mismatches in spot-checks, and confirmed explicit divergence in agent config files
|
||||
3. Every subsystem in `conductor/product.md` has a corresponding guide
|
||||
4. `Readme.md`, `docs/Readme.md`, and every `docs/guide_*.md` is at textbook / purple-tomb fidelity (verified by user review)
|
||||
5. `AGENTS.md` is a thin pointer document; `CLAUDE.md` is a deprecation stub
|
||||
6. The user has signed off on the final verification report per workflow.md
|
||||
@@ -0,0 +1,288 @@
|
||||
# Docker Container & Web-Hosted ImGui Frontend
|
||||
|
||||
**Date:** 2026-06-02
|
||||
**Status:** Draft (pending review)
|
||||
**Reference:** https://imgui-bundle.pages.dev/explorer/ — imgui-bundle web backend via Hello ImGui
|
||||
|
||||
---
|
||||
|
||||
## Context & Motivation
|
||||
|
||||
The user wants to deploy Manual Slop on Unraid (a home server OS) and access the GUI via a web browser. The goal is for agents to operate on projects hosted on the home server, with the user monitoring/controlling via web browser.
|
||||
|
||||
Current state:
|
||||
- `sloppy.py` is a desktop GUI (ImGui via imgui-bundle + Python)
|
||||
- `src/api_hooks.py` provides a FastAPI/Uvicorn headless service on `:8999` for external automation
|
||||
- The app is Windows-oriented (PowerShell subprocesses, `pywin32` for window frame)
|
||||
|
||||
Target state:
|
||||
- Docker container with the full app
|
||||
- Web browser shows the ImGui GUI in real-time
|
||||
- Agents can interact via the existing Hook API on `:8999`
|
||||
- The user's Unraid server can host multiple project directories
|
||||
|
||||
imgui-bundle's web backend ([reference](https://imgui-bundle.pages.dev/explorer/)) uses a server-side render with a client-side WebGL display. The Hello ImGui runner pairs a Python render loop with a JavaScript WebGL canvas via WebSocket.
|
||||
|
||||
---
|
||||
|
||||
## Scope
|
||||
|
||||
### In Scope
|
||||
|
||||
- `Dockerfile` — Container build for the Manual Slop app
|
||||
- `docker-compose.yml` — Multi-container deployment for Unraid
|
||||
- `scripts/docker_build.sh` — Build helper
|
||||
- `scripts/docker_run.sh` — Run helper with env var wiring
|
||||
- `docs/guide_docker_deployment.md` — Unraid setup guide
|
||||
- `tests/test_docker_build.py` — Opt-in Docker build test
|
||||
|
||||
### Out of Scope
|
||||
|
||||
- Migrating `sloppy.py` to use the imgui-bundle web backend (the web backend is an alternative to the desktop backend; switching is a significant refactor and may be deferred)
|
||||
- Multi-user authentication (single-user deployment)
|
||||
- Cloud-specific deployment (AWS, GCP) — Unraid is the target
|
||||
- TLS termination (assumed handled by a reverse proxy like Traefik or Caddy)
|
||||
|
||||
---
|
||||
|
||||
## Design
|
||||
|
||||
### Architecture: V2 — Server-side Python + WebGL client (via WebSocket)
|
||||
|
||||
```
|
||||
┌─────────────────────────────────────────────┐
|
||||
│ Docker Container (unraid:manual_slop:latest) │
|
||||
│ │
|
||||
│ ┌────────────────────────────────────┐ │
|
||||
│ │ Python app │ │
|
||||
│ │ - ImGui renders to framebuffer │ │
|
||||
│ │ - Hello ImGui web backend: │ │
|
||||
│ │ - Python: render loop │ │
|
||||
│ │ - WebSocket: frame deltas │ │
|
||||
│ │ - HTTP: serves JS client │ │
|
||||
│ │ - HookServer on :8999 │ │
|
||||
│ └────────────────────────────────────┘ │
|
||||
│ │
|
||||
│ Exposed ports: │
|
||||
│ - 8080: Web client (HTTP + WS) │
|
||||
│ - 8999: Hook API │
|
||||
│ │
|
||||
│ Volumes: │
|
||||
│ - /projects: project workspaces │
|
||||
│ - /config: app state, presets, personas │
|
||||
└─────────────────────────────────────────────┘
|
||||
↑ ↑
|
||||
│ Browser (Chrome, Firefox) │ Agent (curl, scripts)
|
||||
│ WebSocket for live frames │ HTTP for state
|
||||
```
|
||||
|
||||
### Dockerfile
|
||||
|
||||
```dockerfile
|
||||
FROM python:3.11-slim
|
||||
|
||||
# System deps
|
||||
RUN apt-get update && apt-get install -y --no-install-recommends \
|
||||
git curl ca-certificates \
|
||||
&& rm -rf /var/lib/apt/lists/*
|
||||
|
||||
# Install uv
|
||||
RUN pip install uv
|
||||
|
||||
# App setup
|
||||
WORKDIR /app
|
||||
COPY pyproject.toml uv.lock ./
|
||||
RUN uv sync --frozen
|
||||
|
||||
COPY . .
|
||||
|
||||
# Volumes
|
||||
RUN mkdir -p /projects /config
|
||||
VOLUME ["/projects", "/config"]
|
||||
|
||||
# Expose
|
||||
EXPOSE 8080 8999
|
||||
|
||||
# Health check
|
||||
HEALTHCHECK --interval=30s --timeout=5s --start-period=30s --retries=3 \
|
||||
CMD curl -f http://127.0.0.1:8999/status || exit 1
|
||||
|
||||
# Entrypoint
|
||||
ENTRYPOINT ["uv", "run", "sloppy.py", "--enable-test-hooks", "--web-host=0.0.0.0", "--web-port=8080"]
|
||||
```
|
||||
|
||||
### `docker-compose.yml`
|
||||
|
||||
```yaml
|
||||
version: '3.8'
|
||||
|
||||
services:
|
||||
manual_slop:
|
||||
build: .
|
||||
image: manual_slop:latest
|
||||
container_name: manual_slop
|
||||
ports:
|
||||
- "8999:8999" # Hook API (host)
|
||||
- "8080:8080" # Web client (host)
|
||||
volumes:
|
||||
- /mnt/user/projects:/projects:rw # Unraid project share
|
||||
- /mnt/user/appdata/manual_slop:/config:rw # App state
|
||||
environment:
|
||||
- GEMINI_API_KEY=${GEMINI_API_KEY}
|
||||
- ANTHROPIC_API_KEY=${ANTHROPIC_API_KEY}
|
||||
- DEEPSEEK_API_KEY=${DEEPSEEK_API_KEY}
|
||||
- MINIMAX_API_KEY=${MINIMAX_API_KEY}
|
||||
restart: unless-stopped
|
||||
healthcheck:
|
||||
test: ["CMD", "curl", "-f", "http://127.0.0.1:8999/status"]
|
||||
interval: 30s
|
||||
timeout: 5s
|
||||
retries: 3
|
||||
start_period: 30s
|
||||
```
|
||||
|
||||
### Entry Point Changes (`sloppy.py`)
|
||||
|
||||
The current `sloppy.py` launches the desktop GUI. For web mode, we need:
|
||||
|
||||
```python
|
||||
# In sloppy.py
|
||||
import argparse
|
||||
|
||||
parser = argparse.ArgumentParser()
|
||||
# ... existing args ...
|
||||
parser.add_argument("--web-host", default=None, help="Enable web mode and bind to this host")
|
||||
parser.add_argument("--web-port", type=int, default=8080, help="Web mode port")
|
||||
args = parser.parse_args()
|
||||
|
||||
if args.web_host is not None:
|
||||
from imgui_bundle import hello_imgui
|
||||
runner_params = hello_imgui.RunnerParams()
|
||||
runner_params.app_window_params.borderless = False
|
||||
runner_params.imgui_window_params.default_imgui_window_type = ... # web backend
|
||||
hello_imgui.run(runner_params)
|
||||
else:
|
||||
# Existing desktop launch
|
||||
...
|
||||
```
|
||||
|
||||
The imgui-bundle web backend is selected by the Hello ImGui runner. The exact config is per the [imgui-bundle explorer docs](https://imgui-bundle.pages.dev/explorer/).
|
||||
|
||||
### `docs/guide_docker_deployment.md`
|
||||
|
||||
A complete Unraid setup guide:
|
||||
|
||||
- Prerequisites (Unraid version, Docker template)
|
||||
- Building the image
|
||||
- Configuring volumes and env vars
|
||||
- Accessing the web client (URL, browser requirements)
|
||||
- Agent interaction examples (curl, Python script)
|
||||
- Backup and restore of /config
|
||||
- Updating the image
|
||||
|
||||
### `tests/test_docker_build.py`
|
||||
|
||||
```python
|
||||
import os
|
||||
import subprocess
|
||||
import time
|
||||
|
||||
import pytest
|
||||
import requests
|
||||
|
||||
IMAGE_NAME = "manual_slop:test"
|
||||
CONTAINER_NAME = "manual_slop_test"
|
||||
WEB_PORT = 8080
|
||||
HOOK_PORT = 8999
|
||||
|
||||
|
||||
@pytest.mark.docker
|
||||
def test_docker_container_starts_and_serves(tmp_path):
|
||||
"""Build the Docker image, run the container, verify web client + hook API."""
|
||||
if os.environ.get("RUN_DOCKER_TEST") != "1":
|
||||
pytest.skip("Set RUN_DOCKER_TEST=1 to enable")
|
||||
|
||||
if not _docker_available():
|
||||
pytest.skip("Docker not available")
|
||||
|
||||
# Build
|
||||
result = subprocess.run(
|
||||
["docker", "build", "-t", IMAGE_NAME, "."],
|
||||
capture_output=True, text=True, timeout=300,
|
||||
)
|
||||
assert result.returncode == 0, f"Docker build failed: {result.stderr}"
|
||||
|
||||
# Run
|
||||
subprocess.run(["docker", "rm", "-f", CONTAINER_NAME], capture_output=True)
|
||||
result = subprocess.run([
|
||||
"docker", "run", "-d",
|
||||
"--name", CONTAINER_NAME,
|
||||
"-p", f"{WEB_PORT}:8080",
|
||||
"-p", f"{HOOK_PORT}:8999",
|
||||
IMAGE_NAME,
|
||||
], capture_output=True, text=True, timeout=30)
|
||||
assert result.returncode == 0, f"Docker run failed: {result.stderr}"
|
||||
|
||||
try:
|
||||
# Wait for hook API
|
||||
start = time.time()
|
||||
ready = False
|
||||
while time.time() - start < 60:
|
||||
try:
|
||||
r = requests.get(f"http://127.0.0.1:{HOOK_PORT}/status", timeout=1)
|
||||
if r.status_code == 200:
|
||||
ready = True
|
||||
break
|
||||
except (requests.ConnectionError, requests.Timeout):
|
||||
pass
|
||||
time.sleep(1)
|
||||
|
||||
assert ready, "Container did not start hook API within 60s"
|
||||
|
||||
# Verify web client is served
|
||||
r = requests.get(f"http://127.0.0.1:{WEB_PORT}/", timeout=5)
|
||||
assert r.status_code == 200
|
||||
assert b"<html" in r.content.lower() or b"<!doctype" in r.content.lower()
|
||||
|
||||
finally:
|
||||
subprocess.run(["docker", "rm", "-f", CONTAINER_NAME], capture_output=True)
|
||||
|
||||
|
||||
def _docker_available() -> bool:
|
||||
result = subprocess.run(["docker", "version"], capture_output=True)
|
||||
return result.returncode == 0
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## File Structure
|
||||
|
||||
- `Dockerfile` — NEW
|
||||
- `docker-compose.yml` — NEW
|
||||
- `scripts/docker_build.sh` — NEW
|
||||
- `scripts/docker_run.sh` — NEW
|
||||
- `docs/guide_docker_deployment.md` — NEW
|
||||
- `tests/test_docker_build.py` — NEW
|
||||
- `sloppy.py` — MODIFY: add `--web-host` and `--web-port` args
|
||||
|
||||
---
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- `docker build -t manual_slop:latest .` succeeds on a clean machine
|
||||
- `docker compose up` starts the container, and `:8999/status` returns 200 within 60s
|
||||
- `curl http://localhost:8080/` returns the web client HTML
|
||||
- An agent can `curl http://localhost:8999/api/gui/mma_status` and get a valid response
|
||||
- The user can navigate to the web UI in a browser and see the ImGui panels
|
||||
- File operations on `/projects` persist across container restarts
|
||||
- Env vars for API keys are not committed to the image (use runtime env)
|
||||
|
||||
---
|
||||
|
||||
## Risks
|
||||
|
||||
1. **imgui-bundle web backend maturity:** The web backend is less battle-tested than the desktop backend. There may be rendering quirks, input latency, or unsupported features. Mitigation: this is experimental; expect to iterate.
|
||||
2. **Headless rendering in container:** Some ImGui features (e.g., font hinting) may need extra config for headless rendering. Mitigation: test early in development; fall back to Xvfb + noVNC if web backend is too immature.
|
||||
3. **WebSocket bandwidth:** Streaming frame deltas requires consistent network. On flaky networks, the user experience degrades. Mitigation: implement client-side prediction or reduce frame rate.
|
||||
4. **Container size:** Python + uv + all deps can produce a 1-2GB image. Mitigation: use multi-stage builds; pin Python deps for reproducibility.
|
||||
5. **Unraid-specific quirks:** Unraid uses a specific Docker storage driver and may have path mapping edge cases. Mitigation: test on the actual Unraid deployment; document the path mapping clearly.
|
||||
@@ -0,0 +1,220 @@
|
||||
# Test Consolidation & TOML Sandboxing Enforcement
|
||||
|
||||
**Date:** 2026-06-02
|
||||
**Status:** Draft (pending review)
|
||||
|
||||
---
|
||||
|
||||
## Context & Motivation
|
||||
|
||||
The Manual Slop test suite has grown to ~258 test files. Many tests read or write project TOML files (manual_slop.toml, config.toml, credentials.toml, presets.toml, etc.) for fixtures. The pattern is inconsistent:
|
||||
|
||||
- Some tests use `tmp_path` + `monkeypatch` (good — isolated)
|
||||
- Some tests use real `./` paths (bad — pollutes user config)
|
||||
- Some tests use mock paths at module level (good — fast)
|
||||
|
||||
The user wants to:
|
||||
1. Audit tests for real-TOML usage
|
||||
2. Migrate offenders to sandboxed variants
|
||||
3. Consolidate similar tests where it improves clarity
|
||||
4. Enforce the rule going forward
|
||||
|
||||
The `isolate_workspace` autouse fixture in `tests/conftest.py` (added in the May 2026 docs refresh work) is the foundation for the migration pattern.
|
||||
|
||||
---
|
||||
|
||||
## Scope
|
||||
|
||||
### In Scope
|
||||
|
||||
- Audit all `tests/*.py` for direct path references to `./` TOML files
|
||||
- Migrate offenders to use `tmp_path` + `monkeypatch` (or `isolate_workspace`)
|
||||
- Consolidate similar tests where it improves clarity (judgment call)
|
||||
- Add a `tests/conftest.py` autouse fixture that prevents regression
|
||||
- Add a `scripts/check_test_toml_paths.py` script for CI/pre-commit
|
||||
- Add tests for the enforcement mechanism itself
|
||||
|
||||
### Out of Scope
|
||||
|
||||
- Rewriting tests for clarity (only consolidation where it improves maintainability)
|
||||
- Adding new tests
|
||||
- Changing the test runner (pytest stays)
|
||||
- Coverage tooling changes
|
||||
|
||||
---
|
||||
|
||||
## Design
|
||||
|
||||
### Phase 1: Audit
|
||||
|
||||
A script that greps `tests/*.py` for problematic patterns:
|
||||
|
||||
```python
|
||||
# scripts/check_test_toml_paths.py
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
PROBLEMATIC_PATTERNS = [
|
||||
r'Path\("(?:manual_slop|config|credentials|presets|personas|tool_presets|workspace_profiles)\.toml"\)',
|
||||
r'open\(["\'](?:manual_slop|config|credentials|presets|personas|tool_presets|workspace_profiles)\.toml["\']',
|
||||
r'["\']\.{1,2}/(?:manual_slop|config|credentials|presets|personas|tool_presets|workspace_profiles)\.toml["\']',
|
||||
]
|
||||
|
||||
def find_violations(tests_dir: Path) -> List[Tuple[Path, int, str]]:
|
||||
"""Returns list of (file, line, pattern) for each violation."""
|
||||
...
|
||||
```
|
||||
|
||||
Run this script as the first step. Output a report grouped by file.
|
||||
|
||||
### Phase 2: Migrate Offenders
|
||||
|
||||
For each violation, refactor the test to use the sandboxed pattern:
|
||||
|
||||
**Before (real TOML):**
|
||||
```python
|
||||
def test_load_presets():
|
||||
path = Path("presets.toml") # Real file!
|
||||
if path.exists():
|
||||
data = tomllib.loads(path.read_text())
|
||||
assert data is not None
|
||||
```
|
||||
|
||||
**After (sandboxed):**
|
||||
```python
|
||||
def test_load_presets(tmp_path):
|
||||
path = tmp_path / "presets.toml"
|
||||
path.write_text("[presets.test]\nkey = 'value'\n")
|
||||
# Patch the path module to point to tmp_path
|
||||
monkeypatch = pytest.MonkeyPatch()
|
||||
monkeypatch.setattr("src.paths.get_global_presets_path", lambda: path)
|
||||
data = tomllib.loads(path.read_text())
|
||||
assert data["presets"]["test"]["key"] == "value"
|
||||
monkeypatch.undo()
|
||||
```
|
||||
|
||||
Or use the `isolate_workspace` autouse fixture (already in conftest.py) which redirects all path resolution to `tmp_path`.
|
||||
|
||||
### Phase 3: Consolidate (Judgment Call)
|
||||
|
||||
Examples of consolidation opportunities (NOT a forced refactor):
|
||||
|
||||
| Current | Proposed | Rationale |
|
||||
|---|---|---|
|
||||
| `test_ai_settings_layout.py` + `test_sim_ai_settings.py` | `test_ai_settings.py` with parametrize | Tests cover same surface |
|
||||
| `test_*_provider.py` (5+ files) | `test_providers.py` parametrized | Each provider test has same shape |
|
||||
| `test_*_preset*.py` (3 files) | `test_presets.py` with class organization | Settings/presets/tools all CRUD TOML |
|
||||
| `test_*_screenshot*.py` | `test_screenshots.py` | Currently fragmented |
|
||||
|
||||
Each consolidation is reviewed case-by-case. **Test count is not a goal; test clarity is.** Don't merge tests that test different things just to reduce file count.
|
||||
|
||||
### Phase 4: Enforce
|
||||
|
||||
**4a. Autouse fixture** in `tests/conftest.py`:
|
||||
|
||||
```python
|
||||
@pytest.fixture(autouse=True)
|
||||
def enforce_no_real_toml(monkeypatch, tmp_path):
|
||||
"""Prevents any test from reading ./<name>.toml by detecting file existence
|
||||
and asserting the path is inside tmp_path or explicitly monkeypatched."""
|
||||
|
||||
real_toml_paths = [
|
||||
Path("manual_slop.toml"),
|
||||
Path("config.toml"),
|
||||
Path("credentials.toml"),
|
||||
Path("presets.toml"),
|
||||
Path("personas.toml"),
|
||||
Path("tool_presets.toml"),
|
||||
Path("workspace_profiles.toml"),
|
||||
]
|
||||
|
||||
# If any real TOML exists in the cwd, save it for restoration
|
||||
snapshots = {}
|
||||
for p in real_toml_paths:
|
||||
if p.exists():
|
||||
snapshots[p] = p.read_bytes()
|
||||
p.unlink() # Remove to prevent test from reading
|
||||
yield # Run the test
|
||||
# Restore after test
|
||||
for p, content in snapshots.items():
|
||||
p.write_bytes(content)
|
||||
```
|
||||
|
||||
This is **strict** — any test that tries to read a real TOML will get FileNotFoundError. Tests must use `tmp_path` or `monkeypatch`.
|
||||
|
||||
If this is too aggressive, a softer alternative:
|
||||
|
||||
```python
|
||||
@pytest.fixture(autouse=True)
|
||||
def warn_on_real_toml():
|
||||
"""Warns if a test reads a real TOML. Does not fail by default;
|
||||
set ENFORCE_NO_REAL_TOML=1 to convert warnings to failures."""
|
||||
...
|
||||
```
|
||||
|
||||
**4b. CI script** `scripts/check_test_toml_paths.py` — runs on every commit:
|
||||
|
||||
```python
|
||||
# Greps for direct ./<name>.toml references
|
||||
# Exits non-zero if any found
|
||||
# Output: "test_foo.py:42: Path('presets.toml') — direct reference to real TOML"
|
||||
```
|
||||
|
||||
Add to `conductor/...` workflow or as a pre-commit hook (out of scope for this track — just provide the script).
|
||||
|
||||
### Phase 5: Test the Enforcer
|
||||
|
||||
`tests/test_enforce_no_real_toml.py` — meta-test:
|
||||
|
||||
```python
|
||||
def test_enforcer_catches_violation(tmp_path, monkeypatch):
|
||||
"""Verify the fixture prevents reading a real TOML."""
|
||||
# Create a real-looking TOML in cwd
|
||||
real_path = Path("test_enforcer_temp.toml")
|
||||
real_path.write_text("[test]\nkey='value'")
|
||||
try:
|
||||
# The fixture removes it; try to read it
|
||||
with pytest.raises(FileNotFoundError):
|
||||
real_path.read_text()
|
||||
finally:
|
||||
if real_path.exists():
|
||||
real_path.unlink()
|
||||
|
||||
def test_enforcer_restores_real_tomls(tmp_path):
|
||||
"""Verify the fixture restores real TOMLs after the test."""
|
||||
real_path = Path("test_enforcer_temp2.toml")
|
||||
original = b"[test]\nkey='original'"
|
||||
real_path.write_bytes(original)
|
||||
# The test runs (fixture activates)
|
||||
assert real_path.exists() # The fixture restored it
|
||||
assert real_path.read_bytes() == original
|
||||
real_path.unlink()
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## File Structure
|
||||
|
||||
- `scripts/check_test_toml_paths.py` — NEW: greps for violations, exits non-zero
|
||||
- `tests/conftest.py` — MODIFY: add `enforce_no_real_toml` autouse fixture (strict or warn-only)
|
||||
- `tests/test_enforce_no_real_toml.py` — NEW: tests for the enforcer
|
||||
- Various `tests/test_*.py` — MODIFY: migrate offenders to sandboxed pattern
|
||||
- Various `tests/test_*.py` — MODIFY: consolidate where it improves clarity
|
||||
|
||||
---
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- All existing tests pass after migration
|
||||
- `scripts/check_test_toml_paths.py` exits 0 on the test suite after migration
|
||||
- The autouse fixture catches new violations in CI
|
||||
- Test count is approximately the same after consolidation (slight decrease acceptable)
|
||||
- No real TOML files in the user's project are touched by the test suite
|
||||
|
||||
---
|
||||
|
||||
## Risks
|
||||
|
||||
1. **Test breakage:** Migration may break tests that depend on real-file behavior. Mitigation: run full test suite after each migration batch.
|
||||
2. **Performance:** The autouse fixture adds overhead to every test. Mitigation: keep it cheap (just snapshot/restore file existence).
|
||||
3. **Coverage regression:** Removing real-file behavior may hide bugs. Mitigation: add explicit tests for the sandboxed path resolution.
|
||||
@@ -0,0 +1,277 @@
|
||||
# UI Polish Track — Design Spec
|
||||
|
||||
**Date:** 2026-06-03
|
||||
**Status:** Approved for implementation
|
||||
**Author:** Tier 2 Tech Lead
|
||||
**Scope:** Five discrete UI quality-of-life fixes identified during user review of the GUI.
|
||||
**Related tracks:** `gui_2_cleanup_20260513`, `selectable_ui_text_20260308`, `context_comp_decouple_20260510`.
|
||||
|
||||
---
|
||||
|
||||
## 1. Problem Statement
|
||||
|
||||
User review surfaced five independent UI defects / quality issues that have been outstanding for some time and that previous agent attempts have not been able to fully resolve. This spec decomposes them into five phases of a single track. Each phase is independently shippable and produces visible improvement.
|
||||
|
||||
| # | Severity | Issue | Prior attempts |
|
||||
|---|----------|-------|----------------|
|
||||
| 1 | High | Markdown tables in `imgui_md` are rendered as run-on text — column boundaries lost, headers indistinguishable from body rows. | Multiple agents reported attempted fixes; the underlying limitation is that `imgui-bundle`'s `imgui_md` does not implement GFM tables. |
|
||||
| 2 | High | The `Keep Pairs` numeric input next to `Truncate` in the discussion panel is clipped to 80 px — single-digit values are visibly truncated. | One-line width fix. |
|
||||
| 3 | High | The `Refresh Registry` button in Log Management instantiates a new `LogRegistry` but does not call `.load_registry()`, so the displayed table never reflects on-disk state. | One-line fix. |
|
||||
| 4 | Medium | Operations Hub has no per-vendor session state view (quota, context-window usage, last-error class). Existing "Usage Analytics" tab shows historical aggregates only. | New panel. |
|
||||
| 5 | Medium | Files & Media > Files shows a flat sorted table; the user wants directory-grouped collapsible tree nodes matching the Context Composition visual style. | Refactor mirroring `render_context_files_table` pattern. |
|
||||
|
||||
---
|
||||
|
||||
## 2. Architecture Overview
|
||||
|
||||
All five phases target the rendering layer only. No new infrastructure, no threading changes, no provider API changes. The track stays inside the ImGui immediate-mode model and the existing `markdown_helper.py` / `aggregate.py` / `log_registry.py` modules.
|
||||
|
||||
### 2.1 Phase Boundaries
|
||||
|
||||
```
|
||||
Phase 1 ─ Markdown table pre-processor
|
||||
Phase 2 ─ Discussion Truncate / Keep Pairs layout fix
|
||||
Phase 3 ─ Log Management refresh bug fix
|
||||
Phase 4 ─ Operations Hub > Vendor State panel
|
||||
Phase 5 ─ Files & Media > Files directory tree
|
||||
```
|
||||
|
||||
Each phase is one track-level task with its own Red/Green/Commit cycle. Phase 1 is the only one with non-trivial complexity (it introduces a new sub-module). Phases 2, 3, 5 are surgical. Phase 4 adds a new sub-component.
|
||||
|
||||
### 2.2 Shared Conventions
|
||||
|
||||
- All Python code uses 1-space indentation (per `conductor/code_styleguides/python.md`).
|
||||
- No new comments in source files (per `AGENTS.md`).
|
||||
- All new render functions live at module level in `src/gui_2.py` and follow the `(app: App) -> None` signature, with a thin `_render_vendor_state(self)` wrapper on `App` if they need to be hot-reload-able.
|
||||
- SDM dependency tags required on all new public functions.
|
||||
- Tests live in `tests/test_<module>.py` and follow the existing `live_gui` fixture pattern when they need real GUI state.
|
||||
- Branch coverage target: ≥ 80 % for new code; full regression for the touched panel.
|
||||
|
||||
---
|
||||
|
||||
## 3. Per-Phase Design
|
||||
|
||||
### Phase 1 — Markdown Table Pre-Processor
|
||||
|
||||
**Problem:** `imgui-bundle`'s `imgui_md` (vendored as `src/imgui_md`) does not implement GFM table syntax. Lines like:
|
||||
|
||||
```
|
||||
| Name | Type |
|
||||
|-------|------|
|
||||
| foo | int |
|
||||
```
|
||||
|
||||
render as a single run of `|` characters with no column alignment, no header/body distinction, and no border. The user has reported this as a recurring frustration for months.
|
||||
|
||||
**Why previous fixes failed:** Attempts to monkey-patch `imgui_md` or to convert tables to ASCII pre-formatted blocks lose interactivity (links inside cells, syntax highlighting of code cells) and look bad in high-density themes like NERV.
|
||||
|
||||
**Approach:** Insert a GFM-table interceptor into `MarkdownRenderer.render()` that runs *before* `imgui_md.render()`. The interceptor:
|
||||
|
||||
1. Detects table blocks via the GFM signature: a line of `|`-delimited cells followed by a separator line containing only `|`, `-`, `:`, and spaces.
|
||||
2. Computes the natural width of each column by measuring rendered text (using `imgui.calc_text_size`) — this is what makes it "data-oriented" rather than string-length-based.
|
||||
3. Renders the table via `imgui.begin_table()` with one column per logical column, headers via `imgui.table_headers_row()`, and body rows via `imgui.table_next_row()` / `imgui.table_set_column_index()`.
|
||||
4. Recursively delegates any markdown *inside* cell content (links, emphasis, inline code) to `imgui_md.render()` per cell.
|
||||
5. Falls back to the existing `imgui_md.render()` pass if a block is detected as table-shaped but fails sanity checks (e.g. zero columns, separator with no body).
|
||||
|
||||
**Module boundary:** New module `src/markdown_table.py` exports a single function `render_markdown_tables(text: str) -> str` that returns the text with tables replaced by an inert placeholder (`\x00TABLE_<idx>\x00`), plus a list of table specs. The actual rendering stays inside `MarkdownRenderer` because it needs an ImGui context.
|
||||
|
||||
**Files touched:**
|
||||
- New: `src/markdown_table.py` — pure parser, no ImGui imports.
|
||||
- Modify: `src/markdown_helper.py` — `MarkdownRenderer.render` intercepts and dispatches.
|
||||
- New tests: `tests/test_markdown_table.py` (parser unit tests), `tests/test_markdown_table_render.py` (live_gui render tests).
|
||||
|
||||
**Safety:** The interceptor only activates when a block matches the GFM table shape; everything else passes through unchanged. The placeholder scheme is robust to nested `imgui_md` quirks because we substitute placeholder *after* `imgui_md` has finished its pass.
|
||||
|
||||
**Acceptance criteria:**
|
||||
- A representative 4-column, 3-row table renders with aligned borders, bold header, and proper column widths.
|
||||
- Tables inside code fences (```` ``` ````) are NOT touched.
|
||||
- Inline code / links inside cells still render correctly.
|
||||
- Performance: rendering a 5-table, 50-row document is < 16 ms (one frame budget).
|
||||
- Existing markdown tests still pass.
|
||||
|
||||
---
|
||||
|
||||
### Phase 2 — Truncate / Keep Pairs Input Width
|
||||
|
||||
**Problem:** `src/gui_2.py:3829`:
|
||||
|
||||
```python
|
||||
imgui.text("Keep Pairs:"); imgui.same_line(); imgui.set_next_item_width(80)
|
||||
ch, app.ui_disc_truncate_pairs = imgui.input_int("##trunc_pairs", app.ui_disc_truncate_pairs, 1)
|
||||
```
|
||||
|
||||
The width of 80 px is too narrow for a 2–3 digit number. Image evidence: the digit `2` is partially cut off on the right border.
|
||||
|
||||
**Fix:** Increase `set_next_item_width` from 80 to 140 (matches the width of the adjacent `Truncate` button + spacing). Additionally, switch from `imgui.input_int` to `imgui.drag_int` for the same field — this gives the user the `+/-` stepper buttons inline without needing the `same_line` button pair, and prevents the digit-clipping behavior of `input_int` when the value approaches the field width.
|
||||
|
||||
**Files touched:**
|
||||
- Modify: `src/gui_2.py` — single line at 3829 plus the surrounding `same_line()` chain.
|
||||
|
||||
**Acceptance criteria:**
|
||||
- 3-digit values (`999`) render fully inside the input.
|
||||
- The `Truncate` button remains clickable and aligned on the same row.
|
||||
- `ui_disc_truncate_pairs` still floors at 1.
|
||||
|
||||
---
|
||||
|
||||
### Phase 3 — Log Management `Refresh Registry` Bug
|
||||
|
||||
**Problem:** `src/gui_2.py:1675`:
|
||||
|
||||
```python
|
||||
if imgui.button("Refresh Registry"):
|
||||
app._log_registry = log_registry.LogRegistry(str(paths.get_logs_dir() / "log_registry.toml"))
|
||||
```
|
||||
|
||||
The new `LogRegistry` instance is constructed (which opens the file) but `.load_registry()` is never called, so `app._log_registry.data` stays empty. The user sees the same stale table.
|
||||
|
||||
**Fix:** Two acceptable shapes:
|
||||
|
||||
**A — In-place reload (preferred):**
|
||||
```python
|
||||
if imgui.button("Refresh Registry"):
|
||||
if app._log_registry is not None:
|
||||
app._log_registry.load_registry()
|
||||
```
|
||||
|
||||
**B — Re-instantiate + load (defensive):**
|
||||
```python
|
||||
if imgui.button("Refresh Registry"):
|
||||
app._log_registry = log_registry.LogRegistry(str(paths.get_logs_dir() / "log_registry.toml"))
|
||||
app._log_registry.load_registry()
|
||||
```
|
||||
|
||||
We pick **A** because it preserves any in-memory state (e.g. a pending `update_session_metadata` call) and is what the user likely meant. We add `load_registry` to `LogRegistry`'s public API (it already exists privately at lines 75-103 — we just stop reading the wrong attribute).
|
||||
|
||||
**Files touched:**
|
||||
- Modify: `src/gui_2.py` — single line at 1675.
|
||||
- New test: `tests/test_log_management_refresh.py` — drives a temp registry, calls the button via live_gui or via direct function call, asserts table count increases.
|
||||
|
||||
**Acceptance criteria:**
|
||||
- Pressing the button updates the visible table when the on-disk TOML has changed.
|
||||
- No regression to existing log_management tests.
|
||||
|
||||
---
|
||||
|
||||
### Phase 4 — Operations Hub > Vendor State Panel
|
||||
|
||||
**Problem:** The current Operations Hub has tabs for Comms / Tool Calls / Usage Analytics / External Tools / Workspace Layouts. There is no at-a-glance view of "what is the current vendor's session state?" — things like:
|
||||
- Current provider + model.
|
||||
- Context-window utilization (used / limit, percentage bar).
|
||||
- Cache hit rate for the active session.
|
||||
- Last error class (if any).
|
||||
- Quota state (when the vendor exposes it; Anthropic and Gemini CLI expose different signals).
|
||||
|
||||
**Approach:** Add a new tab `Vendor State` between `Usage Analytics` and `External Tools`. The tab shows a single high-density `imgui.begin_table()` with one row per tracked metric. The metrics are pulled from a new module-level helper `get_vendor_state(app: App) -> list[VendorMetric]` that aggregates from:
|
||||
- `app.current_provider`, `app.current_model` (from `models.PROVIDERS`).
|
||||
- `app.controller.token_tracker` for used / limit / cache stats.
|
||||
- `app.controller.last_error` for error class.
|
||||
- A new `app.controller.vendor_quota` (lazy-loaded dict) populated by `ai_client` when the provider returns a quota-bearing response.
|
||||
|
||||
**Component shape:**
|
||||
```python
|
||||
@dataclass(frozen=True)
|
||||
class VendorMetric:
|
||||
key: str # e.g. "context_window"
|
||||
label: str # e.g. "Context Window"
|
||||
value: str # e.g. "78,234 / 200,000 (39%)"
|
||||
state: str # "ok" | "warn" | "error" | "info"
|
||||
tooltip: str # long-form explanation, shown on hover
|
||||
```
|
||||
|
||||
The new tab is a thin renderer:
|
||||
```python
|
||||
def render_vendor_state(app: App) -> None:
|
||||
metrics = get_vendor_state(app)
|
||||
if imgui.begin_table("vendor_state", 3, imgui.TableFlags_.row_bg | imgui.TableFlags_.borders):
|
||||
imgui.table_setup_column("Metric", imgui.TableColumnFlags_.width_fixed, 180)
|
||||
imgui.table_setup_column("Value", imgui.TableColumnFlags_.width_stretch)
|
||||
imgui.table_setup_column("State", imgui.TableColumnFlags_.width_fixed, 60)
|
||||
imgui.table_headers_row()
|
||||
for m in metrics:
|
||||
...
|
||||
if imgui.is_item_hovered(): imgui.set_tooltip(m.tooltip)
|
||||
```
|
||||
|
||||
**Files touched:**
|
||||
- New: `src/vendor_state.py` — pure aggregator, no ImGui imports.
|
||||
- Modify: `src/gui_2.py` — add `render_vendor_state`, new tab in `render_operations_hub`.
|
||||
- Modify: `src/app_controller.py` — add `vendor_quota: dict[str, Any] = field(default_factory=dict)` and a `set_vendor_quota(provider, payload)` callback.
|
||||
- Modify: `src/ai_client.py` — call `set_vendor_quota` on quota-bearing responses (Anthropic `usage` blocks, Gemini `metadata.tokenInfo`, DeepSeek rate-limit headers).
|
||||
- New tests: `tests/test_vendor_state.py` (aggregator logic), `tests/test_vendor_state_render.py` (live_gui rendering).
|
||||
|
||||
**Acceptance criteria:**
|
||||
- Tab appears and is the default-open tab when Operations Hub is opened.
|
||||
- All four metric categories (provider/model, context window, cache, last error, quota) render with stable keys.
|
||||
- Missing data renders as `—` (em dash), not as a crash.
|
||||
- No regression in `live_gui` test suite.
|
||||
|
||||
---
|
||||
|
||||
### Phase 5 — Files & Media > Files Directory Tree
|
||||
|
||||
**Problem:** `src/gui_2.py:2689` `render_files_and_media` renders `app.files` as a flat 3-column table (Act / Path / Status), sorted alphabetically. This is hard to scan for a user with 50+ files. The user wants the directory-grouped, collapsible tree node style used in `render_context_files_table` (lines 3111-3260).
|
||||
|
||||
**Approach:** Reuse the existing `aggregate.group_files_by_dir()` helper. For each directory key returned, render a `tree_node_ex(..., default_open)` with the directory name as the label, then iterate the files under it as the leaf rows. The 3-column layout (Act / Path / Status) is preserved at the leaf level.
|
||||
|
||||
**Component shape:**
|
||||
```python
|
||||
def render_files_and_media(app: App) -> None:
|
||||
if imgui.collapsing_header("Files", imgui.TreeNodeFlags_.default_open):
|
||||
with imscope.group():
|
||||
grouped = aggregate.group_files_by_dir(app.files)
|
||||
for dir_name, g_files in sorted(grouped.items()):
|
||||
with imscope.tree_node_ex(f"{dir_name}##files_dir", imgui.TreeNodeFlags_.default_open) as is_open:
|
||||
if is_open:
|
||||
# ... existing per-file row logic, with all `i` indices scoped to the directory
|
||||
# ... existing "Add Files to Inventory" button
|
||||
```
|
||||
|
||||
**Files touched:**
|
||||
- Modify: `src/gui_2.py:2689-2750` — wrap the inner per-file loop in a directory group loop.
|
||||
- New test: `tests/test_files_and_media_tree.py` — asserts that two files in different directories render with the directory labels visible, and that `tree_node` open/closed state is preserved across frames.
|
||||
|
||||
**Acceptance criteria:**
|
||||
- Two files in the same directory are grouped under one collapsible node.
|
||||
- One-file "directories" still render (no special-case).
|
||||
- The Act / Path / Status columns still function (Add, Remove, Active / Cached / disabled).
|
||||
- No regression in `tests/test_gui_fast_render.py::test_render_files_and_media_fast`.
|
||||
|
||||
---
|
||||
|
||||
## 4. Cross-Cutting Risks & Mitigations
|
||||
|
||||
| Risk | Mitigation |
|
||||
|------|------------|
|
||||
| Phase 1 markdown change regresses existing markdown rendering (communications, log previews, discussion responses). | All `live_gui` tests for those panels must pass before merging Phase 1. Add a snapshot test that hashes a fixed multi-table markdown input. |
|
||||
| Phase 4 `vendor_quota` thread-safety — providers fire callbacks on background threads. | The new `set_vendor_quota` is a pure field write under the controller's existing lock; `get_vendor_state` reads under the same lock. Document in SDM tag. |
|
||||
| Phase 5 changes the row identity for the `+` and `x` buttons — indices must be globally unique across all directories, not per-directory. | The current code uses `i = enumerate(app.files)`; we preserve the original global index for button IDs by computing it via `app.files.index(f_item)` once per iteration. |
|
||||
| ImGui scope mismatches introduced by the new `tree_node_ex` blocks. | Run `scripts/check_imgui_scopes.py` after each phase. |
|
||||
| Plan exceeds one track's worth of work; some phases might be deferred. | Each phase is independently shippable and self-contained; we will checkpoint after each phase rather than at the end. |
|
||||
|
||||
---
|
||||
|
||||
## 5. Testing Strategy
|
||||
|
||||
- **Unit tests** for every new module (`markdown_table`, `vendor_state`) — pure parser/aggregator logic, no ImGui context needed.
|
||||
- **live_gui tests** for any change that touches `gui_2.py` render functions. Use the session-scoped `live_gui` fixture from `tests/conftest.py`.
|
||||
- **Regression** — full targeted batch of `tests/test_gui_fast_render.py`, `tests/test_log_management_ui.py`, `tests/test_markdown_*.py`, `tests/test_discussion_hub_*.py` after each phase.
|
||||
- **No new `unittest.mock.patch`** on core infrastructure (per the Structural Testing Contract).
|
||||
|
||||
---
|
||||
|
||||
## 6. Rollout
|
||||
|
||||
Phases are checkpointed independently:
|
||||
|
||||
```
|
||||
Phase 1 ─ checkpoint ─ Phase 2 ─ checkpoint ─ Phase 3 ─ checkpoint ─ Phase 4 ─ checkpoint ─ Phase 5 ─ final
|
||||
```
|
||||
|
||||
Each checkpoint:
|
||||
1. Run targeted test batch.
|
||||
2. Spawn live_gui in headless mode and confirm a smoke screenshot of the touched panel.
|
||||
3. Attach a git note with the verification report.
|
||||
4. Update `conductor/tracks.md` with the phase SHA.
|
||||
|
||||
If a phase is approved out-of-order, the others remain independently executable. There are no inter-phase dependencies (Phase 1, 2, 3, 5 do not depend on Phase 4; Phase 4 does not depend on any of them).
|
||||
@@ -0,0 +1,104 @@
|
||||
# Theme & Syntax Highlighting Modularization
|
||||
|
||||
## Problem
|
||||
|
||||
The current theming system in `src/theme_2.py` has three limitations:
|
||||
|
||||
1. **Themes are hardcoded as a Python dict.** Users cannot author new themes without editing Python source and recompiling. This is inconsistent with the rest of the project (presets, personas, tool_presets, context_presets, bias profiles, workspace profiles all use TOML).
|
||||
|
||||
2. **Syntax highlighting is hardcoded.** The `MarkdownRenderer._lang_map` in `src/markdown_helper.py` uses `imgui-bundle`'s `imgui_color_text_edit` language definitions whose token colors are baked into the C++ library. There is no way to align syntax token colors with the active UI theme.
|
||||
|
||||
3. **No way to bundle new themes with a release or share them between projects.**
|
||||
|
||||
## Goals
|
||||
|
||||
- **TOML-based theme authoring.** Themes live in `themes/<name>.toml` (global) and `<project>/project_themes.toml` (project override). Schema mirrors the existing `_PALETTES` dict shape.
|
||||
|
||||
- **Authoring without recompiling.** Drop a new `.toml` file in `themes/` and it appears in the palette selector after the next load (or hot-reload, future).
|
||||
|
||||
- **Syntax palette mapping.** Each theme TOML declares a `syntax_palette` field that maps to one of the four built-in `imgui_color_text_edit` palettes (`dark`, `light`, `mariana`, `retro_blue`). The renderer calls `editor.set_default_palette(...)` whenever the active theme changes.
|
||||
|
||||
- **Scope-based merging** matches the existing pattern: project themes override global themes with the same name.
|
||||
|
||||
## Constraints
|
||||
|
||||
- `imgui-bundle` only ships 4 built-in syntax palettes and exposes no API to define new ones or override individual token colors. This is a hard upstream limit. The plan accepts the limit and works around it via palette mapping.
|
||||
|
||||
- We do NOT attempt to wrap or shadow `imgui_color_text_edit`. The C++ library owns the per-language token regexes and default token colors. We pick the closest of the 4 palettes for each theme and let users override the mapping per theme.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Defining new `imgui_color_text_edit` palettes or overriding token colors per language (blocked by upstream API).
|
||||
- Hot-reload of theme changes (the user can re-apply from the selector).
|
||||
- Per-language color customization (e.g., Python `keyword` color distinct from C `keyword`).
|
||||
|
||||
## File structure
|
||||
|
||||
| File | Action | Responsibility |
|
||||
|---|---|---|
|
||||
| `src/theme_2.py` | Modify | Replace hardcoded `_PALETTES` dict with a load-from-TOML pipeline. Keep `apply()` public API. Expose new helpers `get_syntax_palette_for_theme(name)` and `apply_syntax_palette(palette_id)`. |
|
||||
| `src/paths.py` | Modify | Add `get_global_themes_path()` and `get_project_themes_path(project_root)`. Defaults: `themes.toml` (global) and `project_themes.toml` (project). Override via `SLOP_GLOBAL_THEMES` env var. |
|
||||
| `src/theme_models.py` | Create | Pydantic/dataclass schema for theme TOML files. `ThemePalette` has all `imgui.Col_` keys, `syntax_palette` is a string (one of the 4 IDs). `to_dict()` / `from_dict()` round-trip. |
|
||||
| `themes/solarized_dark.toml` | Create | Authoring artifact. RGB triples in standard `#RRGGBB` form. |
|
||||
| `themes/solarized_light.toml` | Create | Same. |
|
||||
| `themes/gruvbox_dark.toml` | Create | Same. |
|
||||
| `themes/moss.toml` | Create | Same. |
|
||||
| `tests/test_theme_models.py` | Create | Round-trip tests for `ThemePalette` from/to TOML. |
|
||||
| `tests/test_theme.py` | Modify | Add tests for the 4 new palettes, TOML loading, scope merge, and syntax palette mapping. |
|
||||
| `tests/fixtures/themes/minimal.toml` | Create | Minimal valid TOML fixture for loader tests. |
|
||||
| `tests/fixtures/themes/missing_keys.toml` | Create | TOML missing required keys — should raise a clear error. |
|
||||
| `docs/guide_themes.md` | Create | Authoring guide: schema, file locations, scope rules, syntax palette mapping, env vars. |
|
||||
|
||||
## Theme TOML schema (reference, not implementation in this plan)
|
||||
|
||||
```toml
|
||||
# theme name (informational)
|
||||
name = "Solarized Dark"
|
||||
|
||||
# optional: which built-in imgui_color_text_edit palette to use
|
||||
# one of: dark | light | mariana | retro_blue
|
||||
syntax_palette = "dark"
|
||||
|
||||
# which imgui style colors this theme overrides
|
||||
# any key not listed falls back to the base imgui dark/light defaults
|
||||
[colors]
|
||||
window_bg = [ 0, 43, 54] # 0x002b36 base03
|
||||
child_bg = [ 7, 54, 66] # 0x073642 base02
|
||||
text = [147, 161, 161] # 0x93a1a1 base1
|
||||
text_disabled = [ 88, 110, 117] # 0x586e75 base01
|
||||
button_hovered = [ 38, 139, 210] # 0x268bd2 blue
|
||||
check_mark = [ 38, 139, 210]
|
||||
slider_grab = [ 38, 139, 210]
|
||||
tab_selected = [ 88, 110, 117]
|
||||
tab_hovered = [ 38, 139, 210]
|
||||
# ... remaining colors omitted
|
||||
```
|
||||
|
||||
Values are 3-element RGB arrays (0-255) for the body and the syntax palette is a string identifier.
|
||||
|
||||
## Syntax palette mapping (built-in only)
|
||||
|
||||
| Theme | Syntax palette |
|
||||
|---|---|
|
||||
| Solarized Dark | `dark` (closest dark base) |
|
||||
| Solarized Light | `light` |
|
||||
| Gruvbox Dark | `retro_blue` (warm retro feel) |
|
||||
| Moss | `mariana` (deep blue-green base) |
|
||||
| 10x Dark | `dark` |
|
||||
| Nord Dark | `dark` |
|
||||
| Monokai | `dark` |
|
||||
| Binks | `light` |
|
||||
| ImGui Dark | `dark` |
|
||||
| NERV | `dark` (NERV's own custom palette via `theme_nerv.apply_nerv()`) |
|
||||
|
||||
The mapping lives in `src/theme_2.py` as a small dict and is overridable per theme via the TOML `syntax_palette` field.
|
||||
|
||||
## Public API
|
||||
|
||||
Existing `src.theme_2` callsites must continue to work. New surface:
|
||||
|
||||
- `theme.get_palette_names() -> list[str]` — already exists, now also returns TOML-loaded themes
|
||||
- `theme.apply(name) -> None` — already exists, applies the named theme (built-in OR TOML)
|
||||
- `theme.get_syntax_palette_for_theme(name) -> PaletteId` — new
|
||||
- `theme.apply_syntax_palette(palette_id) -> None` — new, calls `editor.set_default_palette(palette_id)`
|
||||
- `theme.load_themes_from_disk() -> None` — new, public for hot-reload
|
||||
@@ -0,0 +1,251 @@
|
||||
# Live-GUI Fragility Fixes — Design
|
||||
|
||||
**Date:** 2026-06-05
|
||||
**Status:** Draft
|
||||
**Track follow-up to:** regression_fixes_20260605
|
||||
**Scope:** Fix 3 failing live_gui tests discovered in the 2026-06-05 batched test run, harden the defer-not-catch pattern doc, restore 100% pass rate on the 272-file test suite.
|
||||
|
||||
## 1. Background
|
||||
|
||||
### Scope decisions (per user review 2026-06-05)
|
||||
- Change 1 (the `b""` → `""` fix): **in scope, critical path.**
|
||||
- Change 2 (test mock fix for prior session test): **SCOPE REDUCED during execution.** The test was more under-mocked than the spec assumed. Initial error at `src/gui_2.py:2333` (imscope.window tuple unpack) was the first of several un-mocked dependencies. After fixing imscope.window, the next failure surfaces at `src/gui_2.py:4496` (render_theme_panel: imgui.begin returning bool where 2-tuple expected). The test calls `render_main_interface` which is a kitchen-sink function requiring 50+ mocks. **Decision: defer Change 2 to a separate follow-up track** that focuses on refactoring the test to either (a) exercise a narrow prior-session render path instead of `render_main_interface`, or (b) add the missing 50+ mocks. The imscope.window fix is still applied as a defensive change (and as a model for future test work).
|
||||
- Change 3 (regression unit test): **in scope, critical path.**
|
||||
- Change 4 (doc hardening of defer-not-catch sections): **DEFERRED to end of track** — user wants to see how long the critical path takes first. If time permits at the end, do Change 4 as a final commit; otherwise leave for a follow-up patch.
|
||||
|
||||
### Revised pass-rate target
|
||||
- Before track: 269/272 (98.9%)
|
||||
- After Change 1: 271/272 (99.6%) — both `test_auto_switch_sim` and `test_workspace_profiles_restoration` should pass; `test_prior_session_no_pop_imbalance` is deferred to a follow-up.
|
||||
- After Change 3: 272/272 if Change 2 also fixed, else 271/272 + new regression unit test passes.
|
||||
|
||||
### Follow-up track: prior_session_test_harden_20260605
|
||||
A new track to be queued in `conductor/tracks.md` covering the `test_prior_session_no_pop_imbalance` test's comprehensive mock setup (or refactor to test a narrow path).
|
||||
|
||||
### Failures (3)
|
||||
|
||||
| Test | File | Symptom | Root cause |
|
||||
|---|---|---|---|
|
||||
| `test_auto_switch_sim` | `tests/test_auto_switch_sim.py:47` | `assert False == True` after triggering tier-3 auto-switch | Category A: profile save raises TypeError → no profile saved → load is no-op |
|
||||
| `test_workspace_profiles_restoration` | `tests/test_workspace_profiles_sim.py:81` | `assert False is True` after `load_workspace_profile` | Category A: same as above |
|
||||
| `test_no_extraneous_pop_when_prior_session_renders` | `tests/test_prior_session_no_pop_imbalance.py:135` | `TypeError: cannot unpack non-iterable NoneType object` at `src/gui_2.py:2333` | Category B: test mock setup for `imscope.window` returns non-iterable, but production code expects `(opened, visible)` tuple |
|
||||
|
||||
### Test run results (2026-06-05, batched via `scripts/run_tests_batched.py`)
|
||||
|
||||
- **272 test files, 68 batches, 269/272 passing (98.9%).**
|
||||
- 3 failing tests, all in `live_gui` (session-scoped fixture) or `integration` marker category.
|
||||
- 0 failing tests in any other category (unit, headless, mock_app, simulation).
|
||||
|
||||
### Root cause analysis (Category A — both profile failures)
|
||||
|
||||
A regression introduced by commit `d7487af4` ("fix(gui_2): defer save_ini_settings on first capture to avoid early-render crash"). That commit added a defer-not-catch guard in `_capture_workspace_profile` (`src/gui_2.py:601-606`):
|
||||
|
||||
```python
|
||||
def _capture_workspace_profile(self, name: str) -> models.WorkspaceProfile:
|
||||
if not getattr(self, "_ini_capture_ready", False):
|
||||
self._ini_capture_ready = True
|
||||
ini = b"" # <-- BUG: bytes, not str
|
||||
else:
|
||||
try:
|
||||
ini = imgui.save_ini_settings_to_memory() # returns str
|
||||
except Exception:
|
||||
ini = b"" # <-- BUG: same
|
||||
...
|
||||
```
|
||||
|
||||
The bug: `ini = b""` is a `bytes` literal, but the `WorkspaceProfile` dataclass declares `ini_content: str` (`src/models.py:799`), AND `tomli_w` (the TOML serializer) raises `TypeError: Object of type 'bytes' is not TOML serializable`.
|
||||
|
||||
Verified empirically:
|
||||
```python
|
||||
>>> import tomli_w
|
||||
>>> tomli_w.dump({"ini_content": b""}, io.BytesIO())
|
||||
TypeError: Object of type 'bytes' is not TOML serializable
|
||||
```
|
||||
|
||||
Trace path for the failure:
|
||||
1. Test: `set_value('ui_separate_tier1', True)` → field is `True` in app state.
|
||||
2. Test: `push_event("custom_callback", {"callback": "save_workspace_profile", ...})`.
|
||||
3. GUI: `_process_pending_gui_tasks` → `_cb_save_workspace_profile` (`src/app_controller.py:2870`).
|
||||
4. App: `_capture_workspace_profile(name)` → returns `WorkspaceProfile(..., ini_content=b"", ...)`.
|
||||
5. `workspace_manager.save_profile(profile)` → `profile.to_dict()` → `{"ini_content": b"", ...}`.
|
||||
6. `_save_file` → `tomli_w.dump(data, f)` → **TypeError raised**.
|
||||
7. Exception propagates; profile is **NOT saved to disk**; `workspace_profiles` is **NOT reloaded**; `self._app.workspace_profiles` is **NOT updated**.
|
||||
8. Test: `set_value('ui_separate_tier1', False)` → field is `False`.
|
||||
9. Test: `push_event("custom_callback", {"callback": "load_workspace_profile", ...})`.
|
||||
10. App: `_cb_load_workspace_profile(name)` → `if name in self.workspace_profiles:` → `False` (save failed) → **does nothing**.
|
||||
11. Test: `assert get_value('ui_separate_tier1') is True` → **fails** (still `False`).
|
||||
|
||||
The original pre-defer code (`ini = imgui.save_ini_settings_to_memory()`) returned a `str` that round-tripped through TOML successfully; tests passed. The defer fix introduced a type-incompatible sentinel value that broke the serialization contract.
|
||||
|
||||
The 1-line fix: change `ini = b""` to `ini = ""` (and add a defensive str-coerce for the non-defer path).
|
||||
|
||||
### Root cause analysis (Category B — prior session test)
|
||||
|
||||
The test mocks `imscope.window(...)` to return a `MagicMock()` whose `__enter__` returns the bare mock. Production code at `src/gui_2.py:2333` does `with imscope.window(...) as (opened, visible):` which expects a 2-tuple. The test's setup (lines ~70-80) sets `__enter__` for many imscope context managers to return non-iterable `MagicMock()` but for `popup_modal` (line ~91) correctly returns `(True, None)`. The `imscope.window` setup is missing the tuple-return — purely a test-authoring bug.
|
||||
|
||||
## 2. Goals
|
||||
|
||||
1. **Restore 100% pass rate on the 272-file test suite** (no regressions in any other test).
|
||||
2. **Preserve the defer-not-catch safety property** of commit `d7487af4` (avoid C-level crash on early-render C calls).
|
||||
3. **Harden the defer-not-catch documentation** to call out the str/bytes type contract (avoid future regressions of the same kind).
|
||||
4. **Tighten the test-authoring contract** for the prior session test: mock imscope context managers with the correct return shape.
|
||||
5. **OPTIONAL/DEFERRED:** Harden the defer-not-catch pattern doc with a "sentinel must match consumer type contract" note. Per user review (2026-06-05), this is deferred to the end of the track. If time permits, do it; otherwise leave for a follow-up patch.
|
||||
|
||||
## 3. Non-Goals
|
||||
|
||||
- Not refactoring the workspace profile save/load architecture.
|
||||
- Not adding wait-for-ready semantics to the test framework (deferred to a separate live_gui harden track; tracked as backlog item 0 in `conductor/tracks.md`).
|
||||
- Not fixing the broader test fragility / session-state issues (deferred).
|
||||
- Not addressing `sloppy.py` startup latency (separate track, also backlog).
|
||||
|
||||
## 4. Design
|
||||
|
||||
### Change 1: Fix `ini = b""` → `ini = ""` in `_capture_workspace_profile`
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/gui_2.py:601-606` (the defer branch)
|
||||
- Modify: `src/gui_2.py:606-609` (the non-defer branch's `except` handler)
|
||||
|
||||
**Approach:** Change `ini = b""` to `ini = ""` in both places. The pre-fix code returned a `str`; we're restoring that contract. Additionally, defensively coerce the non-defer result: `ini = imgui.save_ini_settings_to_memory()` returns a `str` per `imgui-bundle` docs, but to be safe against future imgui-bundle changes, wrap it: `ini = str(imgui.save_ini_settings_to_memory() or "")`.
|
||||
|
||||
```python
|
||||
def _capture_workspace_profile(self, name: str) -> models.WorkspaceProfile:
|
||||
if not getattr(self, "_ini_capture_ready", False):
|
||||
self._ini_capture_ready = True
|
||||
ini = ""
|
||||
else:
|
||||
try:
|
||||
ini = str(imgui.save_ini_settings_to_memory() or "")
|
||||
except Exception:
|
||||
ini = ""
|
||||
panel_states = { ... }
|
||||
return models.WorkspaceProfile(...)
|
||||
```
|
||||
|
||||
**Why:** `WorkspaceProfile.ini_content: str` (`src/models.py:799`); `tomli_w` rejects `bytes`. `imgui.load_ini_settings_from_memory(ini_data: str, ...)` also expects `str`. Restoring the `str` contract is the minimal fix.
|
||||
|
||||
**Alternatives considered:**
|
||||
- A2 — Use `imgui.save_ini_settings_to_disk(path)` then read the file. **Rejected**: adds a side-effect path that's not idempotent; tests can pollute the test artifacts dir.
|
||||
- A3 — Force a frame render in `__init__` so the first call is safe. **Rejected**: changes init semantics; interacts badly with hot-reload (`src/hot_reloader.py`); may regress startup latency (the very thing the new sloppy.py startup track is meant to address).
|
||||
|
||||
### Change 2: Fix the prior session test mock
|
||||
|
||||
**Files:**
|
||||
- Modify: `tests/test_prior_session_no_pop_imbalance.py` (the imscope.window mock setup)
|
||||
|
||||
**Approach:** Add the tuple-return to `imscope.window`'s `__enter__` mock, matching the pattern already used for `popup_modal` at line 91:
|
||||
|
||||
```python
|
||||
mock_imscope.window.return_value.__enter__ = MagicMock(return_value=(True, True))
|
||||
mock_imscope.window.return_value.__exit__ = MagicMock(side_effect=_scope_exit)
|
||||
```
|
||||
|
||||
**Why:** The test's `imscope.window` setup is the only one missing the tuple-return; all other imscope context managers that production code expects to unpack as tuples already have it. This is a 2-line test-only fix.
|
||||
|
||||
### Change 3: Add a regression test for the ini_content type contract
|
||||
|
||||
**Files:**
|
||||
- Create: `tests/test_workspace_profile_serialization.py`
|
||||
|
||||
**Approach:** Add a unit test that verifies a `WorkspaceProfile` with `ini_content=""` (empty str) round-trips through TOML via `to_dict` → `tomli_w.dump` → `tomllib.load` → `from_dict` without raising. This is the contract that the defer fix violated.
|
||||
|
||||
```python
|
||||
def test_workspace_profile_empty_ini_content_roundtrips():
|
||||
from src.models import WorkspaceProfile
|
||||
profile = WorkspaceProfile(name="t", ini_content="", show_windows={"A": True}, panel_states={"x": 1})
|
||||
d = profile.to_dict()
|
||||
import io, tomli_w, tomllib
|
||||
buf = io.BytesIO()
|
||||
tomli_w.dump({profile.name: d}, buf) # this is what save_profile does
|
||||
buf.seek(0)
|
||||
back = tomllib.load(buf)
|
||||
loaded = WorkspaceProfile.from_dict("t", back["t"])
|
||||
assert loaded.ini_content == ""
|
||||
assert loaded.show_windows == {"A": True}
|
||||
assert loaded.panel_states == {"x": 1}
|
||||
```
|
||||
|
||||
**Why:** This test would have caught the `d7487af4` regression. It encodes the type contract for future contributors. It's a pure unit test, no live_gui, runs in <1s.
|
||||
|
||||
### Change 4: Harden the defer-not-catch doc
|
||||
|
||||
**Files:**
|
||||
- Modify: `docs/guide_gui_2.md` "Workspace Profile Defer-Not-Catch" section
|
||||
- Modify: `docs/guide_testing.md` "Early-Render C-Level Crashes" section
|
||||
- Modify: `conductor/workflow.md` "Defer-Not-Catch Pattern for Native Crashes" section
|
||||
|
||||
**Approach:** Add a note: "When implementing a defer-not-catch guard for a return value, **ensure the sentinel value matches the type contract of the downstream consumer**. For `WorkspaceProfile.ini_content: str`, the sentinel must be `""` (str), not `b""` (bytes) — TOML serialization rejects bytes."
|
||||
|
||||
**Why:** Future contributors applying the defer-not-catch pattern should not silently introduce type-incompatible sentinels.
|
||||
|
||||
## 5. Data Flow
|
||||
|
||||
### Before (buggy)
|
||||
```
|
||||
set_value(True) → app.ui_separate_tier1 = True
|
||||
save_workspace_profile → _capture_workspace_profile → ini=b"" (bytes)
|
||||
→ to_dict() → {"ini_content": b""}
|
||||
→ tomli_w.dump → TypeError
|
||||
→ profile NOT saved
|
||||
set_value(False) → app.ui_separate_tier1 = False
|
||||
load_workspace_profile → name not in workspace_profiles → no-op
|
||||
assert get_value is True → FAILS (still False)
|
||||
```
|
||||
|
||||
### After (fixed)
|
||||
```
|
||||
set_value(True) → app.ui_separate_tier1 = True
|
||||
save_workspace_profile → _capture_workspace_profile → ini="" (str)
|
||||
→ to_dict() → {"ini_content": ""}
|
||||
→ tomli_w.dump → OK
|
||||
→ profile saved
|
||||
set_value(False) → app.ui_separate_tier1 = False
|
||||
load_workspace_profile → name in workspace_profiles → _apply_workspace_profile
|
||||
→ setattr(self, "ui_separate_tier1", True)
|
||||
assert get_value is True → PASSES
|
||||
```
|
||||
|
||||
## 6. Error Handling
|
||||
|
||||
- The defer branch and the `except` branch both set `ini = ""`. Empty string is a valid `str` and is safe for `tomli_w`, for the dataclass, and for `imgui.load_ini_settings_from_memory("")` (which is a no-op that lets ImGui use its defaults).
|
||||
- No new exceptions are introduced. The `TypeError` from the buggy `b""` goes away because the type is now `str`.
|
||||
- The new regression test (`test_workspace_profile_serialization.py`) is itself a forward-looking guard: if a future change reintroduces a bytes sentinel, the test will fail with a clear message.
|
||||
|
||||
## 7. Testing Strategy
|
||||
|
||||
### New tests
|
||||
- `tests/test_workspace_profile_serialization.py::test_workspace_profile_empty_ini_content_roundtrips` — pure unit test, <1s, encodes the str contract.
|
||||
|
||||
### Existing tests that should now pass
|
||||
- `tests/test_auto_switch_sim::test_auto_switch_sim` — saves+loads workspace profile.
|
||||
- `tests/test_workspace_profiles_sim::test_workspace_profiles_restoration` — saves+loads workspace profile.
|
||||
- `tests/test_prior_session_no_pop_imbalance::test_no_extraneous_pop_when_prior_session_renders` — mock setup fix.
|
||||
|
||||
### Regression check
|
||||
- Re-run the full batched test suite (`scripts/run_tests_batched.py`) after the fixes; expect 272/272 pass.
|
||||
- Re-run targeted batches of theme tests (`test_theme*`, `test_log_pruner*`, `test_view_presets*`, `test_gui_progress*`, `test_gui_phase4*`) to verify the prior doc-track fixes still pass.
|
||||
|
||||
## 8. Risk Assessment
|
||||
|
||||
| Risk | Likelihood | Impact | Mitigation |
|
||||
|---|---|---|---|
|
||||
| The `str()` coercion in the non-defer branch changes behavior | Low | Low | `imgui.save_ini_settings_to_memory()` is documented to return `str`; the coercion is defensive only. The `or ""` handles a `None` return (which `imgui-bundle` does not produce but we don't want to crash on). |
|
||||
| The new unit test depends on `tomli_w` semantics that change | Very low | Low | `tomli_w` is a stable dep; the test would only break if `bytes` becomes serializable, which would be a major version change. |
|
||||
| The mock fix in the prior session test changes other behavior | Low | Low | The fix only adds the missing tuple-return; existing mocks for other imscope context managers are untouched. |
|
||||
| Removing the `b""` sentinel causes the early-render C crash to return | Very low | High | The `try/except Exception` around `imgui.save_ini_settings_to_memory()` is preserved; the flag-based defer is preserved. Only the type of the sentinel changes. |
|
||||
|
||||
## 9. Out of Scope (Tracked Separately)
|
||||
|
||||
- **live_gui session-state contract** (test-authoring rigor, wait-for-ready pattern) — see [docs/guide_testing.md#authoring-robust-live_gui-tests-dont-assume-clean-state] (added in this session). This is a doc-only change; tests will be hardened over time as they break.
|
||||
- **sloppy.py startup latency** — new backlog item 0 in `conductor/tracks.md`, planned via superpowers writing-plans skill in a future session.
|
||||
- **Other live_gui tests still flagged as fragile in the regression-fixes plan** (MMA engine state transitions, RAG status timing) — these were in the deferred category of the `regression_fixes_20260605` plan; not addressed by this design.
|
||||
|
||||
## 10. References
|
||||
|
||||
- Commit `d7487af4` — the defer-not-catch fix that introduced the `b""` sentinel.
|
||||
- `src/gui_2.py:601-606` — current defer code.
|
||||
- `src/models.py:797-823` — `WorkspaceProfile` dataclass with `ini_content: str`.
|
||||
- `src/workspace_manager.py:48-58` — `save_profile` that calls `to_dict` then `tomli_w.dump`.
|
||||
- `docs/guide_gui_2.md#workspace-profile-defer-not-catch` — the defer-not-catch section to harden.
|
||||
- `docs/guide_testing.md#known-gotchas-2026-06-05` — the early-render C-crash section to harden.
|
||||
- `conductor/tracks.md` — `regression_fixes_20260605` and `multi_themes_20260604` entries.
|
||||
- `conductor/tracks.md` — new backlog item 0 (sloppy.py startup speedup).
|
||||
@@ -0,0 +1,200 @@
|
||||
# Live-GUI State Sync — Design
|
||||
|
||||
**Date:** 2026-06-05
|
||||
**Status:** Draft
|
||||
**Track:** live_gui_state_sync_20260605 (sub-project of v2)
|
||||
|
||||
## Problem Statement
|
||||
|
||||
`App` (`src/gui_2.py`) and `AppController` (`src/app_controller.py`) maintain **parallel state** for the same logical fields. `set_value` writes to the **Controller**, but several code paths read from the **App**, returning stale or wrong values.
|
||||
|
||||
### Concrete failures (from 2026-06-05 batched test run, batches 7, 46, 65, 68)
|
||||
|
||||
1. **`test_auto_switch_sim::test_auto_switch_sim`** — sets `ui_separate_tier1=True` and `show_windows['Diagnostics']=True`, saves `Tier3Profile`, sets to False, triggers tier-3 auto-switch. Expects `show_windows['Diagnostics']=True` restored. **Fails: profile captures from App but is set on Controller.**
|
||||
|
||||
2. **`test_workspace_profiles_restoration::test_workspace_profiles_restoration`** — sets `ui_separate_tier1=True`, saves `test_restore`, sets to False, loads. Expects True. **Fails: same root cause.**
|
||||
|
||||
3. **`test_undo_redo_lifecycle::test_undo_redo_lifecycle`** (NEW regression) — sets `ai_input="Initial Input"`, modifies to `"Modified Input"`, clicks `btn_undo`. Expects `ai_input="Initial Input"`. **Fails: snapshot reads `app.ui_ai_input` but `set_value` writes to `controller.ui_ai_input`.**
|
||||
|
||||
### Discovery (2026-06-05 execution): State sync is NOT the root cause
|
||||
|
||||
Initial hypothesis: App and Controller maintain parallel state for settable fields. Verified during execution: **the App class already has `__getattr__` (line 478) and `__setattr__` (line 483) that auto-delegate to the controller.** Writes go through `__setattr__` → controller. Reads go through `__getattr__` → controller. The state is correctly synced at the descriptor level. The original spec assumption was wrong.
|
||||
|
||||
## REAL root cause: `_capture_workspace_profile` is not a class method
|
||||
|
||||
During execution, AST analysis of `src/gui_2.py` reveals the actual bug:
|
||||
|
||||
```
|
||||
$ uv run python -c "import ast; ..."
|
||||
App methods (count): 59
|
||||
WORKSPACE METHOD: _apply_workspace_profile # ← exists
|
||||
# ← _capture_workspace_profile MISSING
|
||||
```
|
||||
|
||||
`_capture_workspace_profile` is defined at line 607 of `src/gui_2.py` with 2-space indent (intended as a class method), but the AST walks it as **nested inside `_apply_snapshot`** (line 572). The body of `_apply_snapshot` (lines 573-635) absorbs the next `def` as a nested function.
|
||||
|
||||
This means when the live_gui calls `self._app._capture_workspace_profile(name)`, Python's normal class lookup fails to find `_capture_workspace_profile` on the App class. `__getattr__('_capture_workspace_profile')` is triggered, which delegates to `self.controller._capture_workspace_profile`. The controller does NOT have this method. `AttributeError` is raised. The save callback fails silently. The test's `load_workspace_profile` finds no profile to load (because save failed). The test fails.
|
||||
|
||||
### Why AST sees it as nested
|
||||
|
||||
The likely cause is the user's recent cleanup commit `873edf42` ("began to go through the files and organize imports and gui_2.py's new context defs") which touched `src/gui_2.py:261` lines. The cleanup reorganized method placement. Either:
|
||||
- Indentation was accidentally off by 1 space on some lines.
|
||||
- A blank line or comment that closed a function body was removed.
|
||||
- Method definitions were moved but their indentation wasn't updated.
|
||||
|
||||
Specific to the bug: `_apply_snapshot` has a `try:` (line 574) without an `except` (only a `finally:` at line 604). This is valid Python syntax, but the indentation of subsequent lines may have been off, causing the AST to consume the next `def` into the `try` block.
|
||||
|
||||
## Audit of duplicated fields (retained from original spec, for context)
|
||||
|
||||
Static analysis of the 71 settable fields in `AppController._settable_fields` vs the 12 `panel_states` keys captured in `App._capture_workspace_profile`, plus the `show_windows` dict and snapshot fields:
|
||||
|
||||
| Field | In `_settable_fields` (Controller)? | Read by App code? | Sync bug? |
|
||||
|---|---|---|---|
|
||||
| `show_windows` | yes | `_capture_workspace_profile` (line 627), `_apply_workspace_profile` (line 633) | **YES** |
|
||||
| `ui_separate_task_dag` | yes | `_capture_workspace_profile` (line 615) | **YES** |
|
||||
| `ui_separate_usage_analytics` | yes | `_capture_workspace_profile` (line 616) | **YES** |
|
||||
| `ui_separate_tier1` | yes | `_capture_workspace_profile` (line 617) | **YES** |
|
||||
| `ui_separate_tier2` | yes | `_capture_workspace_profile` (line 618) | **YES** |
|
||||
| `ui_separate_tier3` | yes | `_capture_workspace_profile` (line 619) | **YES** |
|
||||
| `ui_separate_tier4` | yes | `_capture_workspace_profile` (line 620) | **YES** |
|
||||
| `ui_ai_input` | yes (`ai_input -> ui_ai_input`) | `_take_snapshot` (line 551), `_apply_snapshot` (line 569) | **YES** |
|
||||
| `ui_separate_context_preview` | no (NOT in settable_fields) | `_capture_workspace_profile` (line 611) | no — App-only |
|
||||
| `ui_separate_message_panel` | no | `_capture_workspace_profile` (line 612) | no — App-only |
|
||||
| `ui_separate_response_panel` | no | `_capture_workspace_profile` (line 613) | no — App-only |
|
||||
| `ui_separate_tool_calls_panel` | no | `_capture_workspace_profile` (line 614) | no — App-only |
|
||||
| `ui_separate_external_tools` | no | `_capture_workspace_profile` (line 621) | no — App-only |
|
||||
| `ui_discussion_split_h` | no | `_capture_workspace_profile` (line 622) | no — App-only |
|
||||
|
||||
**8 confirmed sync bugs.** Plus `ui_ai_input` (snapshot) is a 9th.
|
||||
|
||||
## Root Cause
|
||||
|
||||
`App.__init__` creates a separate `AppController` instance and later sets `self.controller._app = self` (bidirectional link). The two objects each declare their own `self.ui_separate_tier1 = False` (App) and `self.ui_separate_tier1 = False` (Controller) in their respective `__init__`s. They are independent Python attributes.
|
||||
|
||||
`set_value` (`src/api_hooks.py`, line 614) calls `setattr(controller, attr_name, value)` — writes to Controller. But `_capture_workspace_profile` reads `self.ui_separate_tier1` where `self` is the App — never updated.
|
||||
|
||||
## Design
|
||||
|
||||
### Goal
|
||||
|
||||
Eliminate the dual state. **Single source of truth: the Controller.** The App becomes a thin "view" layer that exposes Controller fields as Python properties. `set_value` continues to write to the Controller. All reads (from save, snapshot, render) transparently read from the Controller.
|
||||
|
||||
### Approach: Properties on App that delegate to Controller
|
||||
|
||||
Add `@property` definitions on the `App` class for each field that has a Controller counterpart. The getter returns `self.controller.X`. The setter (where App code writes, e.g. snapshot restore) also delegates to `self.controller.X`.
|
||||
|
||||
**Hypothetical example for `ui_separate_tier1`:**
|
||||
|
||||
```python
|
||||
# In App class (src/gui_2.py)
|
||||
|
||||
@property
|
||||
def ui_separate_tier1(self) -> bool:
|
||||
return self.controller.ui_separate_tier1
|
||||
|
||||
@ui_separate_tier1.setter
|
||||
def ui_separate_tier1(self, value: bool) -> None:
|
||||
self.controller.ui_separate_tier1 = value
|
||||
```
|
||||
|
||||
This makes `app.ui_separate_tier1` and `controller.ui_separate_tier1` the same value, regardless of which path writes. The only writes are via the property setter (or `set_value` via the Controller directly), and all reads go through the getter.
|
||||
|
||||
### Why this approach
|
||||
|
||||
- **Minimal blast radius**: The App class only adds properties; no method bodies change. Methods that read `self.X` continue to work — they just get the Controller's value via the property.
|
||||
- **Bidirectional**: Setter support is critical for `_apply_snapshot` and `_apply_workspace_profile` which set App fields directly (`self.ui_ai_input = snapshot.ai_input`). They go through the property setter, which writes to the Controller.
|
||||
- **No double-write footgun**: A "sync on set_value" alternative requires remembering to write to BOTH objects. A property approach is a single point of truth.
|
||||
- **Easy to migrate incrementally**: Each field is one property pair. Can be added one at a time with a regression test for each.
|
||||
|
||||
### Alternatives considered
|
||||
|
||||
- **A2: Merge App and Controller into one class.** Rejected: would be a 5532-line → 4000-line merge with high risk. The Controller already lives in a separate file; the App delegates to it via `self.controller.X`. Merging would lose the existing boundary.
|
||||
- **A3: Sync on every set_value (write to both).** Rejected: requires touching every writer; easy to miss a site. Property approach is one place per field.
|
||||
- **A4: Pass Controller as a method argument everywhere.** Rejected: invasive; requires changing method signatures throughout `gui_2.py` and `app_controller.py`.
|
||||
|
||||
## File Changes
|
||||
|
||||
### Modify: `src/gui_2.py` (App class)
|
||||
|
||||
Add `@property` + `@X.setter` for each of the 8 sync-bug fields, plus `ui_ai_input`:
|
||||
|
||||
```python
|
||||
@property
|
||||
def ui_separate_tier1(self) -> bool:
|
||||
return self.controller.ui_separate_tier1
|
||||
|
||||
@ui_separate_tier1.setter
|
||||
def ui_separate_tier1(self, value: bool) -> None:
|
||||
self.controller.ui_separate_tier1 = value
|
||||
```
|
||||
|
||||
Fields to add properties for:
|
||||
- `ui_ai_input` (snapshot bug)
|
||||
- `ui_separate_task_dag`
|
||||
- `ui_separate_usage_analytics`
|
||||
- `ui_separate_tier1` through `ui_separate_tier4`
|
||||
- `show_windows` (special: dict, not bool)
|
||||
|
||||
For `show_windows`, the property needs care — `set_value` may pass a new dict; the property should do `self.controller.show_windows = value` to allow full replacement, but for in-place updates (`self.show_windows["X"] = True`), the property getter returns the Controller's dict reference (so in-place mutations work) and the property setter can either replace or do nothing (since the dict is shared).
|
||||
|
||||
```python
|
||||
@property
|
||||
def show_windows(self) -> Dict[str, bool]:
|
||||
return self.controller.show_windows
|
||||
|
||||
@show_windows.setter
|
||||
def show_windows(self, value: Dict[str, bool]) -> None:
|
||||
self.controller.show_windows = value
|
||||
```
|
||||
|
||||
**Do NOT** add properties for fields that are App-only (no Controller counterpart): `ui_separate_context_preview`, `ui_separate_message_panel`, `ui_separate_response_panel`, `ui_separate_tool_calls_panel`, `ui_separate_external_tools`, `ui_discussion_split_h`, etc. — they remain as plain App attributes.
|
||||
|
||||
### Add: `tests/test_app_controller_state_sync.py` (new)
|
||||
|
||||
A new unit test that encodes the contract: **for every field in `_settable_fields` that is also referenced as `self.X` in the App class's `_capture_workspace_profile` and `_take_snapshot`/`_apply_snapshot`, writes to `app.X` and `controller.X` must be observed by both.**
|
||||
|
||||
```python
|
||||
def test_ui_separate_tier1_setter_delegates_to_controller():
|
||||
"""The App's ui_separate_tier1 property is a delegate to the Controller.
|
||||
Writes through app.ui_separate_tier1 = X are visible at controller.ui_separate_tier1,
|
||||
and writes through set_value (which goes to controller) are visible at app.ui_separate_tier1."""
|
||||
from src import app_controller, gui_2
|
||||
from src.app_controller import AppController
|
||||
# Don't fully init App (too heavy); use lightweight setup
|
||||
app = gui_2.App.__new__(gui_2.App)
|
||||
app.controller = AppController()
|
||||
app._app = app # back-ref
|
||||
# set_value goes to controller
|
||||
app.controller.ui_separate_tier1 = True
|
||||
assert app.ui_separate_tier1 is True # reads through property
|
||||
# direct set through app's property
|
||||
app.ui_separate_tier1 = False
|
||||
assert app.controller.ui_separate_tier1 is False # write visible at controller
|
||||
```
|
||||
|
||||
This is a regression test for the contract.
|
||||
|
||||
### Test impact
|
||||
|
||||
After the fix, these tests should pass:
|
||||
- `test_auto_switch_sim::test_auto_switch_sim` (writes to `app.show_windows` and `app.ui_separate_tier1` are observed by save)
|
||||
- `test_workspace_profiles_sim::test_workspace_profiles_restoration` (same)
|
||||
- `test_undo_redo_lifecycle::test_undo_redo_lifecycle` (snapshot reads from `app.ui_ai_input` get the Controller's value)
|
||||
|
||||
If `test_undo_redo_lifecycle` is **also** a flake or a regression from the user's recent cleanup commit `873edf42`, the property fix may not be sufficient. In that case, the test will continue to fail and need its own investigation track.
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
| Risk | Likelihood | Impact | Mitigation |
|
||||
|---|---|---|---|
|
||||
| Existing App code does `del app.ui_X` to reset state | Low | Low | property setter can be a no-op for `del` (raises AttributeError); review call sites |
|
||||
| App class is 5532 lines — risk of regression | High | Medium | Per-field property addition; one regression test per field; ship in a single atomic commit |
|
||||
| User's recent cleanup commit `873edf42` may have added or removed attribute references | Medium | Low | Run targeted regression test after each property addition |
|
||||
| New properties shadow existing class attributes | Low | High | Use `dir(app)` to verify no shadow before commit |
|
||||
|
||||
## Out of Scope
|
||||
|
||||
- **prior_session test mock setup** — separate track (`prior_session_test_harden_20260605`).
|
||||
- **wait-for-ready test pattern** — separate track (`wait_for_ready_test_pattern_20260605`).
|
||||
- **Other App/Controller sync bugs not in the 8 listed** — audit will continue; if more found, queue as v3 sub-track.
|
||||
- **Refactoring App and Controller into one class** — deferred; property approach is sufficient for now.
|
||||
@@ -0,0 +1,118 @@
|
||||
# prior_session_test_harden_20260605 — Design
|
||||
|
||||
**Date:** 2026-06-05
|
||||
**Status:** Draft
|
||||
**Track:** prior_session_test_harden_20260605 (sub-project of v2)
|
||||
|
||||
## Problem Statement
|
||||
|
||||
`tests/test_prior_session_no_pop_imbalance.py::test_no_extraneous_pop_when_prior_session_renders` fails with `TypeError: cannot unpack non-iterable NoneType object` at `src/gui_2.py:2333` (`imscope.window(...) as (opened, visible):`).
|
||||
|
||||
Root cause: the test mocks `imscope.window`'s `__enter__` to return a non-iterable `MagicMock()`, but the production code expects a 2-tuple. **AND** the test exercises `gui_2.render_main_interface(app_instance)`, a kitchen-sink function that calls dozens of other render functions, each with their own mock-shape requirements. After fixing the imscope.window tuple-return, the next failure surfaces at `src/gui_2.py:4496` (render_theme_panel: imgui.begin returning bool where 2-tuple expected). The test would need 50+ mocks to fully exercise `render_main_interface`.
|
||||
|
||||
## Test's Actual Intent
|
||||
|
||||
The test's only assertion is `assert push_count["n"] == pop_count["n"]` — verify that `imscope.style_color` push and pop counts balance when the prior-session render runs. This is a narrow, well-defined contract.
|
||||
|
||||
The test does NOT need to exercise the entire `render_main_interface`. It only needs to exercise the prior-session render path.
|
||||
|
||||
## Design
|
||||
|
||||
### Approach: Call the narrow prior-session render function, not the kitchen sink
|
||||
|
||||
`src/gui_2.py` has a dedicated `render_prior_session_view(app)` function (line ~4400) that handles the prior-session rendering. It's a ~30-line function with a finite, mockable set of imgui/imscope calls.
|
||||
|
||||
**Hypothetical refactor:**
|
||||
|
||||
```python
|
||||
def test_no_extraneous_pop_when_prior_session_renders():
|
||||
from src import gui_2
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
app_instance = MagicMock()
|
||||
app_instance.is_viewing_prior_session = True
|
||||
app_instance.perf_profiling_enabled = False
|
||||
app_instance.prior_disc_entries = [
|
||||
{"role": "User", "content": "test", "collapsed": False, "ts": "t1"}
|
||||
]
|
||||
|
||||
push_count = {"n": 0}
|
||||
pop_count = {"n": 0}
|
||||
def _track_push(*a, **k): push_count["n"] += 1
|
||||
def _track_pop(*a, **k): pop_count["n"] += 1
|
||||
|
||||
with patch("src.gui_2.imgui") as mock_imgui, \
|
||||
patch("src.gui_2.imscope") as mock_imscope, \
|
||||
patch("src.gui_2.theme") as mock_theme, \
|
||||
patch("src.gui_2.markdown_helper") as mock_md:
|
||||
|
||||
# Wire push/pop tracking on imscope.style_color
|
||||
mock_imscope.style_color.return_value.__enter__.side_effect = _track_push
|
||||
mock_imscope.style_color.return_value.__exit__.side_effect = lambda *a: (pop_count.__setitem__("n", pop_count["n"] + 1) or False)
|
||||
|
||||
# Set up tuple-return for ALL imscope context managers (style_color, child, id, etc.)
|
||||
for sc in [mock_imscope.style_color, mock_imscope.child, mock_imscope.id]:
|
||||
sc.return_value.__enter__ = MagicMock()
|
||||
sc.return_value.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
# Mock the small finite set of imgui calls used by render_prior_session_view
|
||||
mock_imgui.Col_ = MagicMock()
|
||||
mock_imgui.button = MagicMock(return_value=False)
|
||||
mock_imgui.same_line = MagicMock()
|
||||
mock_imgui.text_colored = MagicMock()
|
||||
mock_imgui.separator = MagicMock()
|
||||
mock_imgui.get_content_region_avail = MagicMock(return_value=MagicMock(x=800.0, y=600.0))
|
||||
mock_imgui.ImVec2 = lambda *a: MagicMock(x=a[0], y=a[1])
|
||||
mock_imgui.WindowFlags_ = MagicMock()
|
||||
mock_imgui.text = MagicMock()
|
||||
|
||||
mock_theme.get_color = MagicMock(return_value=MagicMock(x=0,y=0,z=0,w=0))
|
||||
mock_theme.ai_text_style.return_value.__enter__ = MagicMock()
|
||||
mock_theme.ai_text_style.return_value.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
mock_md.render = MagicMock()
|
||||
|
||||
# Call the narrow function, NOT the kitchen sink
|
||||
gui_2.render_prior_session_view(app_instance)
|
||||
|
||||
assert push_count["n"] == pop_count["n"], f"Push/pop imbalance: pushes={push_count['n']}, pops={pop_count['n']}"
|
||||
```
|
||||
|
||||
This is ~30 mocks instead of 50+, scoped to what `render_prior_session_view` actually uses. The imscope mocks all return their own context-manager defaults (no need to return a tuple for `style_color` since `with imscope.style_color(...) as c:` doesn't unpack). The test's actual assertion (push/pop balance) is preserved.
|
||||
|
||||
### Why this approach
|
||||
|
||||
- **Smallest change to the test**: removes 50+ mocks, replaces with 30+ scoped mocks. Test runs faster.
|
||||
- **Preserves test intent**: the assertion is still about push/pop balance in the prior-session render.
|
||||
- **Survives future refactors**: as long as `render_prior_session_view` exists, the test is meaningful. If the function is renamed/restructured, the test is localized to that function.
|
||||
- **Aligns with the live_gui test philosophy**: tests should exercise narrow paths, not kitchen sinks. (This is consistent with the [docs/guide_testing.md Authoring Robust live_gui Tests] rules I just authored.)
|
||||
|
||||
### Alternatives considered
|
||||
|
||||
- **A2: Add 50+ mocks to make `render_main_interface` work.** Rejected: the test becomes a maintenance burden (any change to any sub-render function breaks the test). It also tests too much (push/pop balance in the entire GUI, not just prior-session).
|
||||
- **A3: Skip the test entirely, mark as known-flake.** Rejected: the test is meaningful and verifies a real contract. Better to make it work.
|
||||
|
||||
## File Changes
|
||||
|
||||
### Modify: `tests/test_prior_session_no_pop_imbalance.py`
|
||||
|
||||
Replace the `render_main_interface(app_instance)` call with `render_prior_session_view(app_instance)`. Remove the mocks for the 50+ imgui methods that are NOT used by `render_prior_session_view` (e.g. `selectable`, `tree_node`, `set_scroll_here_y`, etc.). Keep the mocks for the 30+ methods that ARE used.
|
||||
|
||||
### No production code changes
|
||||
|
||||
The test is rewritten; `render_prior_session_view` itself does not change.
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
| Risk | Likelihood | Impact | Mitigation |
|
||||
|---|---|---|---|
|
||||
| `render_prior_session_view` signature/name changes | Low | Medium | The test is local to this function; future refactors will update both |
|
||||
| Mocking too aggressively (mocking something the function actually uses) | Medium | Low | Run the test; if it fails, add the missing mock |
|
||||
| Test was testing more than just push/pop balance (e.g. some side effect) | Low | Low | Read the original test docstring; the only assertion is push/pop balance |
|
||||
|
||||
## Out of Scope
|
||||
|
||||
- **State sync fix** — separate track (`live_gui_state_sync_20260605`).
|
||||
- **Wait-for-ready pattern** — separate track (`wait_for_ready_test_pattern_20260605`).
|
||||
- **undo_redo_lifecycle** — separate track (`undo_redo_lifecycle_fix_20260605`).
|
||||
- **Refactoring `render_main_interface` to be smaller** — deferred; out of scope for this track.
|
||||
@@ -0,0 +1,83 @@
|
||||
# undo_redo_lifecycle_fix_20260605 — Design
|
||||
|
||||
**Date:** 2026-06-05
|
||||
**Status:** Draft
|
||||
**Track:** undo_redo_lifecycle_fix_20260605 (sub-project of v2)
|
||||
|
||||
## Problem Statement
|
||||
|
||||
`tests/test_undo_redo_sim.py::test_undo_redo_lifecycle` failed in the 2026-06-05 second batched test run (after the first run had it passing). The test:
|
||||
|
||||
1. Sets `temperature=0.5` and `ai_input="Initial Input"`.
|
||||
2. Modifies to `temperature=1.5` and `ai_input="Modified Input"`.
|
||||
3. Asserts current state — passes.
|
||||
4. Clicks `btn_undo`.
|
||||
5. Asserts `ai_input == "Initial Input"` and `temperature == 0.5`.
|
||||
6. **Fails on the `ai_input` assertion**: gets `''` (empty string).
|
||||
|
||||
The undo restores `temperature` correctly but not `ai_input`. The other 2 tests in the same file (`test_undo_redo_discussion_mutation`, `test_undo_redo_context_mutation`) pass — they don't exercise `ai_input`.
|
||||
|
||||
### Possible causes
|
||||
|
||||
1. **App/Controller state sync bug for `ai_input`.** The snapshot at `src/gui_2.py:551` reads `self.ui_ai_input` (App), but `set_value` writes to `controller.ui_ai_input`. The snapshot captures the App's (stale) value. **This should be fixed by the `live_gui_state_sync_20260605` track** (which adds an `ui_ai_input` property on the App that delegates to the Controller).
|
||||
|
||||
2. **Snapshot doesn't include `ai_input` field at all.** Check `src/history.py:UISnapshot` — if `ai_input` isn't a field, the snapshot stores nothing, and the apply can't restore.
|
||||
|
||||
3. **Test flake.** The test was passing in the first run, failing in the second. The `live_gui` fixture is session-scoped, and different test orders can produce different state. The test's `time.sleep(2.0)` after `btn_undo` may not be enough if the GUI is under load.
|
||||
|
||||
4. **Recent user commit `873edf42` regression.** The user's cleanup commit touched 53 files including `src/gui_2.py:261` lines. If the cleanup accidentally changed the snapshot mechanism, this could break the test.
|
||||
|
||||
## Design
|
||||
|
||||
### Approach: Two-phase investigation
|
||||
|
||||
**Phase 1: Re-run the test after the `live_gui_state_sync_20260605` track lands.**
|
||||
|
||||
If the state-sync property fix for `ui_ai_input` unblocks the test, the issue is resolved. No further work needed.
|
||||
|
||||
**Phase 2: If the test still fails, deep-dive into the snapshot mechanism.**
|
||||
|
||||
Investigate in this order:
|
||||
1. Check `src/history.py:UISnapshot` to see if `ai_input` is a field. If not, add it.
|
||||
2. Check `src/gui_2.py:_apply_snapshot` to see if it restores `ai_input`. If not, add the restore line.
|
||||
3. Check if there's a per-tick snapshot filter that excludes certain fields.
|
||||
4. Add a regression test that explicitly verifies the snapshot/undo round-trip for `ai_input`.
|
||||
|
||||
**Phase 3: If still failing, test-ordering / flake investigation.**
|
||||
|
||||
The test uses `time.sleep(2.0)` after `btn_undo`. Convert to polling (`wait_for_load_completion` from the `wait_for_ready_test_pattern_20260605` track). If the test passes with polling, it was a flake.
|
||||
|
||||
### Why this approach
|
||||
|
||||
- **Sequential investigation**: cheapest fixes first. State-sync is the most likely cause (it just landed as a property fix). Snapshot mechanism is the second most likely. Flake is the third.
|
||||
- **No speculative changes**: don't add `ai_input` to the snapshot if it's already there. Don't change the undo mechanism if the state-sync fix is sufficient.
|
||||
|
||||
## File Changes
|
||||
|
||||
### Phase 1: None (state-sync fix is in a different track)
|
||||
|
||||
### Phase 2 (if needed):
|
||||
|
||||
- Modify: `src/history.py` (add `ai_input` field to UISnapshot if missing)
|
||||
- Modify: `src/gui_2.py:_apply_snapshot` (add `ai_input` restore line if missing)
|
||||
- Add: `tests/test_undo_redo_ai_input_snapshot.py` (regression test for the round-trip)
|
||||
|
||||
### Phase 3 (if needed):
|
||||
|
||||
- Modify: `tests/test_undo_redo_sim.py` (replace `time.sleep(2.0)` with `wait_for_load_completion`)
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
| Risk | Likelihood | Impact | Mitigation |
|
||||
|---|---|---|---|
|
||||
| Phase 1 fixes the issue | High | None | Done |
|
||||
| Phase 2 needed: snapshot already has ai_input but apply doesn't restore | Medium | Low | Check first, then add the restore line |
|
||||
| Phase 2 needed: snapshot doesn't have ai_input | Low | Low | Add the field + apply line |
|
||||
| Phase 3 needed: it's a flake | Low | None | Replace sleeps with polling |
|
||||
|
||||
## Out of Scope
|
||||
|
||||
- **State sync fix** — separate track (`live_gui_state_sync_20260605`).
|
||||
- **prior_session test** — separate track (`prior_session_test_harden_20260605`).
|
||||
- **wait_for_ready pattern** — separate track (`wait_for_ready_test_pattern_20260605`).
|
||||
- **General undo/redo system improvements** — out of scope.
|
||||
@@ -0,0 +1,112 @@
|
||||
# wait_for_ready_test_pattern_20260605 — Design
|
||||
|
||||
**Date:** 2026-06-05
|
||||
**Status:** Draft
|
||||
**Track:** wait_for_ready_test_pattern_20260605 (sub-project of v2)
|
||||
|
||||
## Problem Statement
|
||||
|
||||
Two failing live_gui tests use `time.sleep(N)` to wait for asynchronous GUI operations to complete:
|
||||
|
||||
- `tests/test_workspace_profiles_sim.py` — `time.sleep(2.0)` after save and after load; `time.sleep(1.0)` after each set_value.
|
||||
- `tests/test_auto_switch_sim.py` — `time.sleep(1)` after each `push_event`.
|
||||
|
||||
Fixed sleeps are a fragile test pattern:
|
||||
- On slow machines the sleep may be insufficient; the assertion runs before the operation completes.
|
||||
- On fast machines the sleep is wasted; the test takes longer than necessary.
|
||||
- Tests that pass with `time.sleep(2.0)` in CI may fail on a developer machine with different load.
|
||||
|
||||
After the state-sync fix (`live_gui_state_sync_20260605`) lands, these tests should pass at the current 2-second sleep. **But the test pattern is still wrong** — the tests should poll for completion, not assume timing.
|
||||
|
||||
## Design
|
||||
|
||||
### Approach: Migrate `time.sleep` to a wait-for-ready helper
|
||||
|
||||
`src/api_hook_client.py` already exposes `wait_for_event(event_type, timeout)` and `get_value(item)`. The tests can use these directly.
|
||||
|
||||
**Hypothetical example — the current pattern:**
|
||||
|
||||
```python
|
||||
client.set_value('ui_separate_tier1', True)
|
||||
time.sleep(1.0)
|
||||
client.push_event("custom_callback", {"callback": "save_workspace_profile", "args": ["test_restore", "project"]})
|
||||
time.sleep(2.0) # HOPE the save completes within 2s
|
||||
client.set_value('ui_separate_tier1', False)
|
||||
time.sleep(1.0)
|
||||
client.push_event("custom_callback", {"callback": "load_workspace_profile", "args": ["test_restore"]})
|
||||
time.sleep(2.0) # HOPE the load completes within 2s
|
||||
assert client.get_value('ui_separate_tier1') is True
|
||||
```
|
||||
|
||||
**Migrated pattern:**
|
||||
|
||||
```python
|
||||
def wait_for_save_completion(client, profile_name, timeout=5.0):
|
||||
"""Poll until the saved profile appears in the workspace profiles."""
|
||||
import time
|
||||
deadline = time.time() + timeout
|
||||
while time.time() < deadline:
|
||||
profiles = client.get_value('workspace_profiles') or {}
|
||||
if profile_name in profiles:
|
||||
return
|
||||
time.sleep(0.1)
|
||||
raise TimeoutError(f"Save did not complete within {timeout}s")
|
||||
|
||||
def wait_for_load_completion(client, item, expected, timeout=5.0):
|
||||
"""Poll until the item's value matches expected."""
|
||||
import time
|
||||
deadline = time.time() + timeout
|
||||
while time.time() < deadline:
|
||||
if client.get_value(item) == expected:
|
||||
return
|
||||
time.sleep(0.1)
|
||||
raise TimeoutError(f"Load did not apply {item}={expected} within {timeout}s")
|
||||
|
||||
client.set_value('ui_separate_tier1', True)
|
||||
# No sleep needed; set_value returns when the value is set on the controller
|
||||
client.push_event("custom_callback", {"callback": "save_workspace_profile", "args": ["test_restore", "project"]})
|
||||
wait_for_save_completion(client, "test_restore")
|
||||
client.set_value('ui_separate_tier1', False)
|
||||
client.push_event("custom_callback", {"callback": "load_workspace_profile", "args": ["test_restore"]})
|
||||
wait_for_load_completion(client, 'ui_separate_tier1', True)
|
||||
```
|
||||
|
||||
### Why this approach
|
||||
|
||||
- **Polling, not fixed sleeps**: 100ms poll interval is responsive without busy-waiting.
|
||||
- **Generous timeouts**: 5s default is well over the typical ~100ms operation; catches genuine hangs.
|
||||
- **Reusable helpers**: `wait_for_save_completion` and `wait_for_load_completion` are simple and can be added to a shared test helper module.
|
||||
- **Failure messages are clear**: TimeoutError explicitly says which operation timed out.
|
||||
|
||||
### Alternatives considered
|
||||
|
||||
- **A2: Add wait_for_X helpers to ApiHookClient itself.** Rejected: ApiHookClient should remain a thin transport; test-helper logic doesn't belong there. Keep helpers in `tests/conftest.py` or a `tests/helpers.py` module.
|
||||
- **A3: Use `wait_for_event` exclusively.** The Hook API's `wait_for_event` listens for events the GUI emits. save/load may not emit events in a way the test can match. Polling `get_value` is more direct.
|
||||
|
||||
## File Changes
|
||||
|
||||
### Modify: `tests/test_workspace_profiles_sim.py`
|
||||
|
||||
Replace `time.sleep(...)` with `wait_for_save_completion` and `wait_for_load_completion` calls. Add the helper functions at the top of the file (or import from a shared helper).
|
||||
|
||||
### Modify: `tests/test_auto_switch_sim.py`
|
||||
|
||||
Replace `time.sleep(...)` with similar polling helpers.
|
||||
|
||||
### Optionally: Create: `tests/helpers.py`
|
||||
|
||||
If multiple tests need the same helpers, extract them to a shared module. For now, keep them inline (2 tests, ~30 lines of helpers total).
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
| Risk | Likelihood | Impact | Mitigation |
|
||||
|---|---|---|---|
|
||||
| The polling masks a slow operation that's now flaky | Low | Medium | Generous 5s timeout; if a test times out, the test message points to which operation |
|
||||
| Helper functions added in 2 places diverge | Medium | Low | If 3+ tests need the same helper, extract to `tests/helpers.py` |
|
||||
|
||||
## Out of Scope
|
||||
|
||||
- **State sync fix** — separate track (`live_gui_state_sync_20260605`).
|
||||
- **prior_session test** — separate track (`prior_session_test_harden_20260605`).
|
||||
- **Migrating other live_gui tests that use `time.sleep`** — out of scope for now. Track as a follow-up if more flakes appear.
|
||||
- **Replacing `time.sleep` with `asyncio.sleep`** — out of scope; the live_gui tests are sync, and the GUI event queue is sync.
|
||||
@@ -0,0 +1,148 @@
|
||||
# Prior-Session Sepia Tint — Design
|
||||
|
||||
**Date:** 2026-06-10
|
||||
**Status:** Approved (pending user spec review)
|
||||
**Track:** `prior_session_sepia_20260610`
|
||||
**Spec:** [../../../conductor/tracks/prior_session_sepia_20260610/spec.md](../../../conductor/tracks/prior_session_sepia_20260610/spec.md)
|
||||
|
||||
## Problem Statement
|
||||
|
||||
The current prior-session (Historical Replay) mode uses `theme.get_color("bubble_vendor")` as a
|
||||
background tint at three call sites in `src/gui_2.py` (lines 1028, 3960, plus the "HISTORICAL
|
||||
VIEW" banners at 5142 and 5440). This is a **semantic overload** — `bubble_vendor` is the
|
||||
"Vendor API" role's bubble color, not a dedicated prior-session slot. Theme authors cannot tune
|
||||
the prior-session feel without changing the Vendor API bubble.
|
||||
|
||||
Beyond the missing dedicated slot, the **content** (text, markdown) renders at full
|
||||
palette saturation. The "obviously looking at the past" cue is carried only by the bg, not by
|
||||
the prose.
|
||||
|
||||
**Pre-existing related bug** (surfaced by the user during brainstorming): code blocks rendered
|
||||
by `imgui_color_text_edit` are not tonemap-aware. The library has 4 hardcoded palettes (`dark`,
|
||||
`light`, `mariana`, `retro_blue`) and their colors are never passed through `theme._tone_map()`.
|
||||
The user has called this "disappointing" because it defeats the primary purpose of the tonemapper:
|
||||
allowing a light theme to be usable on a bright monitor without searing the user's retinas.
|
||||
**This bug is NOT fixable in this track** — see "Honest constraint" below.
|
||||
|
||||
## Brainstorm Q&A
|
||||
|
||||
### Q1. Tint scope (data only vs. data+chrome vs. whole window)
|
||||
|
||||
**A1 confirmed sepia. A2 chose "anything that is actually affected by the prior session (is
|
||||
hosting old information, old state)" → scope (ii) data + chrome inside prior-session views, with
|
||||
fallback to (iii) whole window if scope (ii) doesn't look "obviously old" in manual review.**
|
||||
|
||||
### Q2. Two independent effects (distinct bg + tint) or single transform
|
||||
|
||||
**A — Two independent effects, slider only controls the content tint (recommended and chosen).**
|
||||
New theme slots: `prior_session_bg` (flat warm color), `prior_session_tint` (sepia color),
|
||||
`prior_session_amount` (per-palette float 0.0-1.0). Slider controls only the amount; bg is
|
||||
whatever the theme says.
|
||||
|
||||
### Q3. (a) Slider location, (b) tint scope, (c) default per-theme, (d) code blocks
|
||||
|
||||
**(a) Theme Settings panel** (under the existing Tone Mapping section). Mirrors the
|
||||
tonemap pattern exactly.
|
||||
|
||||
**(b) Scope (ii) data + chrome inside prior-session views**, with fallback to (iii) whole
|
||||
window only if (ii) doesn't look obviously old.
|
||||
|
||||
**(c) 0.3 (subtle)** — the user can slide up if they want more aggressive. The default is
|
||||
intentionally subtle because the user said "obviously looking at the past" but the
|
||||
prior-session feature is a niche mode, not a primary view.
|
||||
|
||||
**(d) Originally proposed: bundle the code-block tonemap fix into this track.** The user
|
||||
revealed the pre-existing disappointment and said: *"if you can somehow tint the code blocks
|
||||
lmk cause right now they are not affected by tonemapping."* The fix was proposed as
|
||||
"mutate the `Palette` struct's color slots via `editor.get_palette()` and re-apply."
|
||||
|
||||
### Q4. Float-only math (HARD CONSTRAINT)
|
||||
|
||||
> "Make sure that all math you do is not integer based. I want to have as much accuracy as
|
||||
> possible for smooth calculations."
|
||||
|
||||
Applied to: `apply_prior_tint`, the slider (`slider_float` not `slider_int`), the per-palette
|
||||
state dict (stores `float` not `int`), the TOML key (`prior_session_amount: float`), the
|
||||
code-block palette mutation. The transform pipeline never truncates to int.
|
||||
|
||||
## Approach: A1 — Per-render explicit transform
|
||||
|
||||
Add `apply_prior_tint(rgba, palette) -> rgba` helper in `src/theme_2.py`. Per-palette state
|
||||
lives in `_prior_session_amount: dict[str, float]` mirroring `_brightness` exactly. Each
|
||||
prior-session rendering site in `src/gui_2.py` wraps its `theme.get_color()` call with
|
||||
`apply_prior_tint(...)` (one-line wrap).
|
||||
|
||||
### Honest constraint surfaced during self-review
|
||||
|
||||
I verified the upstream `imgui_bundle 1.92.5` API before writing the plan:
|
||||
|
||||
```
|
||||
$ python -c "from imgui_bundle import imgui_color_text_edit as ed; help(ed.TextEditor.get_palette)"
|
||||
get_palette(self) -> imgui_bundle._imgui_bundle.imgui_color_text_edit.TextEditor.PaletteId
|
||||
$ python -c "from imgui_bundle import imgui_color_text_edit as ed; help(ed.TextEditor.set_palette)"
|
||||
set_palette(self, a_value: imgui_bundle._imgui_bundle.imgui_color_text_edit.TextEditor.PaletteId) -> None
|
||||
```
|
||||
|
||||
**`PaletteId` is a 4-value enum** (`dark`, `light`, `mariana`, `retro_blue`). There is
|
||||
no `Palette` struct with mutable per-color slots. The original brainstorm proposed
|
||||
`apply_color_grades_to_editor_palette(editor, palette)` that would mutate the struct
|
||||
in-place — but the struct doesn't exist in this API surface.
|
||||
|
||||
**This means the code-block tonemap-awareness is NOT fixable in this track** (or any
|
||||
track that doesn't fork the library). The user's disappointment is real and the
|
||||
pre-existing behavior persists. The same constraint forced the `multi_themes_20260604`
|
||||
track to ship a `syntax_palette` enum field rather than custom token colors. The
|
||||
honest answer is in the spec's §1.1.1.
|
||||
|
||||
### Why A1 over A2 (transparent via get_color) and A3 (context manager)
|
||||
|
||||
- **A2** would auto-apply sepia inside `theme.get_color()` when `is_viewing_prior_session` is
|
||||
True. Rejected: violates the data-oriented "view composes" principle; risks accidentally
|
||||
tinting status indicators that must stay saturated.
|
||||
- **A3** would require a `with prior_session_view():` wrapper at every prior-session site.
|
||||
Rejected: same number of wraps as A1 but uglier.
|
||||
|
||||
### Float-only math contract
|
||||
|
||||
```
|
||||
result = lerp(desaturate(input), tint_color, amount)
|
||||
```
|
||||
|
||||
- `desaturate(rgba)`: BT.709 luma `0.2126*r + 0.7152*g + 0.0722*b` (all float, all 0.0-1.0).
|
||||
- `lerp(a, b, t)`: `a + (b - a) * t` per channel, `t` clamped to [0.0, 1.0].
|
||||
- `apply_prior_tint(rgba, palette)`: identity at amount=0.0; pure tint at amount=1.0; alpha
|
||||
passed through unchanged; output is `tuple[float, float, float, float]`.
|
||||
|
||||
## File changes (high-level)
|
||||
|
||||
| File | Action | Purpose |
|
||||
|---|---|---|
|
||||
| `src/theme_2.py` | Modify | Add `_prior_session_amount` dict + 3 accessors; add `_desaturate`, `_lerp_rgba`, `_imvec4_to_rgba`, `_rgba_to_imvec4`, `apply_prior_tint`; add 3 keys to fallback dict; persist to config |
|
||||
| `src/theme_models.py` | Modify | Add 3 fields to `ThemePalette`; update `from_dict` / `to_dict` / validator |
|
||||
| `src/gui_2.py` | Modify | 2 `bubble_vendor` → `prior_session_bg` swaps; 4 `apply_prior_tint` wraps at the 2 banners + 2 render functions; 1 new Theme panel section |
|
||||
| `themes/*.toml` (8 files) | Modify | Add 3 new keys with per-theme defaults |
|
||||
| `tests/test_prior_session_amount.py` | Create | per-palette dict semantics |
|
||||
| `tests/test_prior_session_tint.py` | Create | math contract: identity/pure/monotonic/alpha |
|
||||
| `tests/test_prior_session_toml.py` | Create | round-trip + validation |
|
||||
| `tests/test_prior_session_render.py` | Create | ImVec4 ↔ tuple round-trip |
|
||||
| `tests/test_prior_session_persistence.py` | Create | slider → save → restart round-trip |
|
||||
|
||||
## Risk assessment
|
||||
|
||||
| Risk | Likelihood | Impact | Mitigation |
|
||||
|---|---|---|---|
|
||||
| Float math accumulates rounding error over many frames | Low | Low | Each `apply_prior_tint` call is independent; no accumulation; output clamped to [0.0, 1.0] |
|
||||
| The 8 themes don't all have sensible defaults for the new keys | Low | Low | Defaults in the fallback dict cover missing keys |
|
||||
| Light themes need a different default for `prior_session_bg` (cream vs. dark brown) | Med | Low | Defaults table in the spec sets per-light-theme `prior_session_bg = (235, 220, 190)` |
|
||||
| Live-gui tests regress because of the new `apply_prior_tint` wraps | Low | Med | Phase 4 batch-verifies the full suite; per `live_gui_test_hardening_20260605` rule, batch is the only verification that matters |
|
||||
| The scope (ii) "data + chrome inside prior-session views" doesn't look "obviously old" at the 0.3 default | Med | Low | Phase 4 manual smoke escalates to scope (iii) if needed; the wrap is local to 6 sites, easy to expand |
|
||||
| **Code-block tonemap-awareness disappoints the user again** | High | Low | Explicit §1.1.1 in the spec; the pre-existing bug persists; user was told upfront in the design doc that the API doesn't support per-instance Palette |
|
||||
|
||||
## Out of scope (matches spec §9)
|
||||
|
||||
- Per-language syntax color customization (upstream limitation)
|
||||
- Film grain / vignette / scanline post-effect
|
||||
- Per-theme user overrides via `config.toml` (TOML key is factory default only)
|
||||
- Modifying the upstream `imgui_color_text_edit` library
|
||||
- Process-isolation of the pure helper
|
||||
- Reorganizing the existing tonemap state dicts
|
||||
@@ -0,0 +1,444 @@
|
||||
# Chronology v2 Redo — Design Spec
|
||||
|
||||
**Date:** 2026-07-01
|
||||
**Track ID:** `chronology_v2_20260701`
|
||||
**Priority:** A (meta-tooling / infrastructure)
|
||||
**Status:** design (pre-spec)
|
||||
**Ancestors:**
|
||||
- `conductor/tracks/chronology_20260619/spec.md` (the v2 rewrite spec, 354 lines — designed but never executed)
|
||||
- `docs/reports/2026-06-15/CHRONOLOGY_TRACK_HANDOVER_20260620.md` (the v1 failure report, 128 lines)
|
||||
- `docs/reports/2026-06-15/CHRONOLOGY_MIGRATION_20260619.md` (the v1 migration report)
|
||||
- `docs/reports/2026-06-15/TRACK_COMPLETION_chronology_20260619.md` (the v1 end-of-track report)
|
||||
|
||||
## Overview
|
||||
|
||||
The `chronology_20260619` track produced a broken `conductor/chronology.md` (v1):
|
||||
167 of 216 rows had wrong status (the classifier read stale `metadata.json.status`
|
||||
instead of git history), summaries were metadata-field text instead of track
|
||||
descriptions, and the per-row cross-check was bypassed. A v2 rewrite was specced
|
||||
and planned in detail but never executed. The track sits at `current_phase=10`
|
||||
pending user sign-off that never came, blocking `superpowers_review_20260619`.
|
||||
|
||||
This track is the redo: a **fresh track** that closes out the old one, adopts
|
||||
the v2 design as a starting point, revises it for the current project state
|
||||
(5+ days of desync, new track patterns, the tracks.md bloat), and executes it
|
||||
through to a user sign-off that is actionable this time.
|
||||
|
||||
## What v1 Got Wrong (from the records)
|
||||
|
||||
Per `CHRONOLOGY_TRACK_HANDOVER_20260620.md`:
|
||||
|
||||
1. **`_classify_status()` reads `metadata.json.status`** — a stale field set when
|
||||
each track was created, rarely updated when work completed or was abandoned.
|
||||
167/216 rows had wrong status.
|
||||
2. **Summaries are metadata-field text** (`**Priority:** A (foundational...)`,
|
||||
`**Date:** 2026-06-20`) not actual track descriptions.
|
||||
3. **Phase 8 per-row cross-check was bypassed** in favor of bulk structural
|
||||
verification; the manual summary-adequacy check was partial (15-row sample).
|
||||
4. **Phase 6 user review gate was bypassed** in the autonomous session.
|
||||
5. **No quality gate** to detect a broken classifier before the chronology ships.
|
||||
6. **No maintenance plan** — the chronology desynced within days because nobody
|
||||
regenerated it after new tracks shipped.
|
||||
|
||||
The five lessons from the handover (lines 88-98):
|
||||
1. Bypassing the manual review clause was the original sin.
|
||||
2. `metadata.json` is a snapshot, not a source of truth.
|
||||
3. Git history is the project's audit log — use it.
|
||||
4. Default to "when in doubt, ask" — the chronology is read by humans.
|
||||
5. The user said "manual review" twice; both times an interpretation was found
|
||||
to be less strict — listen to the literal request.
|
||||
|
||||
## Goals
|
||||
|
||||
1. Produce a correct `conductor/chronology.md` where every row's status is
|
||||
backed by git-history evidence, not stale metadata.
|
||||
2. Produce a per-row evidence artifact (the quality report) so the user can
|
||||
audit the classification without re-deriving it.
|
||||
3. Close out `chronology_20260619` (mark superseded, archive, unblock
|
||||
`superpowers_review_20260619`).
|
||||
4. De-gunk `conductor/tracks.md` — remove shipped/completed tracks from the
|
||||
active queue, remove the Phase 0-9 history sections that duplicate
|
||||
chronology.md, leave only the active queue + standby + a pointer.
|
||||
5. Add a `conductor/workflow.md` maintenance rule so the chronology is
|
||||
regenerated after each track ships (closes the desync root cause).
|
||||
6. Ship a quality-gate script that catches a broken classifier before it
|
||||
ships (closes the "no quality gate" root cause).
|
||||
|
||||
## Non-Goals
|
||||
|
||||
- Fixing or executing `superpowers_review_20260619` (just unblocking it).
|
||||
- Changing how `metadata.json.status` is maintained going forward (the
|
||||
classifier uses git history, not metadata; metadata staleness is no longer
|
||||
the problem).
|
||||
- Archiving the 66 folders in `conductor/tracks/` that are already shipped
|
||||
(separate cleanup; the chronology indexes them regardless of location).
|
||||
- Renaming or restructuring `conductor/archive/` (out of scope; the
|
||||
chronology walks it as-is).
|
||||
- A broader `workflow.md` review for other stale rules (the only workflow.md
|
||||
change is the chronology maintenance section).
|
||||
|
||||
## Design
|
||||
|
||||
### 1. New track identity + old track close-out
|
||||
|
||||
**New track:** `chronology_v2_20260701` (Priority A; meta-tooling/infrastructure).
|
||||
Fresh track, not a continuation of `chronology_20260619`.
|
||||
|
||||
**Old track close-out (Phase 1):**
|
||||
- Mark `chronology_20260619` as superseded in its `state.toml`
|
||||
(`status = "superseded"`, `current_phase = 10`, add a `[supersession]`
|
||||
section pointing to `chronology_v2_20260701`).
|
||||
- Update its tracks.md row (line 64) to reflect supersession.
|
||||
- Archive `conductor/tracks/chronology_20260619/` →
|
||||
`conductor/archive/chronology_20260619/`. The v2 spec/plan are preserved
|
||||
in git history; the new track references them by commit SHA.
|
||||
- **Unblock `superpowers_review_20260619`** — remove `chronology_20260619`
|
||||
from its `state.toml` `[blocked_by]` entirely (no re-gating on the new
|
||||
track).
|
||||
|
||||
**v2 design adoption:** The new track's spec explicitly cites
|
||||
`conductor/tracks/chronology_20260619/spec.md` (the v2 rewrite spec) and
|
||||
`CHRONOLOGY_TRACK_HANDOVER_20260620.md` (the failure report) as its design
|
||||
ancestors. It adopts the v2 status enum, the git-history classifier approach,
|
||||
and the quality-gate concept — with the revisions below.
|
||||
|
||||
### 2. The six revisions to v2
|
||||
|
||||
#### Revision 1 — The desync gap (regenerate from current filesystem)
|
||||
|
||||
v2 was specced when the newest track was ~2026-06-20. The chronology now needs
|
||||
to cover 5+ more days of tracks: the layout saga
|
||||
(`default_layout_install_20260629`, `default_layout_extract_20260629`,
|
||||
`default_layout_install_followup_20260629`), the MMA quarantine
|
||||
(`mma_quarantine_rag_test_decoupling_20260701`), the module_taxonomy abort +
|
||||
cleanup (`module_taxonomy_refactor_20260627`, `post_module_taxonomy_de_cruft_20260627`),
|
||||
`cruft_elimination_20260627`, `directive_hotswap_harness_20260627`,
|
||||
`enforcement_gap_closure_20260627`, `test_engine_integration_20260627`,
|
||||
`fix_mma_concurrent_tracks_sim_20260627`, `type_alias_unfuck_20260626`,
|
||||
`video_analysis_campaign_2_20260627`.
|
||||
|
||||
**Change:** The new track's first generation pass runs against the **current**
|
||||
filesystem (all `conductor/tracks/` + `conductor/archive/` as of execution
|
||||
day), not the 2026-06-19 snapshot. The generation script walks both directories
|
||||
fresh each run.
|
||||
|
||||
#### Revision 2 — `superpowers_review_20260619` blocker resolution
|
||||
|
||||
v2 didn't address this because it was rewriting the same track in place. The
|
||||
new track explicitly closes out `chronology_20260619` and removes it from
|
||||
`superpowers_review_20260619/state.toml` `[blocked_by]` (no re-gating).
|
||||
|
||||
#### Revision 3 — Classifier heuristics updated for recent track patterns
|
||||
|
||||
v2's 5-step git-history algorithm was designed 2026-06-20. Since then, new
|
||||
patterns emerged that it would misclassify:
|
||||
|
||||
- **Aborted tracks** (`module_taxonomy_refactor_20260627`): many
|
||||
`conductor(track):` + `conductor(plan):` commits but a
|
||||
`TRACK_ABORTED_*.md` report — classifier must detect the abort report as
|
||||
an `Abandoned`/`Superseded` signal.
|
||||
- **Phase 9 patches after "completion"** (`result_migration_cruft_removal_20260620`):
|
||||
a track that "shipped" then got a patch commit days later — classifier must
|
||||
look at the latest commit, not just count.
|
||||
- **Tier 2 autonomous tracks**: produce many `conductor(plan):` commits (one
|
||||
per task) — the "feat/fix/refactor vs chore/docs" heuristic must not count
|
||||
`conductor(plan):` as a work commit.
|
||||
- **Follow-up tracks** (`default_layout_install_followup_20260629`): short,
|
||||
few commits, but legitimately `Completed` — the "0-1 commits + >14 days
|
||||
old = Abandoned" rule would misfire.
|
||||
|
||||
**Change:** The classifier's commit-message pattern list is extended:
|
||||
|
||||
- `conductor(plan):`, `conductor(state):`, `conductor(track):`,
|
||||
`docs(spec):`, `docs(plan):` are **metadata commits**, not work commits
|
||||
(don't count toward the "≥3 work commits = Completed" threshold).
|
||||
- `feat:`, `fix:`, `refactor:`, `perf:`, `test:`, `docs(report):` are work
|
||||
commits.
|
||||
- Presence of `TRACK_ABORTED_*.md` or `TRACK_COMPLETION_*.md` in
|
||||
`docs/reports/` matching the track ID is a **strong signal** that
|
||||
overrides commit-count heuristics.
|
||||
- The "last commit > 14 days = Abandoned" rule is **removed**; replaced
|
||||
with "no work commits AND no completion/abort report = Needs Review".
|
||||
- Confidence is reported per-row; anything below a threshold goes to the
|
||||
Needs Review queue for manual classification.
|
||||
|
||||
#### Revision 4 — tracks.md de-gunk
|
||||
|
||||
v2's scope was only chronology.md. The new track also restructures
|
||||
`conductor/tracks.md`:
|
||||
|
||||
**Current state (96KB, bloated):**
|
||||
- 60-row "Active Tracks (Current Queue)" table — ~40 of these rows are
|
||||
shipped/completed tracks that belong in history, not the active queue.
|
||||
- Phase 0-9 chronological sections with Completed/Archived subsections —
|
||||
duplicates chronology.md.
|
||||
- 4 backlog/follow-up sections — some entries are shipped, some pending.
|
||||
- "Recently Shipped Tracks (2026-06-29)" section at the bottom.
|
||||
|
||||
**Target state:**
|
||||
- **Section 1: Active Queue** — only tracks that are genuinely unblocked and
|
||||
ready to start OR in-progress. Shipped tracks are removed (they're in
|
||||
chronology.md). Each row: `| # | Priority | Track | Status | Blocked By |`
|
||||
(same columns, filtered to active-only).
|
||||
- **Section 2: Standby / Pending Spec** — tracks with spec TBD or pending
|
||||
decision (the backlog). Same columns.
|
||||
- **Section 3: Pointer** — one line:
|
||||
`> Full project history: see [chronology.md](./chronology.md)`
|
||||
- **Delete:** Phase 0-9 sections, Completed/Archived subsections, backlog/
|
||||
follow-up sections that duplicate chronology.md, the "Recently Shipped"
|
||||
section, the "Archived (Closed 2026-06-23)" video analysis section.
|
||||
- **Keep:** the "Editing this file" / archiving convention notes at the
|
||||
bottom (from v1 Phase 4).
|
||||
|
||||
**Migration safety:** the full tracks.md is preserved in git history; the
|
||||
de-gunk is a single commit. If anything is lost,
|
||||
`git show HEAD~1:conductor/tracks.md` recovers it.
|
||||
|
||||
#### Revision 5 — workflow.md maintenance rule
|
||||
|
||||
v2 had no maintenance plan (the root cause of the desync). The new track adds
|
||||
a section to `conductor/workflow.md`:
|
||||
|
||||
**New subsection under "Documentation Refresh Protocol"** (or a new top-level
|
||||
section "Chronology Maintenance"):
|
||||
|
||||
> **Chronology regeneration cadence.** After every track ships (completion
|
||||
> commit + TRACK_COMPLETION report), the implementing agent must run
|
||||
> `uv run python scripts/audit/generate_chronology.py` to regenerate
|
||||
> `conductor/chronology.md`. The regeneration is a single atomic commit
|
||||
> (`docs(chronology): regenerate after <track-id> shipped`). If the
|
||||
> regeneration produces a diff beyond the new row (e.g., status changes on
|
||||
> other rows), the agent must investigate before committing — a status drift
|
||||
> on an unrelated row indicates a stale classifier, not a chronology bug.
|
||||
>
|
||||
> **Quality gate.** `scripts/audit/chronology_quality_gate.py` runs as part
|
||||
> of the regeneration. It fails (exit 1) if >30% of rows are classified as
|
||||
> `Needs Review`. A failing quality gate blocks the regeneration commit.
|
||||
|
||||
This makes regeneration a per-track-shipping obligation, not a one-shot.
|
||||
|
||||
#### Revision 6 — Report(s)
|
||||
|
||||
Two reports:
|
||||
|
||||
1. **`docs/reports/TRACK_COMPLETION_chronology_v2_20260701.md`** — the
|
||||
standard end-of-track report (what was done, files changed, verification
|
||||
results).
|
||||
2. **`docs/reports/CHRONOLOGY_QUALITY_20260701.md`** — the chronology-quality
|
||||
report (new, not in v1). Contents:
|
||||
- Total rows generated + breakdown by status (Active / In Progress /
|
||||
Completed / Abandoned / Superseded / Special / Needs Review)
|
||||
- Confidence distribution (high / medium / low)
|
||||
- The Needs Review queue (list of rows that need manual classification,
|
||||
with the evidence the classifier found)
|
||||
- Comparison vs v1 (row count delta, status-correction count: "N rows
|
||||
changed status vs v1")
|
||||
- The desync gap closed (list of tracks added that were missing from v1)
|
||||
- Classifier heuristics summary (which patterns matched, which were
|
||||
overridden by completion/abort reports)
|
||||
|
||||
The quality report is the evidence artifact — it's what makes this track
|
||||
auditable rather than "trust the script." v1 failed because there was no
|
||||
quality gate and no evidence per row; this report is the fix.
|
||||
|
||||
### 3. Architecture — the generation script + quality gate
|
||||
|
||||
#### `scripts/audit/generate_chronology.py` (rewritten)
|
||||
|
||||
**Inputs:** `conductor/tracks/` + `conductor/archive/` (walked fresh each
|
||||
run); `git log` per folder for commit evidence; `docs/reports/TRACK_COMPLETION_*.md`
|
||||
+ `TRACK_ABORTED_*.md` for override signals.
|
||||
|
||||
**Extraction pipeline (per folder):**
|
||||
1. **Date** — slug date from folder name (regex, unchanged from v1).
|
||||
2. **ID** — folder name (unchanged).
|
||||
3. **Status** — the new classifier (see below), returns
|
||||
`(status, confidence, reason)`.
|
||||
4. **Summary** — rewritten extractor: rejects lines starting with
|
||||
`**Priority:**`, `**Date:**`, `**Initialized:**`, `**Track:**`,
|
||||
`**Parent umbrella:**`, `**Status:**`, `**Confidence:**`; prefers
|
||||
`metadata.json.description` if it's actual prose (not metadata-field
|
||||
text); falls back to first non-heading, non-metadata line of `spec.md`;
|
||||
truncates to 25 words.
|
||||
5. **Folder** — path (unchanged).
|
||||
6. **Range** — `git log --oneline -- <folder>` → first + last SHA + count.
|
||||
|
||||
**The new classifier (`_classify_status`, returning `(status, confidence, reason)`):**
|
||||
|
||||
Evidence sources, in priority order:
|
||||
|
||||
1. **Override signals (highest confidence):**
|
||||
- `TRACK_COMPLETION_*.md` exists in `docs/reports/` matching this track
|
||||
ID → `Completed`, confidence=high, reason="completion report found".
|
||||
- `TRACK_ABORTED_*.md` exists → `Abandoned`, confidence=high,
|
||||
reason="abort report found". (If `state.toml` also says `superseded`,
|
||||
the `Superseded` classification wins — see next row.)
|
||||
- `state.toml` `status = "superseded"` → `Superseded`,
|
||||
confidence=high (overrides the abort-report signal if both exist).
|
||||
2. **Git commit evidence (medium confidence):**
|
||||
- Count work commits (`feat/fix/refactor/perf/test/docs(report):` prefixes)
|
||||
via `git log --oneline -- <folder>`, excluding metadata commits
|
||||
(`conductor(plan):`, `conductor(state):`, `conductor(track):`,
|
||||
`docs(spec):`, `docs(plan):`).
|
||||
- ≥3 work commits → `Completed`, confidence=medium, reason="N work commits".
|
||||
- 1-2 work commits + in `tracks/` → `In Progress`, confidence=medium.
|
||||
- 0 work commits + in `tracks/` → `Active` (spec/plan only),
|
||||
confidence=medium.
|
||||
3. **Directory location (low confidence):**
|
||||
- In `archive/` + no override signal → `Completed`, confidence=low,
|
||||
reason="archived but no completion report".
|
||||
- In `archive/` + 0 commits → `Abandoned`, confidence=low,
|
||||
reason="archived with 0 commits".
|
||||
4. **Fallback:** `Needs Review`, confidence=none,
|
||||
reason="classifier inconclusive".
|
||||
|
||||
**Status enum:** `Active` / `In Progress` / `Completed` / `Abandoned` /
|
||||
`Superseded` / `Special` / `Needs Review` (7 values; v2 had 5, adding
|
||||
`Superseded` + `Needs Review`).
|
||||
|
||||
**Output format:** Markdown table with 6 columns (Date, ID, Status, Summary,
|
||||
Folder, Range) + a **"Needs Review" section** at the bottom listing rows with
|
||||
`Needs Review` status, each with its evidence reason. Sorted newest-first. A
|
||||
preamble header with generation date + row count.
|
||||
|
||||
#### `scripts/audit/chronology_quality_gate.py` (new)
|
||||
|
||||
**Purpose:** detect a broken classifier before the chronology ships.
|
||||
|
||||
**Checks:**
|
||||
- **Needs Review threshold:** if >30% of rows are `Needs Review`, exit 1
|
||||
(the classifier is failing on too many rows).
|
||||
- **Status distribution sanity:** if 0 rows are `Completed`, exit 1 (the
|
||||
classifier is misclassifying everything).
|
||||
- **Summary quality:** if >20% of summaries still contain metadata-field
|
||||
text (`**Priority:**` etc.), exit 1 (the summary extractor is broken).
|
||||
- **Per-row evidence:** every row must have a non-empty `reason` from the
|
||||
classifier; if any row has no reason, exit 1.
|
||||
|
||||
**Modes:** default informational (exits 0, prints report); `--strict` CI
|
||||
gate (exits 1 on any violation). Follows the project's audit-script
|
||||
convention (per `conductor/workflow.md` "Audit Script Policy").
|
||||
|
||||
#### Tests (TDD)
|
||||
|
||||
`tests/test_generate_chronology.py` (rewritten) +
|
||||
`tests/test_chronology_quality_gate.py` (new). Tests for:
|
||||
- The classifier's 7 status values + the evidence priority chain (override
|
||||
signals > git evidence > directory > fallback).
|
||||
- The summary extractor's rejection of metadata-field lines.
|
||||
- The quality gate's 4 checks.
|
||||
- Edge cases: aborted tracks with completion reports (override conflict),
|
||||
tracks with 0 commits, archive folders with no metadata.json.
|
||||
|
||||
### 4. Execution plan structure (phases)
|
||||
|
||||
6 phases, each a checkpoint with atomic per-task commits.
|
||||
|
||||
#### Phase 1: Close out the old track + scaffold the new one
|
||||
- Task 1.1: Update `chronology_20260619/state.toml` →
|
||||
`status = "superseded"`, add `[supersession]` section. Commit.
|
||||
- Task 1.2: Update `chronology_20260619` row in tracks.md (line 64) to
|
||||
"superseded by `chronology_v2_20260701`". Commit.
|
||||
- Task 1.3: Archive `conductor/tracks/chronology_20260619/` →
|
||||
`conductor/archive/chronology_20260619/`. Commit.
|
||||
- Task 1.4: Update `superpowers_review_20260619/state.toml` `[blocked_by]` —
|
||||
remove `chronology_20260619` entirely. Commit.
|
||||
- Task 1.5: Create `conductor/tracks/chronology_v2_20260701/` with
|
||||
`spec.md`, `metadata.json`, `state.toml`, `plan.md`. Commit.
|
||||
|
||||
#### Phase 2: TDD the classifier + quality gate (Red)
|
||||
- Task 2.1: Write `tests/test_generate_chronology.py` — tests for the
|
||||
7-status classifier, evidence priority chain, summary extractor. Red.
|
||||
- Task 2.2: Write `tests/test_chronology_quality_gate.py` — tests for the
|
||||
4 quality-gate checks. Red.
|
||||
|
||||
#### Phase 3: Implement the classifier + quality gate (Green)
|
||||
- Task 3.1: Rewrite `scripts/audit/generate_chronology.py` — the new
|
||||
`_classify_status` returning `(status, confidence, reason)`, the
|
||||
rewritten summary extractor, the git-history evidence pipeline. Green.
|
||||
- Task 3.2: Create `scripts/audit/chronology_quality_gate.py` — the 4
|
||||
checks + `--strict` mode. Green.
|
||||
|
||||
#### Phase 4: Regenerate chronology.md + write the quality report
|
||||
- Task 4.1: Run the generator against the current filesystem. Capture
|
||||
output to `conductor/chronology.md` (replacing v1). Commit.
|
||||
- Task 4.2: Run the quality gate. If it fails, iterate on the classifier
|
||||
(back to Phase 3) until it passes. Commit the passing state.
|
||||
- Task 4.3: Write `docs/reports/CHRONOLOGY_QUALITY_20260701.md` — the
|
||||
quality report. Commit.
|
||||
|
||||
#### Phase 5: De-gunk tracks.md + add workflow.md maintenance rule
|
||||
- Task 5.1: Restructure `conductor/tracks.md` — remove shipped/completed
|
||||
rows from the active queue, remove Phase 0-9 history sections, remove
|
||||
backlog/follow-up sections that duplicate chronology.md, add the pointer
|
||||
to chronology.md, keep the "Editing this file" notes. Single commit.
|
||||
- Task 5.2: Add the "Chronology Maintenance" section to
|
||||
`conductor/workflow.md` — the regeneration cadence + quality gate
|
||||
obligation. Commit.
|
||||
|
||||
#### Phase 6: Verification + end-of-track report
|
||||
- Task 6.1: Run the quality gate `--strict` mode. Confirm exit 0. Commit.
|
||||
- Task 6.2: Verify the Needs Review queue is empty or small (the user
|
||||
reviews any remaining rows). Commit.
|
||||
- Task 6.3: Write
|
||||
`docs/reports/TRACK_COMPLETION_chronology_v2_20260701.md`. Commit.
|
||||
- Task 6.4: User sign-off (the final gate — same as v1's Phase 10, but
|
||||
this time the quality report + evidence per row makes it actionable).
|
||||
|
||||
### Commit strategy
|
||||
|
||||
- Per-task atomic commits (no batching).
|
||||
- Git notes per commit (task summary).
|
||||
- Phase checkpoints after each phase (per the workflow protocol).
|
||||
|
||||
## Verification Criteria
|
||||
|
||||
1. `conductor/chronology.md` exists with one row per track folder (tracks/
|
||||
+ archive/), sorted newest-first, 6 columns, generated from the current
|
||||
filesystem (no 2026-06-19 snapshot pin).
|
||||
2. Every row's status is backed by git-history evidence (not
|
||||
`metadata.json.status`); the evidence `reason` is non-empty for every
|
||||
row.
|
||||
3. No summary contains metadata-field text (`**Priority:**`, `**Date:**`,
|
||||
`**Initialized:**`, `**Track:**`, `**Parent umbrella:**`,
|
||||
`**Status:**`, `**Confidence:**`).
|
||||
4. `scripts/audit/chronology_quality_gate.py --strict` exits 0.
|
||||
5. `conductor/tracks.md` contains only the active queue + standby/pending +
|
||||
a pointer to chronology.md + the "Editing this file" notes. No Phase 0-9
|
||||
history sections, no shipped-track rows in the active queue.
|
||||
6. `conductor/workflow.md` contains the "Chronology Maintenance" section
|
||||
(regeneration cadence + quality gate obligation).
|
||||
7. `docs/reports/CHRONOLOGY_QUALITY_20260701.md` exists with the status
|
||||
distribution, confidence distribution, Needs Review queue, v1
|
||||
comparison, desync gap list, and heuristics summary.
|
||||
8. `docs/reports/TRACK_COMPLETION_chronology_v2_20260701.md` exists.
|
||||
9. `chronology_20260619` is archived (in `conductor/archive/`) with
|
||||
`status = "superseded"` in its state.toml.
|
||||
10. `superpowers_review_20260619/state.toml` `[blocked_by]` no longer
|
||||
contains `chronology_20260619`.
|
||||
11. `tests/test_generate_chronology.py` +
|
||||
`tests/test_chronology_quality_gate.py` pass.
|
||||
12. User sign-off recorded in the TRACK_COMPLETION report.
|
||||
|
||||
## Risks
|
||||
|
||||
- **R1 (medium):** The git-history classifier may still misclassify some
|
||||
edge cases (e.g., tracks with `conductor(checkpoint):` commits only).
|
||||
Mitigation: the Needs Review queue surfaces these for manual
|
||||
classification; the quality gate fails if >30% are Needs Review.
|
||||
- **R2 (medium):** The tracks.md de-gunk may accidentally remove a row
|
||||
that's still active. Mitigation: the full tracks.md is preserved in git
|
||||
history; recovery is `git show HEAD~1:conductor/tracks.md`.
|
||||
- **R3 (low):** The workflow.md maintenance rule may not be followed by
|
||||
future agents. Mitigation: the rule is in the operational workflow doc
|
||||
that agents read at session start; the quality gate catches a desync
|
||||
when the next regeneration runs.
|
||||
|
||||
## Out of Scope
|
||||
|
||||
- Fixing or executing `superpowers_review_20260619` (just unblocking it).
|
||||
- Changing how `metadata.json.status` is maintained going forward.
|
||||
- Archiving the 66 folders in `conductor/tracks/` that are already shipped.
|
||||
- Renaming or restructuring `conductor/archive/`.
|
||||
- A broader `workflow.md` review for other stale rules.
|
||||
- The `superpowers_review_20260619` track's execution.
|
||||
+192
@@ -0,0 +1,192 @@
|
||||
# Design: MMA Quarantine + RAG Test Decoupling
|
||||
|
||||
**Date:** 2026-07-01
|
||||
**Status:** Draft (pending user review)
|
||||
**Scope:** Two surgical interventions. (1) Quarantine the MMA automation engine behind a config flag so it stops consuming test-suite time and stops breaking when adjacent code changes. (2) Decouple the RAG tests from the live_gui subprocess + chromadb file locks so the default test batch stops bleeding on RAG.
|
||||
|
||||
**Out of scope:** The discussion/session system redesign (owned by the nagent research track). Full removal of MMA code (deferred to a follow-up track if quarantine maintenance becomes painful). Any change to the RAG algorithm itself (`index_file`, `search`, chunking — unchanged). The `conductor/` track system (the `conductor_tech_lead.py` / `project_manager.py` / `dag_engine.py` shared-types layer is preserved as load-bearing).
|
||||
|
||||
## Context
|
||||
|
||||
### Why this work (grounded in git history, not track docs)
|
||||
|
||||
The last 10 days of git history (2026-06-20 through 2026-06-30) show two subsystems consuming disproportionate debugging time:
|
||||
|
||||
**RAG test debugging churn (2026-06-27):** ~10 commits chasing RAG test failures — `_get_chromadb()` NameError in dim check, file-lock dim-check failures, silent `index_file` no-ops on missing files, session-scoped subprocess pollution, hotpatched state instead of project-switch. Two "ADDENDUM" reports because the first root-cause diagnosis was wrong. The commits are fighting the *environment* (chromadb file locks under the live_gui subprocess, CWD drift across the spawn boundary, lazy global teardown/rebuild), not the RAG algorithm.
|
||||
|
||||
**MMA concurrent tracks sim fix (2026-06-27):** `fix_mma_concurrent_tracks_sim_20260627` — 5 fixes to `mock_concurrent_mma.py` (session_id fallback removal, epic branch catch-all, sprint routing by prompt content) plus a `refresh_from_project` task that was overwriting `self.tracks`. The mock itself is brittle; the production engine is brittle in the same shape.
|
||||
|
||||
The user's direction: sunset MMA constructively (quarantine, not delete — full removal is a follow-up if quarantine maintenance hurts), keep RAG but make the tests sane.
|
||||
|
||||
### The shared-types finding (why this is quarantine, not removal)
|
||||
|
||||
`src/mma.py` is a shared types module, not just the MMA engine. Non-MMA code depends on it:
|
||||
- `thinking_parser.py` → `ThinkingSegment` (thinking-trace parsing, used in every AI turn — NOT an MMA feature)
|
||||
- `project_manager.py` → `TrackState`, `EMPTY_TRACK_STATE` (the conductor track system — persists `conductor/tracks/<id>/state.toml`)
|
||||
- `models.py` → `TrackMetadata` (re-exported as legacy `Metadata` alias)
|
||||
- `dag_engine.py` → `Ticket`
|
||||
- `conductor_tech_lead.py` → `Ticket`, `TrackDAG`
|
||||
|
||||
`dag_engine.py` is shared between the MMA loop AND `conductor_tech_lead.py` (the Tier 2 tech lead system). Full removal would require migrating `ThinkingSegment` and `TrackState`/`TrackMetadata` out of `mma.py` first — a mutation of the shared-types module that risks the conductor track system. That work is deferred to a follow-up track.
|
||||
|
||||
### The RAG fragility root cause
|
||||
|
||||
The RAG algorithm is ~50 lines (`index_file` + `search` + chunking). The other ~250 lines of `rag_engine.py` are defensive scaffolding for the test/subprocess environment:
|
||||
- `_validate_collection_dim_result` uses `shutil.rmtree(ignore_errors=True)` to survive Windows file locks held by the live_gui subprocess
|
||||
- `index_file` has a CWD-fallback band-aid for path resolution drift across the spawn boundary
|
||||
- `_sync_rag_engine` is called from 6 sites in `app_controller.py` (files change, project switch, session reset, RAG toggle) — 6 race surfaces
|
||||
- Lazy module globals (`_CHROMADB`, `_GOOGLE_GENAI`, `_SENTENCE_TRANSFORMERS`) tear down and rebuild per-test with no guarantee of state
|
||||
|
||||
The RAG *tests* are testing "does RAG survive the live_gui subprocess lifecycle + chromadb file locks + CWD drift + lazy global teardown/rebuild" — not "does RAG retrieve relevant chunks." That's the bleed.
|
||||
|
||||
## Section 1: MMA Quarantine
|
||||
|
||||
### Mechanism
|
||||
|
||||
Config flag `mma.enabled`, default `false`, in `[ai_settings.toml]` (the per-project settings file, not `manual_slop.toml` which is project-static config). Per `conductor/code_styleguides/feature_flags.md` §2: this is a persistent preference (off by default, not recoverable by a single regenerate command), so config flag + GUI checkbox is the correct pattern — not file presence, not env-var-only.
|
||||
|
||||
### What the flag gates
|
||||
|
||||
#### 1.1 `app_controller.py` (~20 sites)
|
||||
|
||||
State fields initialized to empty/zero-init when `mma.enabled == false`:
|
||||
- `self.engines: Dict[str, ConductorEngine] = {}` (stays empty)
|
||||
- `self.mma_streams: Dict[str, str] = {}` (stays empty)
|
||||
- `self.mma_step_mode: bool = False` (stays False)
|
||||
- `self.mma_tier_usage: Dict[str, Metadata] = {...}` (stays zero-init)
|
||||
- `self.tracks: list[Metadata] = []` (stays empty — note: this is the MMA track list, NOT the conductor track system; `project_manager.get_all_tracks` is still callable for the conductor UI if needed)
|
||||
|
||||
Engine methods return early when the flag is off:
|
||||
- `start_mma` / engine-start paths — return early, no `ConductorEngine` instantiation
|
||||
- `approve_step` / `approve_spawn` — return early, no-op
|
||||
- abort / reset paths — clear the (already empty) state
|
||||
|
||||
The `multi_agent_conductor` import becomes lazy: imported inside the gated methods only, gated on the flag. When the flag is off, the module is never imported at runtime (reduces import-graph weight).
|
||||
|
||||
The `rag_engine` sync paths are untouched — RAG is a separate subsystem (Section 2).
|
||||
|
||||
#### 1.2 `gui_2.py` (12 render functions + dashboard window + modals)
|
||||
|
||||
Every `render_mma_*` function and `render_task_dag_panel` checks `if not app.controller.mma_enabled: return` at the top:
|
||||
- `render_mma_dashboard` (gui_2.py:6610)
|
||||
- `render_mma_modals` (gui_2.py:6658)
|
||||
- `render_mma_track_summary` (gui_2.py:6757)
|
||||
- `render_mma_epic_planner` (gui_2.py:6800)
|
||||
- `render_mma_conductor_setup` (gui_2.py:6818)
|
||||
- `render_mma_track_browser` (gui_2.py:6837)
|
||||
- `render_mma_global_controls` (gui_2.py:6884)
|
||||
- `render_mma_usage_section` (gui_2.py:6924)
|
||||
- `render_mma_ticket_editor` (gui_2.py:7003)
|
||||
- `render_mma_agent_streams` (gui_2.py:7043)
|
||||
- `render_task_dag_panel` (gui_2.py:7351)
|
||||
- `render_mma_focus_selector` (gui_2.py:7570)
|
||||
|
||||
The MMA Dashboard window is not registered in the panel registry when the flag is off (gui_2.py:1995 `_render_window_if_open("MMA Dashboard", ...)` — gated on flag). The approval modals (MMA Step Approval, MMA Spawn Approval) no-op. The ~600 lines of render code stay in-tree but are dead at runtime.
|
||||
|
||||
#### 1.3 `multi_agent_conductor.py`
|
||||
|
||||
Kept in-tree. Imported lazily only inside the gated `app_controller` methods. `ConductorEngine` / `WorkerPool` never instantiate when the flag is off.
|
||||
|
||||
#### 1.4 What stays active (shared types, NOT gated)
|
||||
|
||||
- `mma.py` — `Ticket`, `Track`, `TrackState`, `TrackMetadata`, `WorkerContext`, `ThinkingSegment` — load-bearing for non-MMA code
|
||||
- `dag_engine.py` — `TrackDAG`, `ExecutionEngine` — used by `conductor_tech_lead.py`
|
||||
- `mma_prompts.py` — used by `ai_client.py`, `conductor_tech_lead.py`, `orchestrator_pm.py` (verify whether these usages are MMA-specific or general during implementation; if MMA-specific, gate; if general, leave)
|
||||
- `mma.py` / `dag_engine.py` / `mma_prompts.py` imports in non-MMA consumers remain unchanged
|
||||
|
||||
#### 1.5 GUI surface
|
||||
|
||||
A single `[ ] Enable MMA (deprecated, quarantined)` checkbox in AI Settings. The full MMA dashboard is hidden when the flag is off. The checkbox is the only MMA UI surface. Per `feature_flags.md` §2: the GUI checkbox is a projection of the config file; the config file is the source of truth.
|
||||
|
||||
#### 1.6 Tests (env-gated, opt-in)
|
||||
|
||||
MMA tests gated behind `SLOP_MMA_TESTS=1` env var via `@pytest.mark.skipif`. Skip reason documents: "MMA is quarantined; run with `SLOP_MMA_TESTS=1` to enable." This is the *test* gate — separate from the *runtime* config flag, per `feature_flags.md` §6 (layered flags: file presence / config for runtime, env var for test opt-in).
|
||||
|
||||
Affected test files (the `test_mma_*`, `test_concurrent_*`, `test_conductor_*`, `test_dag_*`, `test_parallel_*`, `test_worker_*`, `test_visual_mma*`, `test_*sim*mma*` set):
|
||||
- `test_mma_agent_focus_phase1.py`, `test_mma_agent_focus_phase3.py`
|
||||
- `test_mma_approval_indicators.py`, `test_mma_concurrent_tracks_sim.py`, `test_mma_concurrent_tracks_stress_sim.py`
|
||||
- `test_mma_dashboard_refresh.py`, `test_mma_dashboard_streams.py`, `test_mma_models.py`, `test_mma_node_editor.py`
|
||||
- `test_mma_orchestration_gui.py`, `test_mma_prompts.py`, `test_mma_skeleton.py`, `test_mma_step_mode_sim.py`
|
||||
- `test_mma_ticket_actions.py`, `test_mma_tier_usage_reset_fix.py`, `test_mma_usage_stats.py`
|
||||
- `test_mma_concurrent_tracks_sim.py`, `test_mma_concurrent_tracks_stress_sim.py`
|
||||
- `test_reset_session_clears_mma_and_rag.py` (split: MMA portion gated, RAG portion stays — see Section 2)
|
||||
- `test_visual_mma.py`, `test_visual_sim_mma_v2.py`
|
||||
- `mock_concurrent_mma.py` (test helper, not a test file — kept as-is for opt-in runs)
|
||||
- `test_conductor_abort_event.py`, `test_conductor_api_hook_integration.py`, `test_conductor_engine_abort.py`, `test_conductor_engine_v2.py`, `test_conductor_tech_lead.py`
|
||||
- `test_dag_engine.py`, `test_gui_dag_beads.py`, `test_perf_dag.py`, `test_task_dag_popout_sim.py`
|
||||
- `test_parallel_execution.py`, `test_run_worker_lifecycle_abort.py`
|
||||
|
||||
Note: `test_conductor_tech_lead.py` and `test_dag_engine.py` may test the shared-types layer (not the MMA engine). During implementation, classify each: if it tests the shared `dag_engine`/`conductor_tech_lead` layer (not the MMA engine), it stays in the default batch. If it tests the MMA engine specifically, it's gated. The classifier: does the test import or instantiate `multi_agent_conductor.ConductorEngine` / `WorkerPool`? If yes → gated. If no → stays.
|
||||
|
||||
## Section 2: RAG Test Decoupling
|
||||
|
||||
### Mechanism
|
||||
|
||||
Three-tier test classification. The RAG algorithm is unchanged. The mock provider already exists (`rag_engine.py`: `provider == 'mock'` short-circuits chromadb). The tests are reclassified, not rewritten from scratch.
|
||||
|
||||
### Tier 1 — Unit tests (default batch, no chromadb, no subprocess)
|
||||
|
||||
Test the 50-line algorithm in isolation against the mock provider:
|
||||
- `test_rag_chunk.py` — `RAGChunk` dataclass (already isolated; stays)
|
||||
- `test_rag_engine.py` — rewrite to use mock provider exclusively; test `index_file` no-op behavior on mock, `search` returns `[]` on mock, chunking logic (`_chunk_text`, `_chunk_code_result`), `is_empty` semantics, `RAGChunk.from_dict`/`to_dict` round-trip
|
||||
- `test_rag_engine_result.py` — Result wrapping (already isolated; stays)
|
||||
- `test_rag_sync_none_error.py` — sync error handling (mock the engine, not chromadb)
|
||||
|
||||
### Tier 2 — Controller lifecycle tests (default batch, mock RAGEngine, no chromadb)
|
||||
|
||||
Test `app_controller.py`'s RAG lifecycle wiring using a mock/stub `RAGEngine`:
|
||||
- `test_rag_engine_ready_status_bug.py` — mock the `rag_engine` on `AppController`, test the ready-status state machine
|
||||
- `test_rag_gui_presence.py` — test the GUI panel renders when RAG is enabled/disabled (no real engine)
|
||||
- `test_sync_rag_engine_coalescing.py` — test `_sync_rag_engine` coalescing logic with a mock engine (controller logic, not RAG logic)
|
||||
- `test_reset_session_clears_mma_and_rag.py` — RAG portion stays (test reset clears the RAG state fields with a mock engine); MMA portion gated per Section 1.6
|
||||
|
||||
### Tier 3 — Integration tests (opt-in, env-gated, real chromadb / live_gui)
|
||||
|
||||
Gated behind `SLOP_RAG_INTEGRATION=1` env var via `@pytest.mark.skipif`. Skip reason: "Integration test requires real chromadb + live_gui subprocess; run with `SLOP_RAG_INTEGRATION=1` to enable." These are the tests that kept breaking — now opt-in, not default batch:
|
||||
- `test_rag_phase4_final_verify.py` — the 3-ADDENDUM fragile one
|
||||
- `test_rag_phase4_stress.py` — stress test
|
||||
- `test_rag_visual_sim.py` — live_gui visual sim
|
||||
- `test_rag_integration.py` — full integration
|
||||
|
||||
### What this kills
|
||||
|
||||
The recurring bleed. The default test batch no longer touches chromadb file locks, the live_gui subprocess RAG state, or CWD drift. When adjacent code changes, Tier 1+2 verify RAG logic + controller wiring in isolation. Tier 3 only runs on explicit opt-in (e.g., before a release, or when actively working on RAG).
|
||||
|
||||
### What this preserves
|
||||
|
||||
The RAG feature itself. `RAGEngine` is unchanged. The mock provider already exists. The integration tests still exist — just not in the default batch.
|
||||
|
||||
## Verification
|
||||
|
||||
### MMA quarantine
|
||||
- `mma.enabled = false` (default): MMA dashboard does not render; engine methods no-op; `multi_agent_conductor` not imported at runtime; MMA tests skipped (not failed)
|
||||
- `mma.enabled = true`: MMA dashboard renders; engine starts; MMA tests run when `SLOP_MMA_TESTS=1`
|
||||
- No regression in non-MMA code (thinking_parser, project_manager, models, conductor_tech_lead — shared types intact)
|
||||
|
||||
### RAG test decoupling
|
||||
- Default batch: Tier 1+2 run in milliseconds, no chromadb, no subprocess, no file locks
|
||||
- `SLOP_RAG_INTEGRATION=1`: Tier 3 runs (the previously-fragile tests)
|
||||
- RAG feature functional end-to-end when `rag.enabled = true` in config (unchanged)
|
||||
|
||||
## Risks
|
||||
|
||||
- **Lazy import correctness:** the `multi_agent_conductor` lazy import inside gated methods must not introduce a circular import or a startup-time import when the flag is off. Verify via `scripts/audit_main_thread_imports.py` after implementation.
|
||||
- **Shared-types boundary:** `mma.py` / `dag_engine.py` / `mma_prompts.py` must remain importable by non-MMA consumers. The flag gates the *engine*, not the *types*. If `mma_prompts.py` usages in `ai_client.py` / `conductor_tech_lead.py` / `orchestrator_pm.py` are MMA-specific (e.g., prompt templates only used by the engine), gate them; if general, leave. Classify during implementation.
|
||||
- **Test classifier drift:** the MMA test classifier ("does it import `ConductorEngine` / `WorkerPool`?") must be applied consistently. A test that tests shared `dag_engine` types but not the engine stays in the default batch.
|
||||
- **Quarantine maintenance cost:** if the quarantined code rots (imports drift, shared types change underneath it), follow-up full removal (Option A) becomes necessary. The quarantine is the lower-risk path *now*; it's not a permanent commitment.
|
||||
|
||||
## Out of Scope
|
||||
|
||||
- The discussion/session system redesign (nagent research track)
|
||||
- Full removal of MMA code (follow-up track if quarantine maintenance hurts)
|
||||
- RAG algorithm changes (`index_file`, `search`, chunking — unchanged)
|
||||
- The `conductor/` track system (`conductor_tech_lead.py` / `project_manager.py` / `dag_engine.py` shared-types layer preserved)
|
||||
- Migrating `ThinkingSegment` / `TrackState` / `TrackMetadata` out of `mma.py` (follow-up to full removal)
|
||||
|
||||
## See Also
|
||||
|
||||
- `conductor/code_styleguides/feature_flags.md` — the config-flag-vs-file-presence decision tree (§2: persistent preference → config flag + GUI checkbox)
|
||||
- `docs/guide_mma.md` — the MMA engine architecture (the thing being quarantined)
|
||||
- `docs/guide_rag.md` — the RAG subsystem architecture (unchanged)
|
||||
- `conductor/tracks/fix_mma_concurrent_tracks_sim_20260627/` — the recent MMA brittleness
|
||||
- Git history 2026-06-27 — the RAG test debugging churn (10 commits, 2 ADDENDUM reports)
|
||||
Reference in New Issue
Block a user