Conversation
Wipes the cached refPoints/ directory across all OPFS scenarios so the next scenario load re-imports ref points from the read folder's *.zip recordings. Useful when the OPFS cache becomes stale relative to the zip files. - Framework: new clearRefPointsCacheForAllScenarios() in file-system.ts (collects per-scenario errors, treats missing caches as no-ops). - Recorder: new "Cache" section in settings modal with confirm dialog. - main.ts: re-imports ref points for the active scenario after clearing so the H3 proximity cache and 3D/2D markers refresh immediately; reports the result via toast. - Tests: confirm/cancel wiring on the button, framework happy-path test for the multi-scenario clear. Co-authored-by: Copilot <copilot@github.com>
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
gps-plus-slam | 99a7f50 | Commit Preview URL Branch Preview URL |
May 07 2026, 04:06 AM |
📝 WalkthroughWalkthroughThis PR adds a new maintenance feature to clear cached reference points across all OPFS scenarios. It introduces a core framework function ChangesClear Reference Point Cache
Sequence DiagramsequenceDiagram
actor User
participant UI as Settings Modal
participant Main as Main App
participant Storage as OPFS Storage
participant Toast as Toast Notifier
User->>UI: Clicks "Clear Cache" button
UI->>UI: Shows confirmation dialog
User->>UI: Confirms action
UI->>Main: Calls handleClearRefPointCache()
Main->>Storage: clearRefPointsCacheForAllScenarios()
Storage->>Storage: Iterate all scenarios
Storage->>Storage: Delete refPoints/ dirs
Storage-->>Main: Return result (count, errors)
alt Current scenario loaded
Main->>Storage: Reload scenario refPoints
Storage-->>Main: RefPoints reimported
else No current scenario
Main->>Main: Clear in-memory refPoints
end
Main->>Toast: Show success/empty/failure message
Main->>Toast: Log outcome
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a maintenance feature to clear the cached reference point directory across all scenarios in the Origin Private File System (OPFS), facilitating a fresh re-import from source ZIP recordings. The implementation includes the core logic in the storage framework, a new "Clear Reference Point Cache" button in the recorder app's settings modal with a confirmation dialog, and corresponding unit tests. Feedback suggests refining the in-memory state clearing logic to avoid disrupting the UI when the application is in Replay mode.
Comment on lines
+364
to
+368
| } else { | ||
| // No active scenario — clear in-memory imported ref points so any | ||
| // proximity checks don't keep referring to stale entries. | ||
| refPointHandlers.setImportedRefPoints([]); | ||
| } |
There was a problem hiding this comment.
Clearing the in-memory imported reference points when no scenario is active will also affect the UI in Replay mode. If a user is currently viewing a replay (which loads ref points from a ZIP, not the OPFS cache), clicking 'Clear Cache' in settings will cause their current replay markers to disappear. Consider checking if the app is in replay mode before clearing the in-memory state.
Suggested change
| } else { | |
| // No active scenario — clear in-memory imported ref points so any | |
| // proximity checks don't keep referring to stale entries. | |
| refPointHandlers.setImportedRefPoints([]); | |
| } | |
| } else if (!replayHandlers.getIsReplayMode()) { | |
| // No active scenario and not in replay mode — clear in-memory imported | |
| // ref points so any proximity checks don't keep referring to stale entries. | |
| refPointHandlers.setImportedRefPoints([]); | |
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
GpsPlusSlamJs_RecorderApp/src/ui/settings-modal.ts (1)
141-143: ⚡ Quick winGuard against concurrent cache-clear requests.
handleClearRefPointCache()can be triggered multiple times before completion, which can run overlapping clears. Add an in-flight guard (and optionally disable the button during execution).Proposed re-entrancy guard
let onClearRefPointCache: (() => void | Promise<void>) | null = null; +let isClearingRefPointCache = false; async function handleClearRefPointCache(): Promise<void> { + if (isClearingRefPointCache) { + log.debug('Clear ref-point cache already in progress'); + return; + } if (!onClearRefPointCache) { log.warn('Clear ref-point cache requested but no callback is wired'); return; } + isClearingRefPointCache = true; const confirmed = await showConfirmDialog({ @@ - try { + try { await onClearRefPointCache(); log.info('Ref-point cache cleared'); } catch (err) { log.error('Failed to clear ref-point cache:', err); + } finally { + isClearingRefPointCache = false; } }Also applies to: 478-502
🤖 Prompt for 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. In `@GpsPlusSlamJs_RecorderApp/src/ui/settings-modal.ts` around lines 141 - 143, handleClearRefPointCache can be invoked re-entrantly and must be serialized: add an in-flight guard (e.g., a boolean like isClearingRefPointCache) scoped near the UI module and check it at the start of the click handler and inside any functions that call handleClearRefPointCache; if the guard is true, return early. When starting the operation set isClearingRefPointCache = true and when finished (or on error) set it back to false in a finally block, and optionally disable/enable the btnClearRefPointCache DOM element while the operation runs. Apply the same guard pattern to the other invocation(s) that call handleClearRefPointCache so overlapping clears cannot occur.GpsPlusSlamJs_AppFramework/src/storage/file-system.test.ts (1)
328-361: ⚡ Quick winTest only verifies cache deletion for one seeded scenario.
The test says “every scenario” but only asserts
refPointsremoval for ScenarioA (Line 359). Add the same assertion for ScenarioB to validate full behavior.Proposed test hardening
const reA = await setCurrentScenario('ScenarioA'); await expect(reA!.getDirectoryHandle('refPoints')).rejects.toThrow(); + const reB = await setCurrentScenario('ScenarioB'); + await expect(reB!.getDirectoryHandle('refPoints')).rejects.toThrow();🤖 Prompt for 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. In `@GpsPlusSlamJs_AppFramework/src/storage/file-system.test.ts` around lines 328 - 361, The test seeds refPoints for both ScenarioA and ScenarioB but only asserts removal for ScenarioA; update the test in file-system.test.ts to also verify ScenarioB's refPoints directory was removed by calling setCurrentScenario('ScenarioB') (e.g., const reB = await setCurrentScenario('ScenarioB')) and then await expect(reB!.getDirectoryHandle('refPoints')).rejects.toThrow(); so both seeded scenarios are validated after clearRefPointsCacheForAllScenarios().
🤖 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 `@GpsPlusSlamJs_AppFramework/src/storage/file-system.ts`:
- Around line 395-405: The function clearRefPointsCacheForAllScenarios currently
proceeds when OPFS root/scenariosDir is unavailable and returns zeroed counts;
instead add a guard at the start of clearRefPointsCacheForAllScenarios that
checks scenariosDir (and any OPFS root dependency) and immediately throw a
descriptive Error (or return a ClearRefPointsCacheResult with an explicit
surfaced error) if it's falsy before calling listScenarios(); update
callers/tests accordingly so they expect the thrown error/explicit failure
rather than a silent no-op.
In `@GpsPlusSlamJs_RecorderApp/src/main.ts`:
- Around line 359-367: When folderManager.loadAndDisplayRefPoints(currentHandle)
throws, the catch only logs the error so stale imported ref points remain;
update the catch block to clear in-memory imported points by calling
refPointHandlers.setImportedRefPoints([]) (and keep the existing log.warn with
the error) so proximity checks won't use stale entries after a failed re-import.
---
Nitpick comments:
In `@GpsPlusSlamJs_AppFramework/src/storage/file-system.test.ts`:
- Around line 328-361: The test seeds refPoints for both ScenarioA and ScenarioB
but only asserts removal for ScenarioA; update the test in file-system.test.ts
to also verify ScenarioB's refPoints directory was removed by calling
setCurrentScenario('ScenarioB') (e.g., const reB = await
setCurrentScenario('ScenarioB')) and then await
expect(reB!.getDirectoryHandle('refPoints')).rejects.toThrow(); so both seeded
scenarios are validated after clearRefPointsCacheForAllScenarios().
In `@GpsPlusSlamJs_RecorderApp/src/ui/settings-modal.ts`:
- Around line 141-143: handleClearRefPointCache can be invoked re-entrantly and
must be serialized: add an in-flight guard (e.g., a boolean like
isClearingRefPointCache) scoped near the UI module and check it at the start of
the click handler and inside any functions that call handleClearRefPointCache;
if the guard is true, return early. When starting the operation set
isClearingRefPointCache = true and when finished (or on error) set it back to
false in a finally block, and optionally disable/enable the
btnClearRefPointCache DOM element while the operation runs. Apply the same guard
pattern to the other invocation(s) that call handleClearRefPointCache so
overlapping clears cannot occur.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2781b11c-6f08-453f-bbab-810e2f40b816
📒 Files selected for processing (7)
GpsPlusSlamJs_AppFramework/src/storage/file-system.test.tsGpsPlusSlamJs_AppFramework/src/storage/file-system.tsGpsPlusSlamJs_AppFramework/src/storage/file-system.ts.mdGpsPlusSlamJs_RecorderApp/index.htmlGpsPlusSlamJs_RecorderApp/src/main.tsGpsPlusSlamJs_RecorderApp/src/ui/settings-modal.test.tsGpsPlusSlamJs_RecorderApp/src/ui/settings-modal.ts
Comment on lines
+395
to
+405
| export async function clearRefPointsCacheForAllScenarios(): Promise<ClearRefPointsCacheResult> { | ||
| const errors: { scenarioName: string; reason: string }[] = []; | ||
| let scenariosCleared = 0; | ||
| let scenariosScanned = 0; | ||
|
|
||
| const scenarios = await listScenarios(); | ||
| for (const scenarioName of scenarios) { | ||
| scenariosScanned++; | ||
| try { | ||
| const handle = await scenariosDir?.getDirectoryHandle(scenarioName); | ||
| if (!handle) continue; |
There was a problem hiding this comment.
Fail fast when storage is unavailable before scanning scenarios.
clearRefPointsCacheForAllScenarios() currently degrades to zero counts if storage isn’t initialized, which can incorrectly report a no-op success path. It should throw (or return a surfaced error) when OPFS root/scenarios dir is unavailable.
Proposed guard
export async function clearRefPointsCacheForAllScenarios(): Promise<ClearRefPointsCacheResult> {
+ if (!storageInitialized) {
+ throw new Error('Storage not initialized. Call initStorage first.');
+ }
+ const scenRoot = await ensureScenariosDir();
+ if (!scenRoot) {
+ throw new Error('Scenarios directory unavailable.');
+ }
+
const errors: { scenarioName: string; reason: string }[] = [];
@@
- const handle = await scenariosDir?.getDirectoryHandle(scenarioName);
+ const handle = await scenRoot.getDirectoryHandle(scenarioName);
if (!handle) continue;🤖 Prompt for 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.
In `@GpsPlusSlamJs_AppFramework/src/storage/file-system.ts` around lines 395 -
405, The function clearRefPointsCacheForAllScenarios currently proceeds when
OPFS root/scenariosDir is unavailable and returns zeroed counts; instead add a
guard at the start of clearRefPointsCacheForAllScenarios that checks
scenariosDir (and any OPFS root dependency) and immediately throw a descriptive
Error (or return a ClearRefPointsCacheResult with an explicit surfaced error) if
it's falsy before calling listScenarios(); update callers/tests accordingly so
they expect the thrown error/explicit failure rather than a silent no-op.
Comment on lines
+359
to
+367
| try { | ||
| await folderManager.loadAndDisplayRefPoints(currentHandle); | ||
| } catch (err) { | ||
| log.warn('Re-import after cache clear failed:', err); | ||
| } | ||
| } else { | ||
| // No active scenario — clear in-memory imported ref points so any | ||
| // proximity checks don't keep referring to stale entries. | ||
| refPointHandlers.setImportedRefPoints([]); |
There was a problem hiding this comment.
Clear in-memory ref points when post-clear re-import fails.
If folderManager.loadAndDisplayRefPoints(currentHandle) fails, stale imported points remain in memory. That can keep old proximity matches even after cache clear.
Proposed fallback
if (currentHandle) {
try {
await folderManager.loadAndDisplayRefPoints(currentHandle);
} catch (err) {
log.warn('Re-import after cache clear failed:', err);
+ refPointHandlers.setImportedRefPoints([]);
}
} else {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try { | |
| await folderManager.loadAndDisplayRefPoints(currentHandle); | |
| } catch (err) { | |
| log.warn('Re-import after cache clear failed:', err); | |
| } | |
| } else { | |
| // No active scenario — clear in-memory imported ref points so any | |
| // proximity checks don't keep referring to stale entries. | |
| refPointHandlers.setImportedRefPoints([]); | |
| try { | |
| await folderManager.loadAndDisplayRefPoints(currentHandle); | |
| } catch (err) { | |
| log.warn('Re-import after cache clear failed:', err); | |
| refPointHandlers.setImportedRefPoints([]); | |
| } | |
| } else { | |
| // No active scenario — clear in-memory imported ref points so any | |
| // proximity checks don't keep referring to stale entries. | |
| refPointHandlers.setImportedRefPoints([]); |
🤖 Prompt for 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.
In `@GpsPlusSlamJs_RecorderApp/src/main.ts` around lines 359 - 367, When
folderManager.loadAndDisplayRefPoints(currentHandle) throws, the catch only logs
the error so stale imported ref points remain; update the catch block to clear
in-memory imported points by calling refPointHandlers.setImportedRefPoints([])
(and keep the existing log.warn with the error) so proximity checks won't use
stale entries after a failed re-import.
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.
Add a "Clear Reference Point Cache" button in settings to remove cached reference points across all scenarios. This feature ensures that stale caches are refreshed by re-importing ref points from the corresponding zip recordings.
Related Issue
Closes #
Type of Change
Bug fix
New feature
Refactoring (no behavior change)
Documentation update
Test improvement
Checklist
Tests added/updated for this change
Sidecar docs (
*.md) updated if behavior changedAll tests pass locally (
pnpm testin both packages)Commit message follows conventional commits format
I have read and agreed to the Contributor License Agreement (a bot will prompt me to sign on this PR if I have not already)
Summary by CodeRabbit
New Features
Tests