refactor(export): build static snapshot from shared live modules#60
Merged
Conversation
The Gist export runtime was a ~2000-line hand-maintained parallel copy of the live session renderer (internal/ui/live_templates/export/app/*.js), kept in sync by discipline alone and prone to silent drift. Replace it with a Vite-built bundle from a new web/src/export/export-entry.js that imports the exact same web/src/session/ rendering modules as the live app (data, tree, filter, format, render, navigation, ui), omitting all live-only code (SSE, chat, artifacts, annotations). vite.config.export.js emits a single self-contained IIFE that reads window.marked/window.hljs from the inlined vendor scripts; export.go now embeds that one built file instead of concatenating nine hand-written ones. - Delete export/app/*.js; export.go shrinks to a single //go:embed. - Add TestExportBundleIsSelfContained to fail the build if a live-only symbol (EventSource/runLiveReload) ever leaks into the export graph. - Repoint the tests that grepped the old unminified bundle at the canonical web/src source files (the minified bundle is no longer greppable). - export.js + dist-export are generated; gitignore them. npm run build now runs build:export after the live build, so make build/check stay green.
The load-earlier spec built a 1600-message session; each "load earlier" click re-renders the whole conversation on the browser main thread. Under the full E2E matrix (8+ browsers + Node runner + one shared pi-web server) CPU starvation intermittently delayed that work — and even Playwright's own poll — past the assertion timeout. The load always completed; the test just gave up first. It flaked locally (workers=undefined, retries=0); CI masked it via retries. Make the work cheap and bound the residual flake: - session_page.go: LargeSessionThreshold/LargeSessionTailEntries become env-configurable (PI_WEB_LARGE_SESSION_THRESHOLD / _TAIL_ENTRIES), defaults unchanged. The existing comment already invited this seam. - e2e/lib/server.ts: lower them to 100/50 so the spec triggers the identical pagination path with a ~150-entry session that renders instantly. Chosen well above every other spec's session size (max ~34) so nothing else truncates. - load-earlier.spec.ts: 150-entry session, modest 15s waits, and retries: 2 to absorb rare contention spikes — a real regression still fails every attempt. - pagination_test.go: const n -> n := (threshold is now a var, not a const).
Gist previews load the export HTML in a sandboxed iframe without allow-same-origin, where even reading window.localStorage throws SecurityError. The unified export bootstrap read target.localStorage eagerly to pass into setupSessionUi, so that throw aborted the whole bundle and left a blank page. (The old hand-written export only touched localStorage inside try/catch, so it never surfaced.) - export-entry.js: wrap localStorage access in a safeLocalStorage() helper that falls back to an in-memory shim when the property access throws — a static snapshot has nothing to persist anyway. Returning a shim (never undefined) also keeps the shared modules off their globalThis.localStorage default, which throws the same way. - share.go: add a GET ?preview=1 mode that returns the rendered export HTML directly, skipping the gh/gist round-trip. Useful for eyeballing a snapshot before sharing, and gives tests a network-free way to load the real page. - share.spec.ts: load the previewed snapshot into a sandboxed (allow-scripts, no allow-same-origin) iframe and assert the conversation renders — the regression guard. Verified it fails without the localStorage fix. - share_test.go: cover preview mode (returns HTML, no gist; requires id).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Collapses a long-standing duplication in the static export/Gist renderer and fixes a blank-page regression it surfaced.
internal/ui/live_templates/export/app/*.js), kept in sync by discipline and prone to drift. It's replaced by a Vite-built bundle from a newweb/src/export/export-entry.jsthat imports the sameweb/src/session/rendering modules as the live app, omitting all live-only code (SSE, chat, artifacts, annotations).export.gonow embeds one built file instead of concatenating nine.allow-same-origin, where even readingwindow.localStoragethrowsSecurityError— which aborted the unified bootstrap and left a blank page. The export now guards localStorage behind an in-memory shim.GET /share?id=…&preview=1returns the rendered export HTML directly (nogh/gist) for local preview; the share E2E now loads that snapshot in a sandboxed iframe and asserts the conversation renders (verified to fail without the fix).Related issue
Closes #
Type of change
feat— new feature (local?preview=1export mode)fix— bug fix (sandboxed-iframe blank page)docs— documentation onlyrefactor— code change that neither fixes a bug nor adds a featurestyle— formatting / UI styling, no behavior changetest— adding or updating testschore— build, tooling, or maintenanceLive vs. Export
web/src/session/rather than a hand-kept copy, so the two render paths can no longer drift (replaces the old manual-sync rule)TestExportBundleIsSelfContainedTesting
go test ./...— passgo vet ./...— passcd web && npm test(vitest) — 466 passmake build(frontend build incl. export bundle +go build) — passmake e2e— full Playwright matrix green; share + load-earlier specs verified across all browser projects