Skip to content

Trustable review UX: highlighted diffs, reviewed checkboxes, timeline-to-diff link#15

Open
minghinmatthewlam wants to merge 9 commits intomainfrom
claude/inspiring-leavitt-27f1fc
Open

Trustable review UX: highlighted diffs, reviewed checkboxes, timeline-to-diff link#15
minghinmatthewlam wants to merge 9 commits intomainfrom
claude/inspiring-leavitt-27f1fc

Conversation

@minghinmatthewlam
Copy link
Copy Markdown
Owner

Summary

  • Syntax-highlight inline diffs (TS/JS/TSX/JSON/Python/Bash) with a CSS-variable-based theme that flips under :root.dark. Bundle delta: ~29 KB gzipped.
  • Reviewed checkboxes + counter on the diff panel, persisted per (workspaceId, sessionId) via localStorage. Survives a full app relaunch and prunes when changed-files mutate.
  • "View in changes" icon-button on Edit/Write timeline rows. One click opens the diff panel and selects + scrolls to the file. Chevron expand-state is unaffected.

Out of scope (rejected): per-hunk accept/revert, inline approval cards, side-by-side.

Plan and review

  • Plan: plans/milestone-a-review-ux/plan.md. Confidence reached 85% after R2 plan review.
  • plan-loop: 2 native reviewers + 1 attempted Codex (MCP unavailable — gpt-5.5/gpt-5*/codex-* rejected by ChatGPT account; flagged at sign-off).
  • review-loop: R1 (4 native, distinct surfaces) found two [high] bugs — scroll-race when panel mounts empty and missing ' entity decode for apostrophes — plus a session-deps gap and a counter-flicker. Fixed in 35807fc. R2 verified all four resolved without regression.
  • simplify: collapsed dual-ref + pending state into a fileRequest prop, narrowed parseHljsHtml to private, unified the token renderer, swapped empty-set setItem("[]") for removeItem, fixed a latent test bug where commitAllInGitRepo blew away every change instead of just foo.ts.

Test plan

  • pnpm --filter @pi-gui/desktop run typecheck
  • pnpm --filter @pi-gui/desktop run test:e2e:runner -- apps/desktop/tests/core/review-ux.spec.ts apps/desktop/tests/core/mentions-diff.spec.ts — 6/6 passing
  • Full core lane — 58/60 passing; the 2 failures (provider-settings, unread-state) reproduce on the parent commit 30868cf and are unrelated to this change

Files

9 commits across apps/desktop/src/{App,conversation-timeline,diff-inline,diff-panel,timeline-item,icons}.tsx, new syntax-highlight.ts, reviewed-files-store.ts, styles/syntax-highlight.css, and tests/core/review-ux.spec.ts. No desktop-state.ts, main, preload, or IPC changes — reviewed-state is renderer-only by design.

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 25, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
pi-gui-website Ready Ready Preview, Comment Apr 25, 2026 4:41pm

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