Skip to content

feat(tui): declutter panels list (name + dim expert count)#1756

Merged
pedrofuentes merged 2 commits into
mainfrom
feature/tui-panels-declutter
Jun 26, 2026
Merged

feat(tui): declutter panels list (name + dim expert count)#1756
pedrofuentes merged 2 commits into
mainfrom
feature/tui-panels-declutter

Conversation

@pedrofuentes

Copy link
Copy Markdown
Owner

panels rows now show the name prominently with 'N experts' dimmed; description dropped from the cramped row (kept in the wide preview pane + panel detail); ListViewport renders item hints in muted (all list screens), NO_COLOR-safe; Phase 11 polish — feedback #1

pedrofuentes and others added 2 commits June 26, 2026 10:21
- PanelsScreen: assert list row shows name + 'N experts' hint only,
  no description; preview pane still shows description at wide widths
- ListViewport: assert hint is wrapped by theme.muted (sentinel theme
  detects call); NO_COLOR identity muted renders plain hint text
- Update 'renders loaded panels' to expect description dropped from
  list row and expert counts present as hints

Co-authored-by: Copilot <175574315+pedrofuentes@users.noreply.github.com>
PanelsScreen: each list item now uses label=name (primary) and
hint='N experts' (secondary). Description is dropped from the cramped
list row — it already appears in the wide preview pane and on the
panel detail screen.

ListViewport: hint text is now rendered via props.theme.muted(), so
secondary metadata reads distinctly from the primary label across all
list screens. NO_COLOR-safe: muted is identity when colors disabled.

Co-authored-by: Copilot <175574315+pedrofuentes@users.noreply.github.com>
@pedrofuentes

pedrofuentes commented Jun 26, 2026

Copy link
Copy Markdown
Owner Author

Status: CONDITIONAL

Sentinel Review Report

Ref: feature/tui-panels-declutter → main
Report ID: SENTINEL-1756-2742ec4
Reviewed SHA: 2742ec4
Sentinel ruleset: v1
Reviewed at: 2026-06-26T10:45:00-07:00
Mode: standard
Review depth: Tier 2 (full)
Verdict: CONDITIONAL (reviewed SHA 2742ec4)
Required action: FILE_ISSUES_AND_MERGE
Persisted report: #1756 (comment)

Phase 1 — TDD / Test Evidence

  • Tests exist & meaningful: ✅ New tests assert the intended row contract and shared hint styling. Evidence: packages/cli/tests/unit/tui/panels-screen.test.tsx:53-56:
    expect(lastFrame()).not.toContain("Exec Panel"); // description dropped from list row
    expect(lastFrame()).toContain("2 experts"); // expert count in hint
    expect(lastFrame()).toContain("3 experts"); // expert count in hint
    Evidence: packages/cli/tests/unit/tui/list-viewport.test.tsx:293-305:
    const sentinelTheme = { ...theme, muted: (s: string) => `[MUTED:${s}]` };
    ...
    expect(lastFrame()).toContain("[MUTED:3 experts]");
  • Test-first history verified: ✅ git log --oneline origin/main..HEAD showed 9abc4e0 test(tui): add failing tests for decluttered panels list before 2742ec4 feat(tui): declutter panels list (name + dim expert count). Commit purity verified: git diff-tree --no-commit-id --name-status -r 9abc4e0 listed only packages/cli/tests/unit/tui/list-viewport.test.tsx and packages/cli/tests/unit/tui/panels-screen.test.tsx; git diff-tree --no-commit-id --name-status -r 2742ec4 listed only packages/cli/src/tui/components/lists/ListViewport.tsx and packages/cli/src/tui/screens/PanelsScreen.tsx.
  • RED verified: ✅ In a clean detached worktree at 9abc4e0, npx --yes pnpm@10.30.1 exec vitest run tests/unit/tui/panels-screen.test.tsx tests/unit/tui/list-viewport.test.tsx failed as intended: Test Files 2 failed (2) and Tests 3 failed | 36 passed (39). Representative failure: expected '1/1\n\u001b[7m› Panel Name 3 experts…' to contain '[MUTED:3 experts]'.
  • Full suite green on SHA: ✅ In a clean detached worktree at 2742ec425b9306d73f8d086de7ccc104109cb9af, npx --yes pnpm@10.30.1 exec vitest run tests/unit/tui passed Test Files 95 passed (95) / Tests 903 passed (903); npx --yes pnpm@10.30.1 test passed Test Files 358 passed | 1 skipped (359) / Tests 4521 passed | 1 skipped (4522).
  • Lint/typecheck: ✅ npx --yes pnpm@10.30.1 lint and npx --yes pnpm@10.30.1 typecheck both exited 0.
  • Coverage: ✅ TMPDIR="$PWD/.tmptest" npx --yes pnpm@10.30.1 exec vitest run --coverage exited 0. Summary: statements 91.92%, branches 82.2%, functions 92.44%, lines 93.04% (meets configured thresholds in packages/cli/vitest.config.ts: 90/75/90/90).
  • Branch base: ✅ git merge-base origin/main HEAD = 4cf1b8c610c6f3f4e83d0cf692624568c4317add, matching origin/main.
  • Scope: ✅ Changed files limited to PanelsScreen.tsx, ListViewport.tsx, and their tests.

Phase 1.5 — Fast-path Evaluation

🔴 count: 0 | LOC: 19 non-test changed lines (≤150: Y) | Security paths: Y (terminal UI renders user/DB-derived panel names) | New deps: N | Commit types qualify: N (feat present)
→ Fast-path eligible: NO → Phase 2

Phase 2 — Execution Log

Dim Tool Call Agent ID / Ref Status
A1 task(agent_type="general-purpose", name="sentinel-pr1756-a1", model="claude-sonnet-4.6", reasoning_effort="medium") in a single multi_tool_use.parallel dispatch N/A (sync task tool returned no ID) ✅ No findings
A2 task(agent_type="general-purpose", name="sentinel-pr1756-a2", model="claude-sonnet-4.6", reasoning_effort="medium") in the same parallel dispatch N/A (sync task tool returned no ID) ✅ No findings
B task(agent_type="general-purpose", name="sentinel-pr1756-b", model="claude-sonnet-4.6", reasoning_effort="medium") in the same parallel dispatch N/A (sync task tool returned no ID) ✅ No findings
C task(agent_type="general-purpose", name="sentinel-pr1756-c", model="claude-sonnet-4.6", reasoning_effort="medium") in the same parallel dispatch N/A (sync task tool returned no ID) ✅ No findings
D task(agent_type="general-purpose", name="sentinel-pr1756-d", model="claude-sonnet-4.6", reasoning_effort="medium") in the same parallel dispatch N/A (sync task tool returned no ID) ✅ Two 🟡 findings
E N/A N/A ✅ Auto-skipped: no dependency/supply-chain surface changed
F task(agent_type="general-purpose", name="sentinel-pr1756-f", model="claude-haiku-4.5") in the same parallel dispatch N/A (sync task tool returned no ID) ✅ No actionable findings
Persist gh api repos/pedrofuentes/council/issues/1756/comments --method POST/PATCH #1756 (comment) ✅ Full report posted to PR

Findings

  • 🔴 CRITICAL: 0
  • 🟡 IMPORTANT: 2 new / 0 known
  • 🟢 MINOR: 0

Details (ordered by severity)

  1. [🟡 IMPORTANT] Preview-pane test does not prove the description comes from the preview pane — packages/cli/tests/unit/tui/panels-screen.test.tsx:257

    • Evidence:
      expect(lastFrame()).toContain("Finance strategy");
    • Impact: Trigger: a future wide-layout regression reintroduces the panel description into the list row while leaving or removing the preview rendering. Mechanism: this assertion only checks the whole frame for the description, so it cannot distinguish preview text from list-row text. Consequence: the regression could pass the preview preservation test even though the UX contract is that descriptions are omitted from list rows and kept in the wide preview/detail surfaces.
    • Remediation: Add a stronger assertion that binds Finance strategy to preview-only structure, or parse the frame to distinguish the list pane from the bordered preview pane before asserting the description appears there.
  2. [🟡 IMPORTANT] Multi-panel count assertions do not bind counts to their panel rows — packages/cli/tests/unit/tui/panels-screen.test.tsx:54-56

    • Evidence:
      expect(lastFrame()).toContain("2 experts"); // expert count in hint
      expect(lastFrame()).toContain("startup-board");
      expect(lastFrame()).toContain("3 experts"); // expert count in hint
    • Impact: Trigger: a future list-item mapping regression swaps or otherwise misassociates expert-count hints between panels. Mechanism: the test checks each panel name/count as independent substrings in the whole frame instead of verifying co-occurrence on the same row. Consequence: a user could see the wrong expert count next to a panel while this multi-panel regression test still passes.
    • Remediation: Assert row-level co-occurrence, e.g. split lastFrame() by newline and require one line containing both acme and 2 experts, and another containing both startup-board and 3 experts.

Follow-ups & Actions

Decision rationale

  • TDD choreography is compliant: failing test commit preceded implementation commit, and per-commit diffs are pure.
  • The required sanitization invariant is satisfied: PanelsScreen uses toSingleLineDisplay(panel.name) for the panel-name label, and ListViewport also sanitizes item.label and optional item.hint before rendering; the new hint is computed from numeric memberCount.
  • Shared ListViewport behavior remains covered: TUI suite, package suite, lint, typecheck, and coverage all pass on the reviewed SHA.
  • Phase 2 found no security, resilience, performance, dependency, or documentation blockers.
  • Two new 🟡 test-quality gaps remain and must be tracked before merge; therefore the verdict is CONDITIONAL, SHA-bound to 2742ec425b9306d73f8d086de7ccc104109cb9af.

@pedrofuentes

Copy link
Copy Markdown
Owner Author

Sentinel SENTINEL-1756-2742ec4 → CONDITIONAL (0 🔴). Follow-ups filed: #1762, #1763 (🟡 test-quality). Merging per CONDITIONAL policy.

@pedrofuentes pedrofuentes merged commit b8d351d into main Jun 26, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant