[PULSE-51] Visitor ID storage: optional localStorage for cross-tab unique visitors #21

Merged
uz1mani merged 17 commits from staging into main 2026-02-11 15:14:13 +00:00
2 changed files with 90 additions and 12 deletions
Showing only changes of commit 5886cc661c - Show all commits

View File

@@ -160,6 +160,23 @@
}
}
cachedSessionId = generateId();
// * Race fix: re-read before writing; if another tab wrote in the meantime, use that ID instead
var rawAgain = localStorage.getItem(key);
if (rawAgain) {
try {
var parsedAgain = JSON.parse(rawAgain);
if (parsedAgain && typeof parsedAgain.id === 'string') {
var expiredAgain = ttlMs > 0 && typeof parsedAgain.created === 'number' && (Date.now() - parsedAgain.created > ttlMs);
if (!expiredAgain) {
cachedSessionId = parsedAgain.id;
// #region agent log
try { fetch('http://127.0.0.1:7243/ingest/50587964-c1c6-436a-a7ce-ff2cde3c5b63',{method:'POST',headers:{'Content-Type':'application/json'},body:JSON.stringify({location:'script.js:getSessionId:local',message:'race fix reused other tab id',data:{sessionIdPrefix:cachedSessionId.substring(0,8)},timestamp:Date.now(),hypothesisId:'E'})}).catch(function(){}); } catch (e4) {}
// #endregion
return cachedSessionId;
}
}
} catch (e2) {}
}
localStorage.setItem(key, JSON.stringify({ id: cachedSessionId, created: Date.now() }));
// #region agent log
try { fetch('http://127.0.0.1:7243/ingest/50587964-c1c6-436a-a7ce-ff2cde3c5b63',{method:'POST',headers:{'Content-Type':'application/json'},body:JSON.stringify({location:'script.js:getSessionId:local',message:'generated new and wrote to localStorage',data:{sessionIdPrefix:cachedSessionId.substring(0,8)},timestamp:Date.now(),hypothesisId:'C,E'})}).catch(function(){}); } catch (e) {}
greptile-apps[bot] commented 2026-02-11 14:38:11 +00:00 (Migrated from github.com)
Review

Two IDs per tab

In the localStorage path, this generates an ID twice: once at cachedSessionId = generateId() (line ~146) and again at cachedSessionId = generateId() right before localStorage.setItem(...) (line ~176). In the “no existing ID” case, each tab will keep the first generated ID in memory and may write a different ID to localStorage, so the tab’s in-memory session_id can differ from the stored cross-tab ID (leading to duplicate visitor counts until reload).

Consider removing the second generateId() and writing the initially generated cachedSessionId instead.

Prompt To Fix With AI
This is a comment left during a code review.
Path: public/script.js
Line: 146:148

Comment:
**Two IDs per tab**

In the `localStorage` path, this generates an ID twice: once at `cachedSessionId = generateId()` (line ~146) and again at `cachedSessionId = generateId()` right before `localStorage.setItem(...)` (line ~176). In the “no existing ID” case, each tab will keep the *first* generated ID in memory and may write a *different* ID to `localStorage`, so the tab’s in-memory `session_id` can differ from the stored cross-tab ID (leading to duplicate visitor counts until reload).

Consider removing the second `generateId()` and writing the initially generated `cachedSessionId` instead.

How can I resolve this? If you propose a fix, please make it concise.
**Two IDs per tab** In the `localStorage` path, this generates an ID twice: once at `cachedSessionId = generateId()` (line ~146) and again at `cachedSessionId = generateId()` right before `localStorage.setItem(...)` (line ~176). In the “no existing ID” case, each tab will keep the *first* generated ID in memory and may write a *different* ID to `localStorage`, so the tab’s in-memory `session_id` can differ from the stored cross-tab ID (leading to duplicate visitor counts until reload). Consider removing the second `generateId()` and writing the initially generated `cachedSessionId` instead. <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: public/script.js Line: 146:148 Comment: **Two IDs per tab** In the `localStorage` path, this generates an ID twice: once at `cachedSessionId = generateId()` (line ~146) and again at `cachedSessionId = generateId()` right before `localStorage.setItem(...)` (line ~176). In the “no existing ID” case, each tab will keep the *first* generated ID in memory and may write a *different* ID to `localStorage`, so the tab’s in-memory `session_id` can differ from the stored cross-tab ID (leading to duplicate visitor counts until reload). Consider removing the second `generateId()` and writing the initially generated `cachedSessionId` instead. How can I resolve this? If you propose a fix, please make it concise. ````` </details>
uz1mani commented 2026-02-11 14:44:20 +00:00 (Migrated from github.com)
Review

Issue: In the localStorage path the ID was generated twice (once at line 146 and again before localStorage.setItem at ~176), so each tab could keep one ID in memory and write another to storage, causing duplicate visitor counts until reload.
Fix: Removed the second generateId() before localStorage.setItem. The write now uses only the ID from line 146 (cachedSessionId).
Why: Ensures one ID per tab on this path so in-memory and stored IDs stay in sync and we avoid duplicate counts from two different IDs per tab.

Issue: In the localStorage path the ID was generated twice (once at line 146 and again before localStorage.setItem at ~176), so each tab could keep one ID in memory and write another to storage, causing duplicate visitor counts until reload. Fix: Removed the second generateId() before localStorage.setItem. The write now uses only the ID from line 146 (cachedSessionId). Why: Ensures one ID per tab on this path so in-memory and stored IDs stay in sync and we avoid duplicate counts from two different IDs per tab.