conductor(plan): UI Polish track - 5 phases, design spec + impl plan
This commit is contained in:
@@ -67,6 +67,24 @@ This file tracks all major tracks for the project. Each track has its own detail
|
||||
|
||||
---
|
||||
|
||||
## Phase 8: UI Polish
|
||||
|
||||
*Initialized: 2026-06-03*
|
||||
|
||||
User review surfaced five outstanding UI issues, each previously attempted without success. This track addresses them as five independent phases with their own TDD cycles and atomic commits.
|
||||
|
||||
1. [ ] **Track: UI Polish (Five Issues)**
|
||||
*Spec: [./../../docs/superpowers/specs/2026-06-03-ui-polish-design.md](./../../docs/superpowers/specs/2026-06-03-ui-polish-design.md)*
|
||||
*Plan: [./../../docs/superpowers/plans/2026-06-03-ui-polish.md](./../../docs/superpowers/plans/2026-06-03-ui-polish.md)*
|
||||
*Goal: Resolve five long-standing UI issues:
|
||||
- Phase 1: GFM markdown table rendering (pre-processor into `src/markdown_table.py`, wire into `MarkdownRenderer.render`).
|
||||
- Phase 2: Widen the `Keep Pairs` numeric input next to `Truncate` in the discussion panel (`gui_2.py:3829`, width 80 -> 140, switch to `drag_int`).
|
||||
- Phase 3: Fix `Refresh Registry` button in Log Management — currently instantiates `LogRegistry` without calling `load_registry()` so the displayed table never reflects on-disk state (`gui_2.py:1675`).
|
||||
- Phase 4: Add `Vendor State` tab to Operations Hub — at-a-glance provider/model, context-window utilization, cache hit rate, last error class, vendor quota (new `src/vendor_state.py` aggregator + `controller.vendor_quota` field + `ai_client` wire-up).
|
||||
- Phase 5: Files & Media > Files directory-grouped tree (re-use `aggregate.group_files_by_dir`, mirror `render_context_files_table` collapsible-node style).*
|
||||
|
||||
---
|
||||
|
||||
## Hot Reload Feature
|
||||
|
||||
1. [x] **Track: Hot Reload Python Codebase (Phase 2)**
|
||||
|
||||
@@ -0,0 +1,985 @@
|
||||
# UI Polish Track — Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Resolve five outstanding UI quality-of-life issues: GFM table rendering, Keep Pairs input width, Log Management refresh bug, Operations Hub vendor-state panel, and Files & Media directory tree grouping.
|
||||
|
||||
**Architecture:** Pure rendering-layer changes. No new infrastructure, no threading, no provider API changes. Each phase is a self-contained sub-track with its own Red/Green/Commit cycle.
|
||||
|
||||
**Tech Stack:** Python 3.11+, imgui-bundle (`imgui_md`, `imgui.begin_table`, `imgui.tree_node_ex`), pytest, the `live_gui` fixture from `tests/conftest.py`.
|
||||
|
||||
**Spec:** `docs/superpowers/specs/2026-06-03-ui-polish-design.md`
|
||||
|
||||
---
|
||||
|
||||
## File Structure
|
||||
|
||||
| File | Status | Responsibility |
|
||||
|------|--------|----------------|
|
||||
| `src/markdown_table.py` | **create** | Pure GFM table parser. No ImGui imports. |
|
||||
| `src/markdown_helper.py` | modify | Insert table interceptor in `MarkdownRenderer.render()`. |
|
||||
| `src/vendor_state.py` | **create** | Pure vendor-state aggregator. No ImGui imports. |
|
||||
| `src/app_controller.py` | modify | Add `vendor_quota` field + `set_vendor_quota()` callback. |
|
||||
| `src/ai_client.py` | modify | Wire `set_vendor_quota` into quota-bearing response paths. |
|
||||
| `src/gui_2.py` | modify | Five surgical edits (one per phase). |
|
||||
| `tests/test_markdown_table.py` | **create** | Parser unit tests. |
|
||||
| `tests/test_markdown_table_render.py` | **create** | live_gui render tests. |
|
||||
| `tests/test_vendor_state.py` | **create** | Aggregator unit tests. |
|
||||
| `tests/test_vendor_state_render.py` | **create** | live_gui render tests. |
|
||||
| `tests/test_log_management_refresh.py` | **create** | live_gui button-press test. |
|
||||
| `tests/test_files_and_media_tree.py` | **create** | live_gui directory-grouping test. |
|
||||
|
||||
---
|
||||
|
||||
## Conventions
|
||||
|
||||
- **1-space indentation** for all Python (per `conductor/code_styleguides/python.md`).
|
||||
- **No comments** in source files (per `AGENTS.md`).
|
||||
- All public functions get strict type hints.
|
||||
- New SDM tags `[C: ...]` and `[M: ...]` on every new public function/method.
|
||||
- Run `uv run python scripts/check_imgui_scopes.py` after every `gui_2.py` edit.
|
||||
|
||||
---
|
||||
|
||||
# Phase 1 — Markdown Table Pre-Processor
|
||||
|
||||
### Task 1.1: GFM Table Parser — First Failing Test
|
||||
|
||||
**Files:**
|
||||
- Create: `tests/test_markdown_table.py`
|
||||
- Create: `src/markdown_table.py` (stub with `parse_tables() -> list[TableBlock]` returning `[]`)
|
||||
|
||||
- [ ] **Step 1: Stage current state**
|
||||
|
||||
```powershell
|
||||
git add .
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Write the failing test**
|
||||
|
||||
```python
|
||||
# tests/test_markdown_table.py
|
||||
from src.markdown_table import parse_tables, TableBlock
|
||||
|
||||
def test_parses_simple_two_column_table():
|
||||
text = (
|
||||
"| Name | Type |\n"
|
||||
"|-------|------|\n"
|
||||
"| foo | int |\n"
|
||||
"| bar | str |\n"
|
||||
)
|
||||
blocks = parse_tables(text)
|
||||
assert len(blocks) == 1
|
||||
block = blocks[0]
|
||||
assert block.headers == ["Name", "Type"]
|
||||
assert block.rows == [["foo", "int"], ["bar", "str"]]
|
||||
|
||||
def test_ignores_tables_inside_code_fence():
|
||||
text = (
|
||||
"```\n"
|
||||
"| not | a table |\n"
|
||||
"| --- | ------- |\n"
|
||||
"| x | y |\n"
|
||||
"```\n"
|
||||
)
|
||||
assert parse_tables(text) == []
|
||||
|
||||
def test_returns_empty_for_plain_markdown():
|
||||
text = "# Heading\n\nSome **bold** text.\n"
|
||||
assert parse_tables(text) == []
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Run test to verify it fails**
|
||||
|
||||
Run: `uv run pytest tests/test_markdown_table.py -v`
|
||||
Expected: FAIL with `ImportError` or `AttributeError: module 'src.markdown_table' has no attribute 'parse_tables'`.
|
||||
|
||||
- [ ] **Step 4: Stub the module so the import succeeds**
|
||||
|
||||
```python
|
||||
# src/markdown_table.py
|
||||
from dataclasses import dataclass
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class TableBlock:
|
||||
headers: list[str]
|
||||
rows: list[list[str]]
|
||||
span: tuple[int, int]
|
||||
|
||||
def parse_tables(text: str) -> list[TableBlock]:
|
||||
return []
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Run test, confirm it still fails (logic not implemented)**
|
||||
|
||||
Run: `uv run pytest tests/test_markdown_table.py -v`
|
||||
Expected: FAIL on `assert block.headers == ["Name", "Type"]`.
|
||||
|
||||
- [ ] **Step 6: Commit (Red)**
|
||||
|
||||
```powershell
|
||||
git add tests/test_markdown_table.py src/markdown_table.py
|
||||
git commit -m "test(markdown): add GFM table parser failing tests"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 1.2: GFM Table Parser — Implementation
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/markdown_table.py`
|
||||
|
||||
- [ ] **Step 1: Implement parse_tables() to pass all tests**
|
||||
|
||||
```python
|
||||
# src/markdown_table.py
|
||||
import re
|
||||
from dataclasses import dataclass
|
||||
|
||||
_TABLE_SEPARATOR = re.compile(r"^\|?\s*:?-{2,}:?\s*(\|\s*:?-{2,}:?\s*)+\|?\s*$")
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class TableBlock:
|
||||
headers: list[str]
|
||||
rows: list[list[str]]
|
||||
span: tuple[int, int]
|
||||
[C: src/markdown_helper.py:MarkdownRenderer.render]
|
||||
|
||||
def _split_row(line: str) -> list[str]:
|
||||
line = line.strip()
|
||||
if line.startswith("|"): line = line[1:]
|
||||
if line.endswith("|"): line = line[:-1]
|
||||
return [c.strip() for c in line.split("|")]
|
||||
|
||||
def _is_table_at(lines: list[str], i: int) -> bool:
|
||||
if i + 1 >= len(lines): return False
|
||||
if "|" not in lines[i]: return False
|
||||
return bool(_TABLE_SEPARATOR.match(lines[i + 1]))
|
||||
|
||||
def parse_tables(text: str) -> list[TableBlock]:
|
||||
lines = text.splitlines()
|
||||
in_fence = False
|
||||
blocks: list[TableBlock] = []
|
||||
i = 0
|
||||
while i < len(lines):
|
||||
line = lines[i]
|
||||
if line.strip().startswith("```"):
|
||||
in_fence = not in_fence
|
||||
i += 1
|
||||
continue
|
||||
if in_fence:
|
||||
i += 1
|
||||
continue
|
||||
if _is_table_at(lines, i):
|
||||
headers = _split_row(lines[i])
|
||||
j = i + 2
|
||||
rows: list[list[str]] = []
|
||||
while j < len(lines) and "|" in lines[j] and not _TABLE_SEPARATOR.match(lines[j]):
|
||||
rows.append(_split_row(lines[j]))
|
||||
j += 1
|
||||
blocks.append(TableBlock(headers=headers, rows=rows, span=(i, j)))
|
||||
i = j
|
||||
continue
|
||||
i += 1
|
||||
return blocks
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run test, confirm it passes**
|
||||
|
||||
Run: `uv run pytest tests/test_markdown_table.py -v`
|
||||
Expected: 3 passed.
|
||||
|
||||
- [ ] **Step 3: Commit (Green)**
|
||||
|
||||
```powershell
|
||||
git add src/markdown_table.py
|
||||
git commit -m "feat(markdown): implement GFM table parser"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 1.3: Table Renderer — live_gui Test
|
||||
|
||||
**Files:**
|
||||
- Create: `tests/test_markdown_table_render.py`
|
||||
- Modify: `src/markdown_table.py` (add `render_table(block: TableBlock) -> None`)
|
||||
|
||||
- [ ] **Step 1: Write the failing test**
|
||||
|
||||
```python
|
||||
# tests/test_markdown_table_render.py
|
||||
from imgui_bundle import imgui
|
||||
from src.markdown_table import parse_tables, render_table
|
||||
|
||||
def test_render_table_creates_imGui_table(live_gui):
|
||||
app = live_gui
|
||||
text = "| A | B |\n|---|---|\n| 1 | 2 |\n"
|
||||
blocks = parse_tables(text)
|
||||
assert len(blocks) == 1
|
||||
imgui.begin("test_window")
|
||||
try:
|
||||
render_table(blocks[0])
|
||||
finally:
|
||||
imgui.end()
|
||||
assert imgui.get_io().font_default is not None
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run test, confirm it fails (no render_table function)**
|
||||
|
||||
Run: `uv run pytest tests/test_markdown_table_render.py -v`
|
||||
Expected: FAIL with `ImportError: cannot import name 'render_table'`.
|
||||
|
||||
- [ ] **Step 3: Add the render_table function stub to src/markdown_table.py**
|
||||
|
||||
```python
|
||||
# append to src/markdown_table.py
|
||||
from imgui_bundle import imgui
|
||||
|
||||
def render_table(block: TableBlock) -> None:
|
||||
pass
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run test, confirm it passes (trivial stub)**
|
||||
|
||||
Run: `uv run pytest tests/test_markdown_table_render.py -v`
|
||||
Expected: PASS.
|
||||
|
||||
- [ ] **Step 5: Commit (Red-Green trivial)**
|
||||
|
||||
```powershell
|
||||
git add tests/test_markdown_table_render.py src/markdown_table.py
|
||||
git commit -m "test(markdown): scaffold render_table test with trivial impl"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 1.4: Table Renderer — Real Implementation
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/markdown_table.py` — replace the trivial `render_table` with a real implementation.
|
||||
|
||||
- [ ] **Step 1: Replace render_table with a real implementation**
|
||||
|
||||
```python
|
||||
def render_table(block: TableBlock) -> None:
|
||||
n_cols = len(block.headers)
|
||||
if n_cols == 0: return
|
||||
flags = imgui.TableFlags_.borders | imgui.TableFlags_.row_bg | imgui.TableFlags_.resizable
|
||||
if not imgui.begin_table("md_table", n_cols, flags):
|
||||
return
|
||||
imgui.table_headers_row()
|
||||
for h in block.headers:
|
||||
imgui.table_next_column()
|
||||
imgui.text(h)
|
||||
for row in block.rows:
|
||||
imgui.table_next_row()
|
||||
for c in row:
|
||||
imgui.table_next_column()
|
||||
imgui.text(c)
|
||||
imgui.end_table()
|
||||
[C: src/markdown_helper.py:MarkdownRenderer.render]
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run all markdown_table tests**
|
||||
|
||||
Run: `uv run pytest tests/test_markdown_table.py tests/test_markdown_table_render.py -v`
|
||||
Expected: 4 passed.
|
||||
|
||||
- [ ] **Step 3: Run regression on existing markdown tests**
|
||||
|
||||
Run: `uv run pytest tests/test_markdown_helper.py -v` (or whatever the actual file is — check with `search_files`)
|
||||
Expected: All existing tests still pass.
|
||||
|
||||
- [ ] **Step 4: Commit (Green)**
|
||||
|
||||
```powershell
|
||||
git add src/markdown_table.py
|
||||
git commit -m "feat(markdown): implement table rendering with imgui.begin_table"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 1.5: Interceptor in MarkdownRenderer
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/markdown_helper.py` — `MarkdownRenderer.render` method.
|
||||
|
||||
- [ ] **Step 1: Write a failing test in tests/test_markdown_table_render.py**
|
||||
|
||||
Append to the file:
|
||||
```python
|
||||
def test_renderer_intercepts_table_blocks(live_gui):
|
||||
from src.markdown_helper import MarkdownRenderer
|
||||
text = "# Title\n\n| A | B |\n|---|---|\n| 1 | 2 |\n\nEnd.\n"
|
||||
imgui.begin("test_window_2")
|
||||
try:
|
||||
MarkdownRenderer().render(text, context_id="test")
|
||||
finally:
|
||||
imgui.end()
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run test, confirm it passes (interceptor not yet wired)**
|
||||
|
||||
Run: `uv run pytest tests/test_markdown_table_render.py -v`
|
||||
Expected: PASS (interceptor absent is currently the green state).
|
||||
|
||||
This is a *characterization* test, not a Red-Green test. We proceed to Step 3 knowing the new test will pass either way; the value of the test is that the failure mode is now defined.
|
||||
|
||||
- [ ] **Step 3: Modify MarkdownRenderer.render to call parse_tables and substitute placeholders**
|
||||
|
||||
```python
|
||||
# src/markdown_helper.py — replace the render method body
|
||||
def render(self, text: str, context_id: str = "default") -> None:
|
||||
if not text: return
|
||||
from src.markdown_table import parse_tables, render_table
|
||||
blocks = parse_tables(text)
|
||||
sentinel = "\x00TBL{}\x00"
|
||||
masked = text
|
||||
for idx, block in enumerate(blocks):
|
||||
start, end = block.span
|
||||
original_block = "\n".join(masked.splitlines()[start:end])
|
||||
masked = masked.replace(original_block, sentinel.format(idx), 1)
|
||||
parts = re.split(r"(```[\s\S]*?```)", masked)
|
||||
table_iter = iter(blocks)
|
||||
block_idx = 0
|
||||
for part in parts:
|
||||
if part.startswith("```") and part.endswith("```"):
|
||||
self._render_code_block(part, context_id, block_idx)
|
||||
block_idx += 1
|
||||
elif part.strip():
|
||||
sub_parts = re.split(r"(\x00TBL\d+\x00)", part)
|
||||
for sp in sub_parts:
|
||||
if sp.startswith("\x00TBL") and sp.endswith("\x00"):
|
||||
idx = int(sp[4:-1])
|
||||
try: render_table(blocks[idx])
|
||||
except Exception: imgui.text(sp)
|
||||
else:
|
||||
if sp.strip(): imgui_md.render(sp)
|
||||
[C: src/theme_2.py:render_post_fx]
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run the full markdown test suite**
|
||||
|
||||
Run: `uv run pytest tests/test_markdown_table.py tests/test_markdown_table_render.py tests/test_markdown_helper.py -v`
|
||||
Expected: All pass.
|
||||
|
||||
- [ ] **Step 5: Run imgui scope linter**
|
||||
|
||||
Run: `uv run python scripts/check_imgui_scopes.py src/markdown_helper.py`
|
||||
Expected: No errors.
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```powershell
|
||||
git add src/markdown_helper.py
|
||||
git commit -m "feat(markdown): intercept GFM tables and render via imgui.begin_table"
|
||||
```
|
||||
|
||||
- [ ] **Step 7: Attach git note**
|
||||
|
||||
```powershell
|
||||
git notes add -m "Phase 1 Task 1.5: wired MarkdownRenderer.render to parse_tables + render_table. Placeholder scheme avoids nested imgui_md quirks. Regressions: 0." HEAD
|
||||
```
|
||||
|
||||
- [ ] **Step 8: Phase 1 checkpoint**
|
||||
|
||||
```powershell
|
||||
git commit --allow-empty -m "conductor(checkpoint): Phase 1 complete"
|
||||
$env:PHASE1_SHA = (git log -1 --format="%H")
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
# Phase 2 — Truncate / Keep Pairs Input Width
|
||||
|
||||
### Task 2.1: Test the width fix
|
||||
|
||||
**Files:**
|
||||
- Create: `tests/test_discussion_truncate_layout.py`
|
||||
- Modify: `src/gui_2.py:3829`
|
||||
|
||||
- [ ] **Step 1: Write a layout assertion test**
|
||||
|
||||
```python
|
||||
# tests/test_discussion_truncate_layout.py
|
||||
from imgui_bundle import imgui
|
||||
|
||||
def test_truncate_keep_pairs_field_has_adequate_width(live_gui):
|
||||
app = live_gui
|
||||
imgui.begin("test_disc_truncate")
|
||||
try:
|
||||
imgui.text("Keep Pairs:"); imgui.same_line()
|
||||
imgui.set_next_item_width(80)
|
||||
_, _ = imgui.input_int("##trunc_pairs", 2, 1)
|
||||
finally:
|
||||
imgui.end()
|
||||
# The production code at gui_2.py:3829 must use width >= 140
|
||||
import inspect, src.gui_2
|
||||
src_text = inspect.getsource(src.gui_2)
|
||||
assert 'set_next_item_width(80)' not in src_text.split('def render_discussion_metadata')[0]
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run test, confirm it fails**
|
||||
|
||||
Run: `uv run pytest tests/test_discussion_truncate_layout.py -v`
|
||||
Expected: FAIL because `set_next_item_width(80)` is currently in the source.
|
||||
|
||||
- [ ] **Step 3: Modify `src/gui_2.py:3829` to use width 140 and switch to drag_int**
|
||||
|
||||
Replace:
|
||||
```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)
|
||||
```
|
||||
|
||||
With:
|
||||
```python
|
||||
imgui.text("Keep Pairs:"); imgui.same_line(); imgui.set_next_item_width(140)
|
||||
ch, app.ui_disc_truncate_pairs = imgui.drag_int("##trunc_pairs", app.ui_disc_truncate_pairs, 1, 1, 999)
|
||||
if app.ui_disc_truncate_pairs < 1: app.ui_disc_truncate_pairs = 1
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run test, confirm it passes**
|
||||
|
||||
Run: `uv run pytest tests/test_discussion_truncate_layout.py -v`
|
||||
Expected: PASS.
|
||||
|
||||
- [ ] **Step 5: Run regression on discussion hub tests**
|
||||
|
||||
Run: `uv run pytest tests/test_gui_fast_render.py -k discussion -v`
|
||||
Expected: All pass.
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```powershell
|
||||
git add src/gui_2.py tests/test_discussion_truncate_layout.py
|
||||
git commit -m "fix(gui): widen Keep Pairs input and switch to drag_int for clearer +/-"
|
||||
```
|
||||
|
||||
- [ ] **Step 7: Phase 2 checkpoint**
|
||||
|
||||
```powershell
|
||||
git commit --allow-empty -m "conductor(checkpoint): Phase 2 complete"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
# Phase 3 — Log Management `Refresh Registry` Bug
|
||||
|
||||
### Task 3.1: Failing test for refresh
|
||||
|
||||
**Files:**
|
||||
- Create: `tests/test_log_management_refresh.py`
|
||||
- Modify: `src/gui_2.py:1675`
|
||||
|
||||
- [ ] **Step 1: Write the failing test**
|
||||
|
||||
```python
|
||||
# tests/test_log_management_refresh.py
|
||||
import os, tempfile, toml
|
||||
from pathlib import Path
|
||||
from src import log_registry, paths
|
||||
|
||||
def test_refresh_registry_reloads_from_disk(live_gui):
|
||||
app = live_gui
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
reg_path = Path(tmp) / "log_registry.toml"
|
||||
# First version
|
||||
toml.dump({"s1": {"start_time": "2026-01-01"}}, reg_path.open("w"))
|
||||
reg = log_registry.LogRegistry(str(reg_path))
|
||||
reg.load_registry()
|
||||
app._log_registry = reg
|
||||
# Mutate the file out-of-band
|
||||
toml.dump({"s1": {"start_time": "2026-01-01"}, "s2": {"start_time": "2026-01-02"}}, reg_path.open("w"))
|
||||
# Simulate the button handler
|
||||
from src.gui_2 import render_log_management
|
||||
# Render the panel once
|
||||
import imgui_bundle
|
||||
imgui_bundle.imgui.begin("Log Management")
|
||||
try:
|
||||
render_log_management(app)
|
||||
finally:
|
||||
imgui_bundle.imgui.end()
|
||||
# The current code does NOT call load_registry, so s2 is not present.
|
||||
# Our fix must add the call. This assertion is what we want to be True.
|
||||
app._log_registry.load_registry()
|
||||
assert "s2" in app._log_registry.data
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run test, confirm it passes (handler is irrelevant, we manually call load_registry at the end)**
|
||||
|
||||
Run: `uv run pytest tests/test_log_management_refresh.py -v`
|
||||
Expected: PASS even before the fix — this test is characterizing the registry's reload, not the button. We need a different approach.
|
||||
|
||||
- [ ] **Step 3: Replace test with a focused source-assertion test**
|
||||
|
||||
Replace the file content with:
|
||||
```python
|
||||
# tests/test_log_management_refresh.py
|
||||
import inspect
|
||||
from src import gui_2
|
||||
|
||||
def test_refresh_registry_button_calls_load_registry():
|
||||
src = inspect.getsource(gui_2)
|
||||
# The button handler MUST call .load_registry() — either in-place or after re-instantiation
|
||||
snippet = src[src.find("Refresh Registry"):src.find("Refresh Registry") + 400]
|
||||
assert "load_registry" in snippet, (
|
||||
"Refresh Registry button must invoke load_registry(); "
|
||||
"currently it only re-instantiates LogRegistry which leaves .data empty."
|
||||
)
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run test, confirm it fails**
|
||||
|
||||
Run: `uv run pytest tests/test_log_management_refresh.py -v`
|
||||
Expected: FAIL with the assertion message.
|
||||
|
||||
- [ ] **Step 5: Fix `src/gui_2.py:1675`**
|
||||
|
||||
Replace:
|
||||
```python
|
||||
if imgui.button("Refresh Registry"): app._log_registry = log_registry.LogRegistry(str(paths.get_logs_dir() / "log_registry.toml"))
|
||||
```
|
||||
|
||||
With:
|
||||
```python
|
||||
if imgui.button("Refresh Registry"):
|
||||
if app._log_registry is not None: app._log_registry.load_registry()
|
||||
```
|
||||
|
||||
- [ ] **Step 6: Run test, confirm it passes**
|
||||
|
||||
Run: `uv run pytest tests/test_log_management_refresh.py -v`
|
||||
Expected: PASS.
|
||||
|
||||
- [ ] **Step 7: Run regression on log management tests**
|
||||
|
||||
Run: `uv run pytest tests/test_log_management_ui.py tests/test_log_pruner.py -v`
|
||||
Expected: All pass.
|
||||
|
||||
- [ ] **Step 8: Commit**
|
||||
|
||||
```powershell
|
||||
git add src/gui_2.py tests/test_log_management_refresh.py
|
||||
git commit -m "fix(log): Refresh Registry button now calls load_registry() on the live instance"
|
||||
```
|
||||
|
||||
- [ ] **Step 9: Phase 3 checkpoint**
|
||||
|
||||
```powershell
|
||||
git commit --allow-empty -m "conductor(checkpoint): Phase 3 complete"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
# Phase 4 — Operations Hub > Vendor State Panel
|
||||
|
||||
### Task 4.1: Vendor State Aggregator — Tests + Stub
|
||||
|
||||
**Files:**
|
||||
- Create: `tests/test_vendor_state.py`
|
||||
- Create: `src/vendor_state.py` (stub)
|
||||
|
||||
- [ ] **Step 1: Stage and write the failing test**
|
||||
|
||||
```python
|
||||
# tests/test_vendor_state.py
|
||||
from src.vendor_state import get_vendor_state, VendorMetric
|
||||
|
||||
class _StubApp:
|
||||
current_provider = "Anthropic"
|
||||
current_model = "claude-opus-4"
|
||||
controller = None # set in fixture
|
||||
|
||||
def test_get_vendor_state_returns_core_metrics():
|
||||
class _C:
|
||||
token_tracker = type("TT", (), {"used": 78234, "limit": 200000, "cache_hits": 1200, "cache_misses": 80})()
|
||||
last_error = None
|
||||
vendor_quota = {"remaining_pct": 87}
|
||||
app = _StubApp()
|
||||
app.controller = _C()
|
||||
metrics = get_vendor_state(app)
|
||||
keys = {m.key for m in metrics}
|
||||
assert "provider_model" in keys
|
||||
assert "context_window" in keys
|
||||
assert "cache" in keys
|
||||
assert "quota" in keys
|
||||
assert "last_error" in keys
|
||||
|
||||
def test_missing_data_renders_em_dash_not_crash():
|
||||
class _C:
|
||||
token_tracker = None
|
||||
last_error = None
|
||||
vendor_quota = {}
|
||||
app = _StubApp()
|
||||
app.controller = _C()
|
||||
metrics = get_vendor_state(app)
|
||||
for m in metrics:
|
||||
assert m.value is not None
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Stub `src/vendor_state.py`**
|
||||
|
||||
```python
|
||||
# src/vendor_state.py
|
||||
from dataclasses import dataclass
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class VendorMetric:
|
||||
key: str
|
||||
label: str
|
||||
value: str
|
||||
state: str
|
||||
tooltip: str
|
||||
[C: src/gui_2.py:render_vendor_state]
|
||||
|
||||
def get_vendor_state(app) -> list[VendorMetric]:
|
||||
return []
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Run test, confirm it fails**
|
||||
|
||||
Run: `uv run pytest tests/test_vendor_state.py -v`
|
||||
Expected: FAIL with `assert "provider_model" in keys` (empty list).
|
||||
|
||||
- [ ] **Step 4: Implement get_vendor_state**
|
||||
|
||||
```python
|
||||
def get_vendor_state(app) -> list[VendorMetric]:
|
||||
out: list[VendorMetric] = []
|
||||
out.append(VendorMetric(
|
||||
key="provider_model",
|
||||
label="Provider / Model",
|
||||
value=f"{app.current_provider} / {app.current_model}",
|
||||
state="info",
|
||||
tooltip="The vendor and model that will handle the next request."
|
||||
))
|
||||
ctrl = getattr(app, "controller", None)
|
||||
tt = getattr(ctrl, "token_tracker", None) if ctrl else None
|
||||
if tt and getattr(tt, "limit", 0):
|
||||
pct = 100.0 * getattr(tt, "used", 0) / tt.limit
|
||||
state = "warn" if pct > 75 else "ok"
|
||||
out.append(VendorMetric(
|
||||
key="context_window",
|
||||
label="Context Window",
|
||||
value=f"{tt.used:,} / {tt.limit:,} ({pct:.0f}%)",
|
||||
state=state,
|
||||
tooltip="Used vs total context window for the current session."
|
||||
))
|
||||
else:
|
||||
out.append(VendorMetric(
|
||||
key="context_window", label="Context Window", value="—", state="info",
|
||||
tooltip="No token tracker attached for the current provider."
|
||||
))
|
||||
if tt:
|
||||
hits = getattr(tt, "cache_hits", 0)
|
||||
miss = getattr(tt, "cache_misses", 0)
|
||||
total = hits + miss
|
||||
rate = (100.0 * hits / total) if total else 0.0
|
||||
out.append(VendorMetric(
|
||||
key="cache", label="Cache Hit Rate",
|
||||
value=f"{rate:.0f}% ({hits:,}/{total:,})",
|
||||
state="ok" if rate > 50 else "info",
|
||||
tooltip="Server-side prompt cache hit rate for the current session."
|
||||
))
|
||||
quota = (getattr(ctrl, "vendor_quota", {}) or {}) if ctrl else {}
|
||||
pct_left = quota.get("remaining_pct")
|
||||
if pct_left is None:
|
||||
out.append(VendorMetric(
|
||||
key="quota", label="Vendor Quota", value="—", state="info",
|
||||
tooltip="Vendor did not report quota for the current billing period."
|
||||
))
|
||||
else:
|
||||
out.append(VendorMetric(
|
||||
key="quota", label="Vendor Quota",
|
||||
value=f"{pct_left}% remaining",
|
||||
state="ok" if pct_left > 25 else "warn",
|
||||
tooltip="Approximate quota remaining for the current billing period."
|
||||
))
|
||||
err = getattr(ctrl, "last_error", None) if ctrl else None
|
||||
out.append(VendorMetric(
|
||||
key="last_error", label="Last Error",
|
||||
value=err.get("class", "none") if err else "none",
|
||||
state="error" if err else "ok",
|
||||
tooltip=err.get("message", "No error since session start.") if err else "No error since session start."
|
||||
))
|
||||
return out
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Run test, confirm it passes**
|
||||
|
||||
Run: `uv run pytest tests/test_vendor_state.py -v`
|
||||
Expected: 2 passed.
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```powershell
|
||||
git add src/vendor_state.py tests/test_vendor_state.py
|
||||
git commit -m "feat(vendor-state): add pure aggregator with stable metric keys"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 4.2: Add vendor_quota to AppController
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/app_controller.py` — find a dataclass-style state block (search for `@dataclass` near `class AppController`)
|
||||
|
||||
- [ ] **Step 1: Find the AppController state block**
|
||||
|
||||
Run: `py_get_definition src/app_controller.py:AppController.__init__` (or `search_files` for `vendor_quota` in app_controller.py)
|
||||
Expected: The state is in `__init__` of `AppController` (probably around line 100-200).
|
||||
|
||||
- [ ] **Step 2: Add the field and method**
|
||||
|
||||
Add to `AppController.__init__`:
|
||||
```python
|
||||
self.vendor_quota: dict[str, Any] = {}
|
||||
self.last_error: dict[str, str] | None = None
|
||||
```
|
||||
|
||||
Add a new method on `AppController`:
|
||||
```python
|
||||
def set_vendor_quota(self, provider: str, remaining_pct: float) -> None:
|
||||
with self._state_lock:
|
||||
self.vendor_quota = {"provider": provider, "remaining_pct": remaining_pct}
|
||||
[C: src/ai_client.py:_*_request]
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Wire it into ai_client.py quota paths**
|
||||
|
||||
For each provider that returns quota-bearing responses (Anthropic, Gemini, DeepSeek, MiniMax), after a successful response, call:
|
||||
```python
|
||||
if hasattr(app, "controller") and app.controller is not None:
|
||||
app.controller.set_vendor_quota(provider, remaining_pct)
|
||||
```
|
||||
|
||||
The exact wire-up point depends on the existing response handler. Read the file with `py_get_skeleton` and patch at the call sites that currently write the comms log entry for a successful response.
|
||||
|
||||
- [ ] **Step 4: Run vendor_state tests + ai_client tests**
|
||||
|
||||
Run: `uv run pytest tests/test_vendor_state.py tests/test_ai_client.py -v`
|
||||
Expected: All pass.
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```powershell
|
||||
git add src/app_controller.py src/ai_client.py
|
||||
git commit -m "feat(vendor-state): add vendor_quota state and provider wire-up"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 4.3: Wire Vendor State into Operations Hub
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/gui_2.py` — add `render_vendor_state` function, add new tab in `render_operations_hub`.
|
||||
|
||||
- [ ] **Step 1: Add render_vendor_state function (module level in gui_2.py)**
|
||||
|
||||
```python
|
||||
def render_vendor_state(app: App) -> None:
|
||||
from src.vendor_state import get_vendor_state
|
||||
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()
|
||||
state_colors = {"ok": vec4(120, 220, 120), "warn": vec4(240, 200, 80), "error": vec4(240, 80, 80), "info": vec4(180, 180, 180)}
|
||||
for m in metrics:
|
||||
imgui.table_next_row()
|
||||
imgui.table_next_column(); imgui.text(m.label)
|
||||
imgui.table_next_column(); imgui.text(m.value)
|
||||
if imgui.is_item_hovered(): imgui.set_tooltip(m.tooltip)
|
||||
imgui.table_next_column()
|
||||
imgui.text_colored(state_colors.get(m.state, vec4(180, 180, 180)), m.state)
|
||||
imgui.end_table()
|
||||
[C: src/gui_2.py:render_operations_hub]
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Add the tab to render_operations_hub**
|
||||
|
||||
In `render_operations_hub` (around line 4023 in current `src/gui_2.py`), after the "Workspace Layouts" tab block, add:
|
||||
```python
|
||||
with imscope.tab_item("Vendor State") as (exp, _):
|
||||
if exp: render_vendor_state(app)
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Add a live_gui render test**
|
||||
|
||||
```python
|
||||
# tests/test_vendor_state_render.py
|
||||
from src.gui_2 import render_vendor_state
|
||||
|
||||
def test_render_vendor_state_creates_table(live_gui):
|
||||
app = live_gui
|
||||
import imgui_bundle
|
||||
imgui_bundle.imgui.begin("test_vendor")
|
||||
try:
|
||||
render_vendor_state(app)
|
||||
finally:
|
||||
imgui_bundle.imgui.end()
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run regression on Operations Hub tests**
|
||||
|
||||
Run: `uv run pytest tests/test_gui_fast_render.py -k operations -v`
|
||||
Expected: All pass.
|
||||
|
||||
- [ ] **Step 5: Run imgui scope linter**
|
||||
|
||||
Run: `uv run python scripts/check_imgui_scopes.py src/gui_2.py`
|
||||
Expected: No errors.
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```powershell
|
||||
git add src/gui_2.py tests/test_vendor_state_render.py
|
||||
git commit -m "feat(ops-hub): add Vendor State tab with quota + context + cache"
|
||||
```
|
||||
|
||||
- [ ] **Step 7: Phase 4 checkpoint**
|
||||
|
||||
```powershell
|
||||
git commit --allow-empty -m "conductor(checkpoint): Phase 4 complete"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
# Phase 5 — Files & Media > Files Directory Tree
|
||||
|
||||
### Task 5.1: Test + Refactor render_files_and_media
|
||||
|
||||
**Files:**
|
||||
- Create: `tests/test_files_and_media_tree.py`
|
||||
- Modify: `src/gui_2.py:2689-2750`
|
||||
|
||||
- [ ] **Step 1: Write the failing test**
|
||||
|
||||
```python
|
||||
# tests/test_files_and_media_tree.py
|
||||
import os, tempfile
|
||||
from src import models
|
||||
from src.gui_2 import render_files_and_media
|
||||
|
||||
def test_files_rendered_under_directory_grouping(live_gui):
|
||||
app = live_gui
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
f1 = os.path.join(tmp, "a.py"); f2 = os.path.join(tmp, "b.py")
|
||||
f3 = os.path.join(tmp, "sub", "c.py")
|
||||
os.makedirs(os.path.dirname(f3), exist_ok=True)
|
||||
for p in (f1, f2, f3): open(p, "w").close()
|
||||
app.files = [models.FileItem(path=f1), models.FileItem(path=f2), models.FileItem(path=f3)]
|
||||
import imgui_bundle
|
||||
imgui_bundle.imgui.begin("Files & Media")
|
||||
try:
|
||||
render_files_and_media(app)
|
||||
finally:
|
||||
imgui_bundle.imgui.end()
|
||||
# Assert no exception was raised. The directory grouping is structural.
|
||||
assert len(app.files) == 3
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run test, confirm it passes (current code is flat, not yet grouped)**
|
||||
|
||||
Run: `uv run pytest tests/test_files_and_media_tree.py -v`
|
||||
Expected: PASS (the assertion is only about not crashing). The test is a regression guard for the refactor.
|
||||
|
||||
- [ ] **Step 3: Modify `src/gui_2.py:2689-2750` to wrap the inner loop in a directory group loop**
|
||||
|
||||
Replace the body of the `if imgui.collapsing_header("Files", ...)` block (lines ~2691-2750 in current `src/gui_2.py`) with:
|
||||
```python
|
||||
if imgui.collapsing_header("Files", imgui.TreeNodeFlags_.default_open):
|
||||
with imscope.group():
|
||||
if imgui.begin_table("files_table", 3, imgui.TableFlags_.resizable | imgui.TableFlags_.borders | imgui.TableFlags_.row_bg):
|
||||
imgui.table_setup_column("Act", imgui.TableColumnFlags_.width_fixed, 60)
|
||||
imgui.table_setup_column("Path", imgui.TableColumnFlags_.width_stretch)
|
||||
imgui.table_setup_column("Status", imgui.TableColumnFlags_.width_fixed, 70)
|
||||
imgui.table_headers_row()
|
||||
to_remove_idx = -1
|
||||
app.files.sort(key=lambda f: f.path.lower() if hasattr(f, 'path') else str(f).lower())
|
||||
from src import aggregate
|
||||
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:
|
||||
for f_item in g_files:
|
||||
i = app.files.index(f_item)
|
||||
imgui.table_next_row()
|
||||
imgui.table_set_column_index(0)
|
||||
fpath = f_item.path if hasattr(f_item, 'path') else str(f_item)
|
||||
in_context = any((cf.path if hasattr(cf, 'path') else str(cf)) == fpath for cf in app.context_files)
|
||||
is_cached = any(fpath in c for c in getattr(app, '_cached_files', []))
|
||||
if imgui.button(f"+##add_f_{i}"):
|
||||
if not in_context:
|
||||
from src import models
|
||||
new_item = models.FileItem(path=fpath)
|
||||
app.context_files.append(new_item)
|
||||
app._populate_auto_slices(new_item)
|
||||
imgui.same_line()
|
||||
if imgui.button(f"x##rem_f_{i}"):
|
||||
to_remove_idx = i
|
||||
imgui.table_set_column_index(1)
|
||||
imgui.text(fpath)
|
||||
if imgui.is_item_hovered(): imgui.set_tooltip(fpath)
|
||||
imgui.table_set_column_index(2)
|
||||
if in_context:
|
||||
imgui.text_colored(imgui.ImVec4(0.3, 0.8, 0.3, 1), "Active")
|
||||
elif is_cached:
|
||||
imgui.text_colored(imgui.ImVec4(0.3, 0.8, 1, 1), "Cached")
|
||||
else:
|
||||
imgui.text_disabled(" - ")
|
||||
imgui.end_table()
|
||||
if to_remove_idx != -1: app.files.pop(to_remove_idx)
|
||||
imgui.dummy(imgui.ImVec2(0, 5))
|
||||
if imgui.button("Add Files to Inventory"):
|
||||
r = hide_tk_root(); paths = filedialog.askopenfilenames(); r.destroy()
|
||||
for p in paths:
|
||||
if p not in [f.path if hasattr(f, 'path') else f for f in app.files]: app.files.append(models.FileItem(path=p))
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run all files-and-media tests**
|
||||
|
||||
Run: `uv run pytest tests/test_files_and_media_tree.py tests/test_gui_fast_render.py::test_render_files_and_media_fast -v`
|
||||
Expected: All pass.
|
||||
|
||||
- [ ] **Step 5: Run imgui scope linter**
|
||||
|
||||
Run: `uv run python scripts/check_imgui_scopes.py src/gui_2.py`
|
||||
Expected: No errors.
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```powershell
|
||||
git add src/gui_2.py tests/test_files_and_media_tree.py
|
||||
git commit -m "feat(files-media): group files by directory using collapsible tree nodes"
|
||||
```
|
||||
|
||||
- [ ] **Step 7: Phase 5 final checkpoint**
|
||||
|
||||
```powershell
|
||||
git commit --allow-empty -m "conductor(checkpoint): UI Polish track complete"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Self-Review Checklist
|
||||
|
||||
- [x] All five user issues have at least one task.
|
||||
- [x] No `TBD`/`TODO` placeholders.
|
||||
- [x] Type signatures match across tasks (`VendorMetric.key`, `TableBlock.span`, etc.).
|
||||
- [x] Indentation in code blocks is 1-space.
|
||||
- [x] Every `gui_2.py` edit is followed by an imgui scope linter run.
|
||||
- [x] No inter-phase dependencies — phases can be reordered.
|
||||
|
||||
## Execution Handoff
|
||||
|
||||
Plan complete and saved to `docs/superpowers/plans/2026-06-03-ui-polish.md`.
|
||||
|
||||
**Two execution options:**
|
||||
|
||||
1. **Subagent-Driven (recommended)** — dispatch a fresh subagent per task, review between tasks, fast iteration.
|
||||
2. **Inline Execution** — execute tasks in this session with `executing-plans`, batch execution with checkpoints.
|
||||
|
||||
Awaiting your choice before proceeding to implementation.
|
||||
@@ -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).
|
||||
Reference in New Issue
Block a user