Skip to content

refactor(ui): split pierre diff theme from markdown fence theme (#705) - #708

Merged
Astro-Han merged 2 commits into
devfrom
claude/i705-pierre-token-bind
May 18, 2026
Merged

refactor(ui): split pierre diff theme from markdown fence theme (#705)#708
Astro-Han merged 2 commits into
devfrom
claude/i705-pierre-token-bind

Conversation

@Astro-Han

@Astro-Han Astro-Han commented May 17, 2026

Copy link
Copy Markdown
Owner

Summary

  • Fork a new PawWorkDiff shiki theme alongside OpenCode in marked.tsx. PawWorkDiff spreads OpenCode's tokenColors and semanticTokenColors and only overrides editor.background to var(--bg-base). OpenCode itself is restored to editor.background = var(--code-surface), restoring the [Task] Design token system debt log (drift, redundancy, dual systems) #642 PR0 contract.
  • Point pierre.createDefaultOptions at theme: "PawWorkDiff". Markdown fence-code keeps using OpenCode. The two surfaces are now decoupled at the theme layer.
  • Bind pierre's --diffs-bg-{addition,deletion}{,-number}-override to PawWork's --diff-add / --diff-del semantic tokens. The dark :host([data-color-scheme='dark']) add/del branch is dropped because the tokens already carry their own light/dark values (and the dropped branch had drifted to rgba(63,179,115,0.18) vs the token's rgba(58,176,107,0.15)).
  • Flatten the hunk separator + chevron expand button (--diffs-bg-{buffer,context,separator}-override: transparent) and hide the 2 px gutter seam (--diffs-gap-style: 2px solid transparent) so non-add/non-del rows sit flat on the PawWorkDiff surface.
  • Drop the local [data-column-number] { background-color: var(--bg-base) } rule that was clobbering pierre's own number-column tint on add/del rows.
  • Switch diffIndicators from bars (4 px side-stripe — banned by DESIGN.md absolute bans) to classic (+/− char markers).

Why

Resolves #705. The issue surfaced as "diff viewer is cool gray, not warm --bg-base," but the root cause was deeper: pierre serializes editor.background from its shiki theme into a <pre> inline style, and that inline value wins the cascade against every --diffs-bg rule in unsafeCSS. The earlier iteration on this branch (1df0105) routed the fix through OpenCode.editor.background = var(--bg-base), but that surface is shared with markdown fence-code and is pinned to var(--code-surface) by the #642 PR0 contract (theme-parity.test.ts:354). CI broke; chat-thread fence-code became visually identical to body in dark.

The fence-code surface and the diff surface have different jobs — fence-code is a card-like alpha overlay against chat body, the diff viewer is a flat surface so add/del row tints carry the signal — so coupling them through one theme is the wrong architecture. Forking PawWorkDiff lets each surface declare its own editor.background while sharing all syntax highlighting (tokenColors / semanticTokenColors via spread). Future edits to chat code rendering cannot silently move the review diff, and vice versa.

Related Issue

Closes #705.

Adjacent: #709 (rework dark mode color system) will revisit --bg-base / --code-surface / etc. holistically; this PR ships independently.

Human Review Status

Pending. A human should make the final merge decision after reviewing the final diff and verification evidence.

Review Focus

  • The new PawWorkDiff theme is the load-bearing change. It is OpenCode with one field overridden; both themes share tokenColors via spread so there is no duplicated palette to maintain.
  • OpenCode.editor.background is back to var(--code-surface). The #642 PR0 theme-parity contract (packages/ui/test/theme-parity.test.ts:354) is green again.
  • The :host([data-color-scheme='dark']) block under [data-diff], [data-file] now contains only selection-related tokens. Add/del live in the PawWork --diff-add / --diff-del tokens.
  • marked.tsx shows a large diff because the OpenCode theme object was extracted to a top-level OPENCODE_THEME const (indent shift). Use "Hide whitespace changes" or git diff -w to see the real delta (+29 / -11 lines logical).

Risk Notes

  • Markdown fence-code rendering is unchanged from dev. The earlier iteration's "unified --bg-base background" idea is reverted in this commit; that direction will be revisited under [Feature] Rework dark mode color system #709 if it makes sense at all.
  • diffIndicators: "bars""classic" changes the per-row marker style; affects every diff render path.
  • DESIGN.md L191 + L482 were updated locally to permit the --diff-add / --diff-del tokens and to align the dark alpha to the runtime 0.15 value; docs/ is excluded from this worktree's tracking, so the doc change is not in this PR — flagged here for spec coherence.

How To Verify

bun --cwd packages/ui test test/theme-parity.test.ts → 202 pass / 0 fail (includes #642 PR0 contract)
bun turbo typecheck → 8 packages successful (tsgo --noEmit)
E2E chat fence-code light + dark (ad-hoc probe spec):
  inline style    = background-color: var(--code-surface)
  light fence bg  = rgba(0, 0, 0, 0.04)      (= --code-surface, alpha overlay)
  dark fence bg   = rgba(255, 255, 255, 0.06) (= --code-surface, alpha overlay)
  body bg matches --bg-base in both modes; fence sits above on the alpha overlay

Screenshots or Recordings

Review-panel + chat-fence screenshots were taken locally via E2E (docs/debug-screens/i705-{chat,}-{light,dark}.png). Attaching them to the PR body requires the GitHub web UI's drag-and-drop. I will add them as a follow-up comment once the human reviewer is ready.

Checklist

  • Human review status is stated above as pending, approved, or not required
  • I linked the related issue, or stated why there is no issue
  • This PR has exactly one type label (bug, enhancement, task, or documentation), at least one primary routing label (app, ui, platform, harness, or ci), and exactly one priority label (P0 to P3), or I requested maintainer labeling
  • I described the review focus and any meaningful risks
  • I listed the relevant verification steps and the key result for each
  • I did not introduce unrelated refactors, dependencies, generated files, or file changes beyond the stated scope
  • I manually checked visible UI or copy changes when needed, with screenshots or recordings
  • I considered macOS and Windows impact for platform, packaging, updater, signing, paths, shell, or permissions changes
  • I called out docs, release notes, dependencies, permissions, credentials, deletion behavior, generated content, or local file changes when relevant
  • 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

…--diff-del (#705)

pierre's OpenCode theme wrote `editor.background = var(--code-surface)` as an
inline style on `<pre>`, so every stylesheet attempt to set `--diffs-bg` lost
the cascade and the diff viewer rendered a cool half-transparent gray shell.
Switch `editor.background` to `var(--bg-base)` and point the add/del row
overrides at PawWork's `--diff-add` / `--diff-del` semantic tokens, which
already carry the correct light/dark values; this also removes the dark
`:host([data-color-scheme='dark'])` branch and the drift it was hiding
(rgba 63,179,115,0.18 vs the token's 58,176,107,0.15).

Flatten the hunk separator + chevron expand button by overriding
`--diffs-bg-{buffer,context,separator}` to transparent, kill the 2 px gutter
seam with `--diffs-gap-style: 2px solid transparent`, and switch
`diffIndicators` from `bars` (4 px side-stripe banned by DESIGN.md) to
`classic` (+/− char markers).

Drop the local `[data-column-number] { background-color: var(--bg-base) }`
rule that was clobbering pierre's own add/del number-column tint.
@github-actions github-actions Bot added the ui Design system and user interface label May 17, 2026
@coderabbitai

coderabbitai Bot commented May 17, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

The PR unifies editor and diff styling by updating the OpenCode editor background to use --bg-base and refactoring diff color derivation to rely on explicit override variables instead of color-mix. Default diff indicators change from "bars" to "classic", and new transparent gap styling is added.

Changes

Diff and Editor Theme Unification

Layer / File(s) Summary
Editor background color synchronization
packages/ui/src/context/marked.tsx
OpenCode editor background token updated from var(--code-surface) to var(--bg-base) for consistent base color usage.
Diff styling overrides refactoring
packages/ui/src/pierre/index.ts
Explicit --diffs-bg-* override variables replace prior color-mix derivation, buffer/context/separator backgrounds made transparent, and redundant background-color declaration removed to expose row backgrounds.
Diff indicators and gap styling
packages/ui/src/pierre/index.ts
Default diffIndicators changed from "bars" to "classic", and new --diffs-gap-style: "2px solid transparent" variable added to styleVariables for transparent column seam spacing.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

Suggested labels

P3, ui

Poem

🐰 Colors blend and themes align,
Editor and diffs now design,
From color-mix to crisp variables bright,
Classic indicators set just right,
A transparent seam—oh what a sight!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ⚠️ Warning The PR title states 'split pierre diff theme from markdown fence theme' but the actual changes are about binding pierre diff tokens to PawWork theme and unifying backgrounds, not splitting themes. Update the title to accurately reflect the main objective: 'fix(ui): bind pierre diff tokens to PawWork theme' or similar, matching the PR's actual purpose of alignment and integration rather than separation.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description is comprehensive and well-structured, covering all major template sections including summary, rationale, related issues, review focus, risk notes, and verification steps.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/i705-pierre-token-bind

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.

@Astro-Han Astro-Han added the bug Something isn't working label May 17, 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 non-doc, non-test paths outside the low-risk bucket).

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 P2 Medium priority label May 17, 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 OpenCode theme background to use var(--bg-base) and refactors Pierre diff styling by replacing internal CSS variables with semantic token overrides. It also changes the diff indicators from 'bars' to 'classic' and adjusts column spacing for better visual consistency. A critical issue was identified where the removal of --diffs-deletion-base and --diffs-addition-base would break word-level diff highlights and selection rules, so these should be restored.

Comment thread packages/ui/src/pierre/index.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/ui/src/pierre/index.ts`:
- Line 199: The CSS variable "--diffs-gap-style" in
packages/ui/src/pierre/index.ts is not a recognized pierre variable and has no
effect; either confirm pierre 1.1.0-beta.18 supports it via authoritative docs
or remove it and implement a supported alternative: delete the
"--diffs-gap-style" entry and instead apply a supported visual fix (for example,
add a box-shadow or use background-clip/padding-box on the diff row selector
such as the component/class that renders rows in pierre to hide the seam while
keeping the row tint), or add a custom pierce-through rule if pierre exposes
one; update the relevant styling export in index.ts and any associated
class/selector (the diff row renderer) to use the chosen approach.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: df09889d-b430-4bb1-ad4c-32be063a27da

📥 Commits

Reviewing files that changed from the base of the PR and between 94b4792 and 1df0105.

📒 Files selected for processing (2)
  • packages/ui/src/context/marked.tsx
  • packages/ui/src/pierre/index.ts

Comment thread packages/ui/src/pierre/index.ts
@github-actions

github-actions Bot commented May 17, 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 / homepage-cold 40 -> 24 (-16) 64 -> 48 (-16) 97 -> 64 (-33) 47 -> 14 (-33) 33.3 -> 16.8 (-16.5) 166.7 -> 166.7 (0) 4 -> 2 (-2) 0 -> 0 (0) pass
default / long-session-input-lag 40 -> 48 (+8) 48 -> 64 (+16) 0 -> 0 (0) 0 -> 0 (0) 16.8 -> 16.7 (-0.1) 16.8 -> 16.8 (0) 0 -> 0 (0) 0 -> 0 (0) pass
default / session-streaming-long 56 -> 48 (-8) 56 -> 72 (+16) 0 -> 0 (0) 0 -> 0 (0) 16.8 -> 16.8 (0) 33.4 -> 33.4 (0) 0 -> 0 (0) 0 -> 0 (0) pass
default / tool-call-expand 24 -> 16 (-8) 24 -> 24 (0) 0 -> 0 (0) 0 -> 0 (0) 16.7 -> 16.8 (+0.1) 16.7 -> 16.8 (+0.1) 0 -> 0 (0) 0 -> 0 (0) pass
default / tool-default-open-heavy-bash 24 -> 24 (0) 32 -> 32 (0) 66 -> 67 (+1) 18 -> 28 (+10) 50 -> 50 (0) 166.6 -> 150.1 (-16.5) 2 -> 3 (+1) 0 -> 0 (0) pass
default / terminal-side-panel-open 48 -> 48 (0) 48 -> 48 (0) 0 -> 0 (0) 0 -> 0 (0) 33.3 -> 33.3 (0) 33.3 -> 33.3 (0) 0 -> 0 (0) 0 -> 0 (0) pass
default / session-scroll-reading 24 -> 32 (+8) 32 -> 32 (0) 0 -> 0 (0) 0 -> 0 (0) 16.7 -> 16.8 (+0.1) 16.7 -> 16.8 (+0.1) 0 -> 0 (0) 0.505 -> 0.505 (0) warn: cls

The earlier fix on this branch (1df0105) routed the pierre diff fix through
the shared OpenCode shiki theme by changing editor.background to var(--bg-base).
That fixed the diff but broke the #642 PR0 contract that pins editor.background
to var(--code-surface) for markdown fence-code, because the two surfaces were
sharing one theme — and they have different visual jobs: fence-code wants an
alpha-overlay card against chat body, the diff viewer wants the surface to
recede so add/del row tints carry the signal.

Fork a "PawWorkDiff" theme that spreads OpenCode's tokenColors and only
overrides editor.background to var(--bg-base); point pierre.createDefaultOptions
at it. OpenCode itself returns to var(--code-surface) and the theme-parity
guard goes green again. pierre's add/del row overrides remain bound to
PawWork's --diff-add / --diff-del semantic tokens (which carry their own
light/dark values), so the dark :host([data-color-scheme='dark']) branch
that had drifted to rgba(63,179,115,0.18) vs the token's
rgba(58,176,107,0.15) stays removed.

Net effect: markdown fence-code visuals are unchanged from main; review-panel
diff renders on the warm --bg-base shell with PawWork add/del tints; the two
code surfaces are now decoupled at the theme layer so future edits to one
cannot silently move the other.
@Astro-Han Astro-Han changed the title fix(ui): bind pierre diff tokens to PawWork theme (#705) refactor(ui): split pierre diff theme from markdown fence theme (#705) May 18, 2026
@Astro-Han
Astro-Han merged commit d16641d into dev May 18, 2026
33 of 34 checks passed
@Astro-Han
Astro-Han deleted the claude/i705-pierre-token-bind branch May 18, 2026 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working P2 Medium priority ui Design system and user interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Review panel diff colors do not match DESIGN.md — pierre tokens never bound to PawWork theme

1 participant