ciphera-net/pulse
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 15 Wiki Activity

[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
Conversation 28 Commits 17 Files Changed 3 +60 -10
2 changed files with 60 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit f423d01d7b - Show all commits

28
public/script.js
View File

@@ -26,11 +26,6 @@
// * When storage is "local", optional TTL in hours; after TTL the ID is regenerated (e.g. 24 = one day)
const ttlHours = storageMode === 'local' ? parseFloat(script.getAttribute('data-storage-ttl') || '24', 10) : 0;
const ttlMs = ttlHours > 0 ? ttlHours * 60 * 60 * 1000 : 0;
// #region agent log
try {
fetch(apiUrl + '/api/v1/debug-log',{method:'POST',headers:{'Content-Type':'application/json'},body:JSON.stringify({location:'script.js:init',message:'storage config',data:{storageMode,dataStorageAttr:script.getAttribute('data-storage'),ttlHours,ttlMs,domain},timestamp:Date.now(),hypothesisId:'A,B'})}).catch(function(){});
} catch (e) {}
// #endregion
// * Performance Monitoring (Core Web Vitals) State
let currentEventId = null;
@@ -125,9 +120,6 @@
// * With data-storage="local": persistent (localStorage, cross-tab), optional TTL in hours.
function getSessionId() {
if (cachedSessionId) {
// #region agent log
try { fetch(apiUrl + '/api/v1/debug-log',{method:'POST',headers:{'Content-Type':'application/json'},body:JSON.stringify({location:'script.js:getSessionId',message:'return cached',data:{sessionIdPrefix:cachedSessionId.substring(0,8),storageMode},timestamp:Date.now(),hypothesisId:'E'})}).catch(function(){}); } catch (e) {}
// #endregion
return cachedSessionId;
}
@@ -137,11 +129,6 @@
if (storageMode === 'local') {
try {
const raw = localStorage.getItem(key);
// #region agent log
var rawType = raw === null ? 'null' : typeof raw;
var rawPrefix = raw && raw.substring ? raw.substring(0, 60) : '';
try { fetch(apiUrl + '/api/v1/debug-log',{method:'POST',headers:{'Content-Type':'application/json'},body:JSON.stringify({location:'script.js:getSessionId:local',message:'localStorage read',data:{rawType,rawPrefix,hasRaw:!!raw},timestamp:Date.now(),hypothesisId:'C,D,E'})}).catch(function(){}); } catch (e) {}
// #endregion
if (raw) {
try {
const parsed = JSON.parse(raw);
@@ -149,9 +136,6 @@
const expired = ttlMs > 0 && typeof parsed.created === 'number' && (Date.now() - parsed.created > ttlMs);
if (!expired) {
cachedSessionId = parsed.id;
// #region agent log
try { fetch(apiUrl + '/api/v1/debug-log',{method:'POST',headers:{'Content-Type':'application/json'},body:JSON.stringify({location:'script.js:getSessionId:local',message:'reused from localStorage',data:{sessionIdPrefix:cachedSessionId.substring(0,8)},timestamp:Date.now(),hypothesisId:'D,E'})}).catch(function(){}); } catch (e) {}
// #endregion
return cachedSessionId;
}
}
@@ -169,31 +153,19 @@
var expiredAgain = ttlMs > 0 && typeof parsedAgain.created === 'number' && (Date.now() - parsedAgain.created > ttlMs);
if (!expiredAgain) {
cachedSessionId = parsedAgain.id;
// #region agent log
try { fetch(apiUrl + '/api/v1/debug-log',{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;
}
}
greptile-apps[bot] commented 2026-02-11 14:48:36 +00:00 (Migrated from github.com)
Review
Copy Link

Legacy value causes churn
In the localStorage mode, localStorage.getItem(key) is always JSON.parse’d; if parsing fails (including the “old string format” case mentioned in the comment), the code regenerates a new ID instead of reusing the existing string value. If any existing installs already have a plain-string ID under ciphera_session_id in localStorage, this will force an unexpected visitor-id reset on upgrade. A small migration path (e.g., if raw is a non-empty string and not JSON, treat it as the ID and wrap it into the JSON format with created) would avoid that churn.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

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

Comment:
**Legacy value causes churn**
In the `localStorage` mode, `localStorage.getItem(key)` is always `JSON.parse`’d; if parsing fails (including the “old string format” case mentioned in the comment), the code regenerates a new ID instead of reusing the existing string value. If any existing installs already have a plain-string ID under `ciphera_session_id` in `localStorage`, this will force an unexpected visitor-id reset on upgrade. A small migration path (e.g., if `raw` is a non-empty string and not JSON, treat it as the ID and wrap it into the JSON format with `created`) would avoid that churn.

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.
**Legacy value causes churn** In the `localStorage` mode, `localStorage.getItem(key)` is always `JSON.parse`’d; if parsing fails (including the “old string format” case mentioned in the comment), the code regenerates a new ID instead of reusing the existing string value. If any existing installs already have a plain-string ID under `ciphera_session_id` in `localStorage`, this will force an unexpected visitor-id reset on upgrade. A small migration path (e.g., if `raw` is a non-empty string and not JSON, treat it as the ID and wrap it into the JSON format with `created`) would avoid that churn. <sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub> <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: public/script.js Line: 129:132 Comment: **Legacy value causes churn** In the `localStorage` mode, `localStorage.getItem(key)` is always `JSON.parse`’d; if parsing fails (including the “old string format” case mentioned in the comment), the code regenerates a new ID instead of reusing the existing string value. If any existing installs already have a plain-string ID under `ciphera_session_id` in `localStorage`, this will force an unexpected visitor-id reset on upgrade. A small migration path (e.g., if `raw` is a non-empty string and not JSON, treat it as the ID and wrap it into the JSON format with `created`) would avoid that churn. <sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub> How can I resolve this? If you propose a fix, please make it concise. ````` </details>
uz1mani commented 2026-02-11 14:52:23 +00:00 (Migrated from github.com)
Review
Copy Link

Issue: In localStorage mode, a plain-string value in ciphera_session_id (e.g. from an older script) was treated as invalid and caused a new ID to be generated on upgrade.
Fix: If JSON.parse(raw) throws and raw is a non-empty string, we treat it as a legacy ID: set cachedSessionId = raw.trim(), write { id: cachedSessionId, created: Date.now() } to localStorage (first read only), and return. The same legacy handling is used in the two re-read paths (use the string as ID and return; no write there to avoid extra writes).
Why: Existing plain-string IDs are kept across upgrades and migrated to the new JSON shape in one step.

Issue: In localStorage mode, a plain-string value in ciphera_session_id (e.g. from an older script) was treated as invalid and caused a new ID to be generated on upgrade. Fix: If JSON.parse(raw) throws and raw is a non-empty string, we treat it as a legacy ID: set cachedSessionId = raw.trim(), write { id: cachedSessionId, created: Date.now() } to localStorage (first read only), and return. The same legacy handling is used in the two re-read paths (use the string as ID and return; no write there to avoid extra writes). Why: Existing plain-string IDs are kept across upgrades and migrated to the new JSON shape in one step.
} catch (e2) {}
}
localStorage.setItem(key, JSON.stringify({ id: cachedSessionId, created: Date.now() }));
// #region agent log
try { fetch(apiUrl + '/api/v1/debug-log',{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) {}
// #endregion
} catch (e) {
cachedSessionId = generateId();
// #region agent log
try { fetch(apiUrl + '/api/v1/debug-log',{method:'POST',headers:{'Content-Type':'application/json'},body:JSON.stringify({location:'script.js:getSessionId:local',message:'localStorage error fallback',data:{sessionIdPrefix:cachedSessionId.substring(0,8),err:String(e&&e.message)},timestamp:Date.now(),hypothesisId:'C'})}).catch(function(){}); } catch (e2) {}
// #endregion
}
return cachedSessionId;
}
// * data-storage="session": session storage (ephemeral, per-tab)
// #region agent log
try { fetch(apiUrl + '/api/v1/debug-log',{method:'POST',headers:{'Content-Type':'application/json'},body:JSON.stringify({location:'script.js:getSessionId',message:'using session branch',data:{storageMode},timestamp:Date.now(),hypothesisId:'A'})}).catch(function(){}); } catch (e) {}
// #endregion
try {
cachedSessionId = sessionStorage.getItem(key);
if (!cachedSessionId && legacyKey) {