Skip to content

chore(deps): bump @pierre/diffs to 1.3.x with theme major (stage 2 of 2) - #1189

Closed
backnotprop wants to merge 1 commit into
chore/pierre-diffs-1.2.12from
chore/pierre-diffs-1.3.x
Closed

chore(deps): bump @pierre/diffs to 1.3.x with theme major (stage 2 of 2)#1189
backnotprop wants to merge 1 commit into
chore/pierre-diffs-1.2.12from
chore/pierre-diffs-1.3.x

Conversation

@backnotprop

Copy link
Copy Markdown
Owner

Stage 2 of the @pierre/diffs upgrade. Bumps @pierre/diffs 1.2.12 -> 1.3.1 (newest stable; 1.3.2 does not exist, and all 1.3.0 betas/rcs were skipped) together with its pinned transitive majors @pierre/theme@2.0.0 and @pierre/theming@1.0.0.

Base branch note: opened against chore/pierre-diffs-1.2.12 (stage 1, #1188, reviewed but unmerged). GitHub will retarget this PR to main automatically when #1188 merges.

Edit mode is NOT enabled. No EditProvider, no edit props, no @pierre/diffs/edit imports anywhere in this PR (verified by grep; the dist/edit entry ships in the package but nothing references it).

What 1.3.x delivers for our usage

Per the stage audit: SlotPortals memoization (fewer portal re-renders in the all-files view), annotation wrap fixes, and the split-view offset fix. Plus the hover pipeline rework described below. SSR also picked up a dehydrated split-scroll shared-grid layout to prevent pre-hydration row shift (not exercised by our current preloadFile usage, which is unified single-file).

Hover-styling re-tune (the real work of this stage)

At 1.2.12, hovered changed lines were styled by per-selector rules ([data-hovered] { --mix-light: 80% ... }) that our buildLineBgOverrides in packages/review-editor/hooks/usePierreTheme.ts simply out-specified. In 1.3.x those rules are gone: hover is one central @media (pointer: fine) rule that re-mixes --diffs-computed-editor-active-line-bg 97% (light) / 91% (dark) toward --diffs-hover-mix-target (the addition/deletion base color on changed lines). Our hovered --mix-* values now feed the rest formula first and get multiplied by 0.97 / 0.91 on the way to the screen, so unchanged overrides would render hover slightly too strong (and asymmetrically: -1.35pp bg share in light, -3.15pp in dark at "normal").

Because color-mix(in lab, ...) is linear, the compensation is exact: the re-tune divides the target final bg shares by 0.97 / 0.91 (hoverEmittedMix()). Additionally, subtle (which used to ride Pierre's own per-selector hover values and would otherwise silently shift to 1.3's weaker defaults, 85.4%/72.8% effective vs the old 80%/75% deletion and 80%/70% addition) now pins the 1.2.x hover finals explicitly, preserving the old per-type split. Rest-state values are untouched at all intensities.

Measured before/after (CDP, headless Chrome 150, computed background-color of changed [data-line]s, default color theme)

Rest colors: byte-identical in all 12 configurations (dE(lab) = 0.000).

Hovered colors (before = 1.2.12 build, after = 1.3.1 build + re-tune):

Config Line 1.2.12 hovered 1.3.1 hovered dE(lab)
dark/subtle addition lab(23.13 -16.53 3.37) lab(23.13 -16.53 3.37) 0.003
dark/subtle deletion lab(16.39 18.90 7.86) lab(16.39 18.90 7.86) 0.002
dark/normal addition lab(46.63 -35.66 12.79) lab(46.63 -35.66 12.79) 0.001
dark/normal deletion lab(37.83 49.35 27.96) lab(37.83 49.35 27.96) 0.002
dark/strong addition lab(60.06 -46.59 18.16) lab(60.06 -46.59 18.16) 0.003
dark/strong deletion lab(48.55 64.58 38.00) lab(48.56 64.58 38.00) 0.003
light/subtle addition lab(89.13 -9.52 2.26) lab(89.13 -9.52 2.26) 0.003
light/subtle deletion lab(86.81 12.72 6.24) lab(86.81 12.72 6.24) 0.004
light/normal addition lab(76.23 -25.71 9.52) lab(76.23 -25.71 9.52) 0.001
light/normal deletion lab(69.85 35.44 20.44) lab(69.85 35.44 20.45) 0.002
light/strong addition lab(68.86 -34.97 13.66) lab(68.85 -34.97 13.66) 0.002
light/strong deletion lab(60.16 48.43 28.56) lab(60.16 48.43 28.56) 0.003

Max dE(lab) 0.004 across all 24 measurements (rest + hovered), pure rounding residue from the two-decimal emitted percentages; the perceptibility threshold is ~1.0. Hover screenshots (rest + hovered, all 12 configs, both versions) captured as evidence during verification.

Theme major (1.1.0 -> 2.0.0) verification

  • @pierre/theming@1.0.0 still registers Pierre themes under kebab ids (pierre-dark, pierre-light, ...); only displayName uses the "Pierre Dark" format. Our fallback lookup keys are unchanged and correct.
  • @pierre/theme@2.0.0 exports ./pierre-dark and ./pierre-light subpaths that theming imports; both resolve.
  • All 44 distinct shiki theme ids referenced by SHIKI_THEME_MAP exist in the installed @shikijs/themes@3.23.0 bundle: zero missing.
  • The new colorblind variants (protanopia-deuteranopia, tritanopia, soft, vibrant) are shipped but deliberately NOT added to the picker (separate follow-up as agreed).

bunfig.toml change

@pierre/theme and @pierre/theming added to minimumReleaseAgeExcludes alongside @pierre/diffs (review follow-up from #1188). Publish-date disclosure:

Package Published Age today
@pierre/diffs@1.3.1 2026-08-01 ~2 days (younger than the 7-day gate; installed via the pre-existing @pierre/diffs exclude)
@pierre/theme@2.0.0 2026-07-24 ~10 days
@pierre/theming@1.0.0 2026-07-24 ~10 days

Lockfile forensics (every changed line)

bun.lock changes are exactly nine lines, nothing else:

1-6. The six workspace "@pierre/diffs": "1.2.12" pins -> "1.3.1" (root, packages/ui, packages/server, packages/review-editor, apps/review, apps/pi-extension).
7. @pierre/diffs package entry: version 1.2.12 -> 1.3.1, its pinned deps @pierre/theme 1.1.0 -> 2.0.0 and @pierre/theming 0.0.2 -> 1.0.0, new integrity hash. All other deps of the entry unchanged (diff@9.0.0 scoped override intact).
8. @pierre/theme entry: 1.1.0 -> 2.0.0, new hash.
9. @pierre/theming entry: 0.0.2 -> 1.0.0, new hash (peer range on @pierre/theme still ^1.1.0, see supply-chain note).

Invariants held: root diff stays 8.0.4; shiki stays 3.23.0 (3.x; nothing dragged shiki or @shikijs/* to 4.x); no other package added, removed, or re-resolved.

Verification battery

Check Result
bun run typecheck (official script) PASS
Options-bag narrowing (controlledSelection / createEditor / onSelectedLinesChange absent from AllFilesCodeView options) PASS (bag contains none; typecheck clean; ad-hoc tsc -p packages/review-editor errors are byte-identical to the 1.2.12 baseline, all pre-existing)
bun test (full) 2793 pass / 0 fail (214 skip)
CI DOM sets (DOM_TESTS=1, exact CI file lists) 6 + 115 pass / 0 fail
Builds in order (apps/review -> build:hook -> build:opencode -> pi build) All PASS, no dist files tracked by git
Bundle size vs 1.2.12 branch review.html 18,066,117 -> 18,132,083 (+65,966 B, +0.37%); plan index.html 23,014,044 -> 23,026,880 (+12,836 B, +0.06%)
Row-height validation (VITE_PIERRE_VALIDATE_HEIGHTS=1, live dev server, scrolled full list) No NEW drift: 1.3.1 emits the exact same 16 pre-existing validation messages (byte-identical payloads, e.g. sticky deltas -66/-8, first-item -33) as 1.2.12 under the same headless harness; itemMetrics constants left unchanged
DOM data-attribute touchpoints ([data-separator], [data-expand-button], [data-diff-span], [data-line], diffs-container, [data-hovered]) All present in 1.3.1 dist and confirmed live against a real diff (28 separators / 28 expand buttons rendered)
onToken*-dropped-before-transformer quirk Still present in 1.3.1 (DiffHunksRenderer/FileRenderer pool options only honor explicit useTokenTransformer); the forced useTokenTransformer: true workaround is kept
CDP runtime, built 1.2.12 vs 1.3.1, standalone demo + real review server on this branch's diff Identical DOM counts in both scenarios; zero new console errors (the demo's parseLineType noise and two 404s occur identically on both versions)
Fresh-install #880 check PASS: bun pm pack of apps/pi-extension, clean-cache npm install of the tarball, preloadFile executed under plain Node 24 (prerenderedHTML length 53063, no module-resolution errors)
SSR parity (preloadFile 1.2.12 vs 1.3.1, same options as our call sites: { disableFileHeader: true }) Structural changes explained below
Supply chain npm audit signatures: 0 invalid/missing; publisher amadeusdemarzi for all three versions; maintainer set identical to @pierre/diffs' known six accounts (fat, imownbey, mdo, nicolas.pierreco, slexaxton, amadeusdemarzi); zero lifecycle scripts in all three manifests; no new dependencies vs 1.2.12

SSR parity: structural changes in preloadFile output

  1. Trailing-newline handling changed (behavioral): content ending in \n now renders one extra empty trailing context line (grid-row span +1). Every SSR-preloaded file preview of a newline-terminated file shows one extra blank row under 1.3.1. Cosmetic, but real.
  2. New CSS custom properties in the embedded style (hover pipeline + edit-mode vars): --diffs-hover-mix-target, --diffs-computed-editor-active-line-bg, --diffs-editor-active-line-source-mix, --diffs-selection-emphasis-mix-target, --diffs-warning-light, --diffs-warning-dark.
  3. Hover CSS consolidated into the central rule described above (matches the re-tune).
  4. New selectors shipped but inert for us: [data-editor-overlay], [data-editor-active-line], dehydrated split-scroll grid, an input, button font reset.
  5. Unchanged: wrapper markup, icon sprite set, token variable naming, all other nesting.

Supply-chain note (upstream packaging defect, not a compromise)

@pierre/theming@1.0.0 still declares its optional peer @pierre/theme: ^1.1.0, not bumped for the theme 2.0.0 major. Bun accepts this; a strict npm install of the three exact pins standalone hits ERESOLVE (needs --legacy-peer-deps), and the pi-extension tarball install resolves it by hoisting a separate @pierre/theme@1.1.0 for theming while @pierre/diffs gets its nested 2.0.0. The fresh-install smoke test passes regardless. Worth reporting upstream.

What remains for human visual QA

The CDP measurements cover computed colors of changed code lines at all three intensities in both modes, DOM parity, and console cleanliness. They do NOT cover: perceived hover feel while actually mousing across a large diff (gutter/line-number hover, selected-plus-hovered combinations, decoration-stacked lines), non-default color themes interacting with the new hover target chain, the extra SSR trailing blank line in reference previews, split-view hover, and general 1.3.x rendering polish across real-world diffs. A human pass over a real review session in light and dark at each intensity is still warranted before release.

… @pierre/theming 1.0.0

Retune buildLineBgOverrides for the 1.3.x hover pipeline: per-selector
hover mix rules are gone; hover is now one central rule mixing the
active-line bg 97% (light) / 91% (dark) toward --diffs-hover-mix-target.
Emitted hover --mix-* values are divided by those factors so the final
rendered hover bg shares match 1.2.12 exactly at normal and strong, and
subtle pins the 1.2.x hover finals (deletion 80/75, addition 80/70).

Also add @pierre/theme and @pierre/theming to the bunfig minimumReleaseAge
excludes (review follow-up from #1188).
@backnotprop

Copy link
Copy Markdown
Owner Author

Adversarial review (at 7485f987)

Verdict: merge as-is, after #1188, with a required rebase (see operational note). Every headline claim reproduced independently.

The hover math is proven, not plausible. All 41 color-mix calls in 1.3.1's stylesheet are in lab, so composition is uniformly linear and the compensation algebra is exact: both mix targets dereference the same variable, so the line color cancels and emitting f/k yields exactly f. Independent CDP measurement across all 24 combinations: rest colors dE 0.000000, hover max dE76 0.0037 against the PR's claimed 0.004, roughly 625x below the just-noticeable-difference threshold, with dark-scheme values matching the PR digit for digit. The subtle-intensity pins reproduce real 1.2.12 output. Better still: a re-run with deliberately foreign base colors (purple additions, teal deletions) measured identically, so the compensation is theme-independent by construction. The PR's deferral of non-default-theme hover QA to a human can be dropped; that risk is closed.

The peer-dep defect is real but inert. The ERESOLVE only fires when a consumer explicitly pins all three packages, which nothing does; the real published pi-extension tarball installs cleanly under default npm and strict-peer-deps on the realistic path, so this is not a #880-class regression. The hoisted duplicate theme@1.1.0 is genuinely loaded (proven by renaming it and watching SSR fail), but a byte comparison shows zero theme-data files differ between 1.1.0 and 2.0.0, so no server/client skew is possible. Cost is ~888KB of duplicate disk. The upstream peer-range bug is worth reporting as the PR says.

Provenance on the 3-day-old release is as strong as it gets: 237 of 237 first-party source files byte-identical to the diffs-v1.3.1 tag via shipped sourcesContent, the inlined theming dist 8/8 identical to the published package, nothing in the tarball the repo does not contain, zero lifecycle scripts, unchanged six-account maintainer set, signatures present.

The SSR trailing line is cosmetic and contained: every newline-terminated code-file reference gains one phantom numbered empty row (editor convention rather than git convention). All three consumers pass the HTML through opaquely, zero tests or fingerprints key on it, and the client-side fallback runs the same library so SSR and client agree.

Standard sweep all green: 2793 pass / 0 fail, DOM sets 6/6 and 163/163, four builds in order, exactly nine lockfile lines all accounted for with root diff and shiki unchanged, #880 fresh-install pass, live CDP A/B with zero console errors and identical DOM counts, height-validator output byte-identical across versions, bunfig excludes correct, no dist commits, no em dashes.

Findings, none blocking:

1. (Medium, operational) The stacked PR needs a rebase, not just a retarget. This repo squash-merges exclusively, so after #1188 is squashed, d2b5a526 is not an ancestor of main and this PR's diff would re-show all of stage 1. Required sequence: merge #1188, then git rebase --onto main d2b5a526 chore/pierre-diffs-1.3.x and force-push, then merge #1189.

2. (Medium) The theme "major" is a license change, and the PR never says so. theme 1.1.0 to 2.0.0 changes only LICENSE (MIT to Apache-2.0), package.json, and adds a NOTICE attributing GitHub's primer themes; every theme file is byte-identical. Two consequences: the PR undersells its own safety (the major carries zero pixels, which shrinks the human QA ask), and Apache-2.0 asks distributors to propagate NOTICE while the repo has no THIRD_PARTY_NOTICES file. Largely pre-existing (diffs and theming were already Apache-2.0), but this PR widens it. Follow-up issue, not a blocker.

3. (Low) DOM test count misreported in the PR body (says 6+115; actual 6+163, all passing either way).

4. (Low) The trailing blank line deserves its own line in the human QA checklist; it is the single most user-visible change here and is buried under structural changes.

5. (Info) The height-validator errors are pre-existing on both versions: the hand-maintained itemMetrics already drift against the DOM. Separate follow-up.

PR body honesty otherwise very strong: the young-release exception has its own disclosure table, the peer-dep defect is characterized correctly, bundle sizes and measurements match independent numbers exactly, and edit mode's absence is stated up front.

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