Skip to content

refactor(app): replace right-panel status todo dots with widget marker#861

Merged
Astro-Han merged 7 commits into
devfrom
claude/area-b-slice3-status-dots
May 23, 2026
Merged

refactor(app): replace right-panel status todo dots with widget marker#861
Astro-Han merged 7 commits into
devfrom
claude/area-b-slice3-status-dots

Conversation

@Astro-Han

Copy link
Copy Markdown
Owner

Summary

Right-panel Status tab rendered todo progress with coloured 6/8px dots. DESIGN.md L201 forbids dots as state signals; the only sanctioned dot in PawWork is the sidebar session-row unread orange dot.

This PR swaps the dot for the canonical todo widget marker already used in session-todo-dock.tsx and todowrite.tsx:

  • completed<Icon name="circle-check"> 16×16
  • pending / cancelled<Icon name="circle"> 16×16
  • in_progress → 13×13 ring with brand-primary top border + --animate-pw-spin
  • Completed and cancelled rows uniformly get line-through + text-fg-weak. Previously only cancelled was struck, and cancelled was text-fg-weaker; aligning to the dock convention.
  • gap-2.5gap-2 to match the dock row spacing.

Net effect: same Status tab, same todo data, marker style now matches the same todos rendered elsewhere in the app.

Why

Slice 3 of the Area B right-panel audit (docs/research/2026-05-23-area-b-right-panel-audit.md, local-only) flagged status dots as a P0 DESIGN.md drift. Slice 1+2 (#854) was the first cut; this is the second.

The audit originally bundled session-status-connections.tsx (servers / MCP / LSP / plugins) into the same slice. During implementation we narrowed scope: whether that section belongs in the right panel at all is a separate product question (it carries the only live infrastructure-health signal, and moving it requires deciding on an alert channel first). A follow-up issue will track that question.

Related Issue

Refs #602 (Area B right-panel rewrite, slice 3).

Human Review Status

Pending

Review Focus

  • Marker logic in TodoMarker: the icon-name mapping and the in_progress 13×13 ring are a direct port of the same JSX block in session-todo-dock.tsx lines 227-259. Confirm parity if you have a strong opinion on either visual.
  • Text treatment change: completed rows now also get line-through (previously only cancelled was struck). This is the dock convention; flag if the Status tab should diverge.
  • Cancelled text colour shifted from --fg-weaker to --fg-weak (one tier darker, dock convention). Tiny perceptual shift.
  • session-status-connections.tsx is intentionally untouched; its dots remain. Follow-up issue tracks the position question.

Risk Notes

  • Behavioural change: completed rows now show a strikethrough where they didn't before. Same data, different rendering, no migration. If user feedback says completed shouldn't be struck in this surface, revert is one-line.
  • No platform, packaging, IPC, or shell surface touched.
  • No new dependencies.

How To Verify

typecheck:                bun run typecheck — 8/8 packages pass
lint:                     bun run lint packages/app/src/components/session/session-status-summary.tsx
                            packages/app/src/components/session/session-status-summary.test.ts
                          — clean (1 expected warning: .test.ts file matches eslint ignore pattern)
focused tests:            bun test --preload ./happydom.ts
                            src/components/session/session-status-summary.test.ts
                            src/pages/session/session-side-panel.test.tsx
                          — 17 pass, 0 fail across both files
visual baseline:          bun run snap app-shell — passed; default app-shell screenshot
                            regenerated unchanged (the right-panel Status tab is not in
                            the app-shell snap surface, but app-shell confirms no
                            collateral damage to the shell layout).
visual coverage of new
markers:                  the new TodoMarker JSX is a direct port of session-todo-dock.tsx
                            lines 227-259, which is already snap-tested by
                            packages/app/e2e/snap/todo-dock-restored.snap.ts.
                            Same icon names, same ring spec, same animation token.

A dedicated right-panel-status-todos snap target was considered. Deferred for cost/benefit: ~80 lines of e2e setup (open session → seed todos → toggle right panel → screenshot) to verify markers that are byte-for-byte the same as a surface already in baseline. Happy to add it if a reviewer wants the durable check.

Screenshots or Recordings

Local design-decision page (untracked, local checkout only): docs/design/scratch/right-pane-status-icons-compare.html shows current dots vs new markers in light and dark themes for both Status Summary and Status Connections sections (Connections side preserved for reference; not touched in this PR).

Checklist

  • Type label — this PR carries exactly one of bug, enhancement, task, documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.
  • Routing labels — this PR carries at least one of app, ui, platform, harness, ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.
  • Priority label — this PR carries exactly one of P0, P1, P2, P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.
  • Human Review Status above is set to Pending, Approved by @<reviewer>, or Not required: <reason> (default is Pending; "not required" is restricted to bot-authored low-risk PRs).
  • I linked the related issue, or stated in Summary why there is no issue.
  • I described the review focus and any meaningful risks.
  • I replaced the example block in How To Verify with the real verification steps and the key result for each.
  • I did not introduce unrelated refactors, dependencies, generated files, or file changes beyond the stated scope.
  • (conditional) I manually checked visible UI or copy changes when needed, with screenshots or recordings. Leave unticked only if no visible UI or copy changed.
  • (conditional) I considered macOS and Windows impact for platform, packaging, updater, signing, paths, shell, or permissions changes. Leave unticked only if no platform/packaging surface was touched.
  • (conditional) I called out docs, release notes, dependencies, permissions, credentials, deletion behavior, generated content, or local file changes when relevant. Leave unticked only if none of those surfaces was touched.
  • I reviewed the final diff for unrelated changes and suspicious dependency changes.
  • I am targeting dev, and my PR title and commit messages use Conventional Commits in English.

Right-panel Status tab rendered todo progress with coloured 6/8px dots
(bg-icon-success-base, bg-icon-info-base, bg-border-weak). DESIGN.md L201
forbids dots as state signals; the only sanctioned dot in PawWork is the
sidebar session-row unread orange dot.

This commit aligns the Status tab todo rendering with the canonical todo
widget already used in session-todo-dock.tsx and todowrite.tsx:

- completed → `<Icon name="circle-check">` 16x16
- pending / cancelled → `<Icon name="circle">` 16x16
- in_progress → 13x13 ring with brand-primary top border and pw-spin
- completed and cancelled rows uniformly get line-through + text-fg-weak
  (previously only cancelled was struck; cancelled was text-fg-weaker)
- gap-2.5 → gap-2 to match the dock spacing

Net: same Status tab, same todo data, marker style swapped to the visual
already shipping elsewhere in the app.

Refs #602 (Area B right-panel rewrite, slice 3)

Out of scope for this PR:
session-status-connections.tsx (servers / MCP / LSP / plugins) keeps its
dots for now; whether that section belongs in the right panel at all is
a separate product question (alert channel needed if it moves) and will
be tracked in a follow-up issue.
@coderabbitai

coderabbitai Bot commented May 23, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@Astro-Han, we couldn't start this review because you've used your available PR reviews for now.

Your plan currently allows 1 review/hour. Refill in 15 minutes and 55 seconds.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more review capacity refills, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e96dc316-0ce9-4342-89c0-0d49b1bc25ca

📥 Commits

Reviewing files that changed from the base of the PR and between 694a390 and c9600d9.

📒 Files selected for processing (7)
  • packages/app/e2e/snap/status-summary-todos.snap.ts
  • packages/app/src/components/session/session-status-summary.test.ts
  • packages/app/src/components/session/session-status-summary.tsx
  • packages/app/src/pages/session/composer/session-todo-dock.tsx
  • packages/ui/src/components/message-part/tools/todowrite.tsx
  • packages/ui/src/components/todo-status-marker.test.ts
  • packages/ui/src/components/todo-status-marker.tsx
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/area-b-slice3-status-dots

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added app Application behavior and product flows ui Design system and user interface P2 Medium priority labels May 23, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested priority: P2 (includes user-path files (packages/app/src/components/session/session-status-summary.test.ts, packages/app/src/components/session/session-status-summary.tsx)).

P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.

@Astro-Han Astro-Han added the task Narrow execution, audit, spike, migration, tracking, or upstream follow-up work label May 23, 2026
@Astro-Han

Copy link
Copy Markdown
Owner Author

Follow-up filed: #862 tracks the Connections section position decision (kept out of this PR's scope per Risk Notes).

@github-actions github-actions Bot removed the task Narrow execution, audit, spike, migration, tracking, or upstream follow-up work label May 23, 2026
@Astro-Han Astro-Han added the task Narrow execution, audit, spike, migration, tracking, or upstream follow-up work label May 23, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the session-status-summary component to replace status dots with canonical icons and a spinner, aligning the UI with design specifications. It also introduces a new test file to verify these UI contracts. Feedback includes a recommendation to use import.meta.url for file resolution in tests to ensure stability and a suggestion to add break-words to the todo content to handle long text within the sidebar.

Comment thread packages/app/src/components/session/session-status-summary.test.ts Outdated
Comment thread packages/app/src/components/session/session-status-summary.tsx Outdated
@Astro-Han

Copy link
Copy Markdown
Owner Author

CI red but unrelated to this PR's code:

  • pr-triage failed reading the initial label snapshot (P2, app, uitask had not been added yet). All four required labels (task, app, ui, P2) are now in place. The workflow triggers on opened/synchronize/reopened and does not re-run on label changes, so the check shows stale. Maintainer can rerun pr-triage or close/reopen to refresh.
  • lint job failed at the actions/checkout step with could not read Username for 'https://github.com' — appears to be a submodule auth flake in CI. No actual lint was run. Local bun run lint on the touched files is clean (reported in How To Verify).

Code checks that did run: typecheck ✓, unit-app ✓, unit-ui-focused ✓, unit-opencode ✓, frontend-architecture ✓, changes (×4) ✓, lint-pr-title ✓, lint-commits ✓, dependency-review ✓, CodeRabbit ✓.

@github-actions

github-actions Bot commented May 23, 2026

Copy link
Copy Markdown

Perf delta summary

Comparator: pass

Profile / Scenario interaction median interaction worst long task max tbt frame gap p95 frame gap max jank count cls status
default / session-streaming-long 48 -> 40 (-8) 72 -> 64 (-8) 0 -> 0 (0) 0 -> 0 (0) 16.7 -> 16.8 (+0.1) 16.8 -> 16.8 (0) 0 -> 0 (0) 0 -> 0 (0) pass
default / session-scroll-reading 16 -> 16 (0) 16 -> 16 (0) 0 -> 0 (0) 0 -> 0 (0) 16.8 -> 33.3 (+16.5) 16.8 -> 33.3 (+16.5) 0 -> 0 (0) 0 -> 0 (0) pass

- Add packages/app/e2e/snap/status-summary-todos.snap.ts: opens the
  right panel after seeding todos in all four states (completed /
  in_progress / pending / cancelled) and screenshots the panel.
  Gives Slice 3 a durable visual baseline in addition to the
  todo-dock-restored target that already covers the marker shape.

- Address gemini-code-assist[bot] review on PR #861:
  - session-status-summary.test.ts: resolve source via
    new URL("…", import.meta.url) instead of __dirname + path.join,
    matching the repository test convention.
  - session-status-summary.tsx: add break-words to the todo content
    container so long todo strings wrap inside the narrow right panel,
    matching session-todo-dock.tsx text styling.

Refs #602, #861
@github-actions github-actions Bot removed the task Narrow execution, audit, spike, migration, tracking, or upstream follow-up work label May 23, 2026
Astro-Han added 2 commits May 23, 2026 16:38
Applies fixes flagged by the crosscheck round on PR #861:

- snap target: pass animations: "disabled" to panel.screenshot() so the
  in_progress 13×13 pw-spin ring captures the same orientation every
  run. Previously the screenshot froze whatever frame Playwright
  happened to land on, making the baseline non-deterministic across
  rebuilds (Codex P2).

- snap target: poll until at least four todo rows are visible before
  capturing, so a partial render does not silently produce a
  misleading baseline (Claude P3).

- unit test: assert literal 13px width/height on the spin ring so a
  size drift surfaces as a test failure instead of a passing comment
  (Codex P3).
Fresh-eye crosscheck micro-fixes on PR #861:

- snap: strict equality on todo row count (toBe(4) instead of
  toBeGreaterThanOrEqual(4)) so a leaked prior turn or duplicate
  render fails instead of silently extending the baseline (Claude P3).

- summary: drop unused data-slot="status-summary-todo-running" — no
  test or selector references it (YAGNI, Claude P3).
@Astro-Han Astro-Han added the task Narrow execution, audit, spike, migration, tracking, or upstream follow-up work label May 23, 2026
@Astro-Han

Copy link
Copy Markdown
Owner Author

Crosscheck record

Three rounds of multi-model code review (Claude Opus reviewer + Codex high-reasoning) were run on this PR. Summary:

Round Codex Claude Action taken
1 (initial) 1× P2, 1× P3 4× P2, 7× P3, 2× design alts Applied 3: animations: "disabled" for snap determinism, count >= 4 wait, literal 13px size assertion
2 (post-fix) 0 findings 2× P2, 3× P3, 2× design alts Applied 2: dropped unused data-slot, tightened count to toBe(4)
3 (final fresh-eye) 0 findings 3× P3 only, 2× design alts None blocking; remaining P3 are polish (createMemo micro-opt, px→rem migration, magic-4 → seed.length)

Final status: zero P0, zero P1, zero P2 across both reviewers. Pass criteria met.

Deferred (not blocking)

  • Both reviewers, all three rounds: extract a shared TodoStatusMarker consumed by session-todo-dock.tsx, todowrite.tsx, and session-status-summary.tsx. This is the most consistently flagged design observation. Deferred because it broadens scope beyond the right-panel surface and pulls in two other files. A follow-up refactor PR can do this cleanly.
  • Three P3 polish items above (memoise isDone, decouple "13px" literal, derive 4 from seed array) — all minor.

Per-finding rejection rationale

  • text-fg-base vs dock's var(--fg-strong) (Claude round 2 P2): preserved from original code; todowrite.tsx also doesn't use fg-strong; the slice mirrors the marker, not the entire row chrome.
  • isDone vs isTerminalTodo helper from todo-model.ts (Claude round 1 P2): adopting the helper diverges from the dock + todowrite, both of which use the same inline ternary pattern; aligning with the dock is the explicit goal.
  • Inline styles bypassing Tailwind (Claude round 1 P2): identical pattern in the dock; alignment is the goal.

Local verification of the final state

typecheck:           bun run typecheck — 8/8 packages pass
focused unit tests:  bun test --preload ./happydom.ts
                       src/components/session/session-status-summary.test.ts
                       src/pages/session/session-side-panel.test.tsx
                     — 17 pass, 0 fail
focused snap:        bun run snap status-summary-todos — passed,
                       baseline at docs/design/preview/screenshots/status-summary-todos.png
                       shows all four marker states (completed / in_progress
                       / pending / cancelled) at frozen-frame.
visual baseline:     bun run snap app-shell — unchanged (no regression to shell)

Three places rendered the same 13×13 ring + circle/circle-check marker:

  packages/ui/src/components/message-part/tools/todowrite.tsx   (37 lines)
  packages/app/src/pages/session/composer/session-todo-dock.tsx (33 lines)
  packages/app/src/components/session/session-status-summary.tsx (37 lines, added by Slice 3)

Both rounds of crosscheck on PR #861 and a follow-up review flagged this
as a drift risk. Extract the marker once into packages/ui so any future
DESIGN.md change (ring size, palette, icon swap) propagates with a single
edit.

- New: packages/ui/src/components/todo-status-marker.tsx
  - status prop typed as TodoStatus | (string & {}) so the SDK's bare
    string status field (gen/types.gen.ts) flows through without casts.
  - Optional marginTop prop: dock + status-summary pass "1px" to align
    with body text baseline; todowrite leaves it unset (different row
    baseline).
- New: todo-status-marker.test.ts — source-text contract covering the
  status → icon mapping, 13×13 ring dimensions, --animate-pw-spin token,
  and conditional marginTop application.
- session-status-summary.test.ts: marker assertions moved out; only row
  behaviour (no leftover dot classes, strikethrough rule, delegation to
  the shared component) remains here.

Visual: byte-for-byte identical to the prior inline JSX at each callsite.
The new marker is a 1:1 lift of the existing pattern, not a rewrite, so
the existing snap baselines (status-summary-todos, todo-dock-restored)
both pass unchanged.

Refs #602, #861
@github-actions github-actions Bot removed the task Narrow execution, audit, spike, migration, tracking, or upstream follow-up work label May 23, 2026
@Astro-Han Astro-Han added the task Narrow execution, audit, spike, migration, tracking, or upstream follow-up work label May 23, 2026
Both Show branches now share a single 16x16 inline-flex wrapper that owns
the optional marginTop nudge. Previously the in_progress branch put the
nudge on its outer span, while the fallback Icon routed it through
splitProps onto the inner <svg> where the wrapper's align-items: center
erased it — so marginTop=1px only nudged in_progress rows, not the
pending/completed/cancelled ones. The shared wrapper makes the prop
behave symmetrically regardless of state.

Also drops the redundant 'color: var(--icon-base)' on the fallback Icon;
icon.css already applies the same colour to [data-component=icon].

Surfaced by crosscheck reviewer feedback on #861.
@github-actions github-actions Bot removed the task Narrow execution, audit, spike, migration, tracking, or upstream follow-up work label May 23, 2026
@Astro-Han Astro-Han added the task Narrow execution, audit, spike, migration, tracking, or upstream follow-up work label May 23, 2026
@github-actions github-actions Bot removed the task Narrow execution, audit, spike, migration, tracking, or upstream follow-up work label May 23, 2026
Spell out that marginTop is read once at render time and that callsites
pass a static literal — guards against a future reader assuming the
inline-style object will re-spread reactively when the prop changes.

Touch only the JSDoc comment; behaviour unchanged.
@Astro-Han Astro-Han added the enhancement New feature or request label May 23, 2026
@Astro-Han

Copy link
Copy Markdown
Owner Author

Reopening to retrigger pr-triage with corrected enhancement label (labeler sync-labels was removing the manually-added task label per .github/labeler.yml glob rules).

@Astro-Han

Copy link
Copy Markdown
Owner Author

Reviewer P3 反馈处理:新测试未接入 CI

已验证 reviewer 的判断有效且范围更大

.github/workflows/ci.yml 的三个 unit 入口(unit-app / unit-opencode / unit-desktop)都用 turbo test:ci --filter=<具体包> 调用,没有任何 job 触发 @opencode-ai/ui#test:ci,且 turbo.json 里也没定义这个 task。unit-ui-focused 只显式跑 packages/ui/test/ 下 5 个 token/theme/typography guard。

结果:packages/ui/src/**34 个 *.test.ts 文件(不只是本 PR 新加的 todo-status-marker.test.ts,还包括 markdown、scroll-view、picker、tool-contract、session-turn-* 等)全部在 CI 上沉默。

为什么不在本 PR 修

.github/workflows/ci.yml 会触发 .github/labeler.ymltask 标签的 glob 规则(绑 .github/workflows/**),同时 actions/labelersync-labels: true强制保留 task——会与本 PR 当前的 enhancement 标签冲突(与 #854 同质先例对齐),且 label policy 要求 exactly-one-type。

且这是项目级 CI 覆盖洞,影响范围 33 个其他测试,不应让 Slice 3(UI 重构)PR 兼带这次清扫。

已开 follow-up

#869 — 含完整证据 + 三个修复方向(推荐方向 B:在 packages/ui/package.jsontest:citurbo.json 定义 @opencode-ai/ui#test:ci,让 turbo graph 自动接入,零 ci.yml 增量、未来新测试自动覆盖)。

@Astro-Han Astro-Han merged commit d528ab8 into dev May 23, 2026
57 of 67 checks passed
@Astro-Han Astro-Han deleted the claude/area-b-slice3-status-dots branch May 23, 2026 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app Application behavior and product flows enhancement New feature or request P2 Medium priority ui Design system and user interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant