Repository navigation
4.2.0 release review: Low fixes - #452
Merged
Merged
Conversation
Release review A3: the history load had no catch, so a failed IndexedDB read left the panel on "Loading…" forever, and a rollback or duplicate that threw was swallowed (the busy flag cleared, nothing said why). The panel now shows an error with a Retry button when the load fails, and a failed rollback/duplicate raises an error toast instead of a silent no-op. Strings added in every locale. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZ3Pt89yubdPtYn5Wc8fjW
Release review A4 + SEC-1. A4: an unknown TYPE character in one GPS9 stream threw out of extractGpsPayload and aborted the whole GoPro import. A camera writes one payload per second, so a single corrupt one should cost one second, not the session. Each stream decode is now isolated (a bad GPS9 can still fall back to GPS5 in the same payload), and readComplex rejects a TYPE wider than its record instead of reading into the next one. SEC-1: a uniform-size stsz box is just a size and a uint32 count, so a hostile file could ask new Array() for ~4 billion entries and OOM the tab (the regression test OOMs the worker without the fix). Every sample occupies `size` bytes of the file, so the count is capped at fileSize / size. The other sample tables were already bounded by their box length. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZ3Pt89yubdPtYn5Wc8fjW
Release review B6: the shell runs one native player and insta360_player_close carries no player id, so an earlier NativePlayerElement closing late (its close() after a newer one opened, or an open that resolved after it was closed) killed whichever stream was live. Elements now record which one last opened the player and only that one may release it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZ3Pt89yubdPtYn5Wc8fjW
Release review T2: the check that decides "this shell can't do it, fall back quietly" was copied into nativeVideoExport.ts, nativeVideoStore.ts and insta360/ipc.ts, untested, and matched any message containing "unknown", "not found" or "not allowed" — so a real failure such as "file not found" or "unknown error" could be mistaken for an old shell and silently take the fallback path. It now lives once in lib/nativeUnavailable.ts and only accepts the `unsupported:` sentinel prefix or Tauri's own missing-command / ACL rejection shapes. The Insta360 copy had no callers and is removed (insta360SdkInfo already treats any error as unavailable). nativeVideoExport's exports and its handling of every real shell error string are unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZ3Pt89yubdPtYn5Wc8fjW
Release review T3: the runtime-cache route matched a hand-inlined /^\/(?:samples|loggers)\// because workbox-build serializes the urlPattern callback by its source text, so a closure over the constant would be empty in the worker. Adding a directory to the list without editing the regex would precache-exclude it and then never cache it at runtime. The list moves to scripts/deferredAssets.ts with a builder that derives the regex from it and splices it into the matcher's source; both are unit-tested, including the toString round-trip workbox does. The generated service worker route is byte-identical. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZ3Pt89yubdPtYn5Wc8fjW
Release review T4: the order a fresh load resolves in (confident course, then drag runs, then waypoint, then the nearest-track prompt) and the rule that a restored track/course shadows a saved drag tag lived only inline in useDataLoader. Both are extracted as pure functions (chooseAutoDetectPath, shouldRestoreDragSession) and covered, including that drag detection never runs when a course already matched. Extract only: the branch bodies are unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZ3Pt89yubdPtYn5Wc8fjW
Release review T5: useOfflineReadiness's only logic not already in the tested lib/offlineReadiness was the "how many deferred assets are cached" count. It moves there as countCachedAssets (injectable cache, a throwing lookup counts as a miss, no Cache API means zero) with tests; the hook keeps only browser plumbing. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZ3Pt89yubdPtYn5Wc8fjW
Release review PERF-3: the only font-brand use (the landing hero) is italic, so the upright variable face was ~35 KB in every offline install that no page ever drew. Only the italic face ships now; the index.css comment says how to bring the upright one back. Precache 4251.83 -> 4222.66 KiB (together with the PERF-2 split). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZ3Pt89yubdPtYn5Wc8fjW
Release review PERF-2 (+ REL-5). The eager graph statically imported the GoPro MP4/GPMF extractor (via FileImport and datalogParser), the Insta360 camera player and the native video-store bridge (via useVideoSync), so every web visitor downloaded code only a GoPro import or the LapWing shell ever runs. - goproDetect.ts holds the cheap name/ftyp gates the routers need on every import; goproImport (and mp4Boxes/gpmf behind it) is now dynamic-imported when a video actually arrives. - useVideoSync loads nativeVideoStore and NativePlayerElement on first use (type-only imports otherwise); the store's change-event constant moves to nativeVideoStoreEvents.ts so the hook can listen without pulling in the bridge. - VideoPlayer (lazy GraphViewTab chunk) loads the Insta360 IPC and the native export bridge only inside the native app. Main chunk: 754.68 -> 738.13 kB raw, 231.04 -> 225.82 kB gzip. None of the insta360_*/video_store_*/GPMF code remains in it. Also drops the two leftover console.log calls in VideoPlayer's save paths (Rule 7). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZ3Pt89yubdPtYn5Wc8fjW
- REL-3/DOC-2: fold the stray second "### Added" block (drag mode) into the first one and cite plan 0022. - REL-4: the read-only banner fix moves from Changed to Fixed. - Fixed entries for this pass's user-visible fixes (setup history errors, malformed/hostile GoPro files, Insta360 stream replacement). - DOC-5: plan 0022 is done, not IN PROGRESS. - DOC-6: CLAUDE.md's video-overlays entry described the per-widget *Overlay components plan 0023 deleted; it now names the single scene renderer. - DOC-7: README marks .360 GoPro import experimental, matching plan 0029's "untested". - CLAUDE.md / plan 0029 updated for goproDetect, the dynamic GoPro import, the deferredAssets module and the italic-only Archivo. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZ3Pt89yubdPtYn5Wc8fjW
The README already flags .360 (MAX/Fusion) as untested; the in-app supported-files list promised it unqualified in every language. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZ3Pt89yubdPtYn5Wc8fjW
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
lapwing | 19a332d | Commit Preview URL Branch Preview URL |
Oct 03 2026, 01:07 AM |
Coverage SummaryLines: 62.67% (9146/14592) · Statements: 61.69% · Functions: 58.51% · Branches: 59.09% Per-file coverage
|
Resolves the conflicts left by #450 and #451 landing first. CHANGELOG keeps both Fixed lists. useDataLoader tests keep the load-precedence tests and the course-clearing ones. countCachedAssets (plan 0027) now takes a predicate, so the readiness hook keeps the revision-aware isDeferredAssetCached check from #451 while the counting stays in the tested lib. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZ3Pt89yubdPtYn5Wc8fjW
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
2 participants
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.
Requested by Dove · project thread
Summary
Fixes all 16 Low findings from the 4.2.0 release review of #423. The High fixes are in #450 and the Medium fixes in #451.
Before:
.360GoPro import without the caveat that it's untested.After:
stszcounts are capped against the file size..360is marked experimental in all 7 locales and the README.How:
SetupHistoryPanelare caught and reported, with newsetupHistorykeys in all 7 locales (plan 0028).readComplexvalidates its TYPE width, andstszcounts are capped (plan 0029, with regression tests).nativePlayer.ts(plan 0025).gopro/goproDetect.ts, withgoproImportloaded by dynamic import.useVideoSyncloadsnativeVideoStoreandNativePlayerElementon first use.nativeVideoStoreEvents.ts.VideoPlayerloads the Insta360 IPC andnativeVideoExportonly inside the native app.console.logs inVideoPlayer.lib/nativeUnavailable.tsclassifier. It replaces three copies (the review found two) and has a tighter regex.scripts/deferredAssets.tsis the single source for the deferred dirs; the route regex and SW matcher are derived from it (generated SW is unchanged).useDataLoader, with 7 tests.countCachedAssetsmoved intolib/offlineReadiness, with tests..360is marked experimental.Type of Change
Checklist
bun run lintpassesbun run typecheckpassesbun run test:runpasses (3225 tests)bun run buildsucceedsREADME.md,CLAUDE.md,CHANGELOG.md)Notes for Reviewers
useDataLoader.ts,useOfflineReadiness.ts,vite.config.tsandVideoPlayer.tsx, depending on merge order.🤖 Generated with Claude Code
https://claude.ai/code/session_01NZ3Pt89yubdPtYn5Wc8fjW
Generated by Claude Code