fix(diffs): share SSR preloads and repair language-pack hydration#100487
Conversation
Render viewer and file documents from a single @pierre/diffs SSR preload per file (mode=both previously ran the full diff+highlight pipeline twice; 651ms -> 303ms on an 8-file patch), apply the file-mode font bump as a document-level override, and keep hydration payloads variant-faithful. Fix the language-pack runtime downgrading pack-only languages to plain text at hydration by defining a per-target build flag and forwarding it to payload normalization. Also: case-insensitive language hints, identical before/after short-circuit with details.changed, patch input failures classified as tool input errors, canonical config values now win over deprecated aliases, hash-pinned viewer runtime served immutable, truthful browser-vs-render errors, timing-safe artifact token compare, unref idle browser timer.
openclaw-barnacle
Bot
added
docs
…eline # Conflicts: # CHANGELOG.md
|
Codex review: needs real behavior proof before merge. Reviewed July 5, 2026, 7:18 PM ET / 23:18 UTC. Summary PR surface: Source +52, Tests +235, Docs +8, Other +3. Total +298 across 24 files. Reproducibility: yes. Source inspection of the base shows Review metrics: 1 noteworthy metric.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Land the diffs fixes with runtime defaults as the canonical compatibility path, after maintainer acceptance of the config-schema change and exact-head CI/proof gates. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection of the base shows Is this the best way to solve the issue? Yes, with a compatibility caveat. Sharing the preload, injecting a per-target language-pack flag, and moving alias-sensitive defaults to runtime normalization are maintainable fixes; the schema-default change needs maintainer acceptance before merge. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 2525ea41fd09. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +52, Tests +235, Docs +8, Other +3. Total +298 across 24 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2480bb7955
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| ### Fixes | ||
|
|
||
| - **Diffs rendering:** render viewer and image output from one SSR preload, preserve language-pack highlighting through hydration, normalize language hints case-insensitively, skip identical before/after inputs with an explicit `changed` result, report truthful file-render and input errors, cache hash-pinned viewer runtimes, and prefer canonical file settings over stale aliases. (#100487) |
There was a problem hiding this comment.
This commit is a normal diffs fix, but AGENTS.md:238 says CHANGELOG.md is release-only and must not be edited for normal PRs or direct main fixes because release generation owns it. Keeping this hand-added Unreleased line makes the release notes non-canonical and can duplicate generated entries when the release tooling derives notes from merged PRs/direct commits, so remove the changelog edit and keep the release-note context in the PR or commit message instead.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🐚 platinum hermit
…eline # Conflicts: # CHANGELOG.md
…enclaw#100487) * fix(diffs): share SSR preloads and repair language-pack hydration Render viewer and file documents from a single @pierre/diffs SSR preload per file (mode=both previously ran the full diff+highlight pipeline twice; 651ms -> 303ms on an 8-file patch), apply the file-mode font bump as a document-level override, and keep hydration payloads variant-faithful. Fix the language-pack runtime downgrading pack-only languages to plain text at hydration by defining a per-target build flag and forwarding it to payload normalization. Also: case-insensitive language hints, identical before/after short-circuit with details.changed, patch input failures classified as tool input errors, canonical config values now win over deprecated aliases, hash-pinned viewer runtime served immutable, truthful browser-vs-render errors, timing-safe artifact token compare, unref idle browser timer. * docs(changelog): link diffs rendering entry to PR * test(diffs): narrow manifest validation results before value access * test(tooling): allowlist diffs viewer-client define suppression
…enclaw#100487) * fix(diffs): share SSR preloads and repair language-pack hydration Render viewer and file documents from a single @pierre/diffs SSR preload per file (mode=both previously ran the full diff+highlight pipeline twice; 651ms -> 303ms on an 8-file patch), apply the file-mode font bump as a document-level override, and keep hydration payloads variant-faithful. Fix the language-pack runtime downgrading pack-only languages to plain text at hydration by defining a per-target build flag and forwarding it to payload normalization. Also: case-insensitive language hints, identical before/after short-circuit with details.changed, patch input failures classified as tool input errors, canonical config values now win over deprecated aliases, hash-pinned viewer runtime served immutable, truthful browser-vs-render errors, timing-safe artifact token compare, unref idle browser timer. * docs(changelog): link diffs rendering entry to PR * test(diffs): narrow manifest validation results before value access * test(tooling): allowlist diffs viewer-client define suppression
1 participant
Deep review of the
diffsextension surfaced one real bug, one config-precedence bug, and a 2x render cost in the default mode. This PR fixes the verified findings.Render pipeline: stop rendering every diff twice
mode=both(the default) ran the full SSR pipeline — diff computation plus shiki highlighting — twice per file, because the viewer and file variants differ only in an inertunsafeCSSstring (the file variant bumps--diffs-font-sizeto >=16px). The expensiveasyncRendernever reads that CSS; SSR just appends it as a<style>element.Now the preload runs once per file and both documents share it; the file-mode document applies the font bump as a document-level override on the shadow host (outer host rules beat
:hostcustom properties), and hydration payloads keep their per-variant options.Measured on an 8-file / 240-changed-line patch (5-run average after warmup, M-series):
Language pack: highlighting survived SSR but died at hydration
The language-pack runtime re-normalized payload languages without
languagePackAvailable, so pack-only languages (abap, haskell, zig, ...) were rewritten totextand the boot-time rerender replaced the correctly server-highlighted HTML with plain text. The 10MB pack bundle was functionally dead in the interactive viewer. The build now defines a per-target flag (__OPENCLAW_DIFFS_LANGUAGE_PACK__, same pattern as__OPENCLAW_VERSION__) and hydration forwards it.Also fixed
lang: "Python"/"TypeScript"silently degraded to plain text; hints are lowercased before lookup.defaults.fileFormat: "png"was overridden by a stale deprecatedimageFormat: "pdf"because "equal to schema default" was misread as "not set". Explicit canonical values now always win; schema defaults on the two alias-carrying fields moved to runtime normalization so alias-only configs keep working (covered by manifest-validated tests).details.changed: falsewith a clear message, and rendered results now carrychanged: true(documented in README, docs, skill, and prompt guidance).ToolInputErrorinstead of internal tool failures.viewer-runtime.jsis content-hash-pinned by the loader but was servedno-store, re-downloading on every page view. It is nowpublic, max-age=31536000, immutable; the loader and HTML stayno-store.safeEqualSecret), relevant withsecurity.allowRemoteViewer: true.Proof
vitest run --config test/vitest/vitest.extension-diffs.config.ts— 145/145 passingtsc --noEmit -p extensions/diffs/tsconfig.json— clean; scoped oxlint — cleanViewer runtime assets are untracked build outputs; the next packaging build picks up the hydration fix via
scripts/build-diffs-viewer-runtime.mjs.