[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
3 changed files with 108 additions and 18 deletions

View File

@@ -4,18 +4,23 @@ All notable changes to Pulse (frontend and product) are documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and Pulse uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html) with a **0.x.y** version scheme while in initial development. The leading `0` indicates that the public API and behaviour may change until we release **1.0.0**. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and Pulse uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html) with a **0.x.y** version scheme while in initial development. The leading `0` indicates that the public API and behaviour may change until we release **1.0.0**.
## [Unreleased] ## [0.2.0-alpha] - 2026-02-11
- No unreleased changes yet; add items here as you work toward the next release.
## [0.1.0] - 2026-02-09
### Added ### Added
- Initial changelog and release process (PULSE-28). - **Smarter unique visitor counts.** If someone opens your site in several tabs or windows, theyre now counted as one visitor by default, so your stats better reflect real people.
- **Control over how visitors are counted.** You can switch back to “one visitor per tab” (more private, no lasting identifier) by adding an option to your script embed. The dashboard shows the right snippet for both options.
- **Optional expiry for the visitor ID.** You can set how long the cross-tab visitor ID is kept (e.g. 24 hours); after that its refreshed automatically.
## [0.1.0-alpha] - 2026-02-09
### Added
- Initial changelog and release process.
- Release documentation in `docs/releasing.md` and optional changelog check script. - Release documentation in `docs/releasing.md` and optional changelog check script.
--- ---
[Unreleased]: https://github.com/ciphera-net/pulse/compare/v0.1.0...HEAD [Unreleased]: https://github.com/ciphera-net/pulse/compare/v0.2.0-alpha...HEAD
[0.1.0]: https://github.com/ciphera-net/pulse/releases/tag/v0.1.0 [0.2.0-alpha]: https://github.com/ciphera-net/pulse/compare/v0.1.0-alpha...v0.2.0-alpha
greptile-apps[bot] commented 2026-02-11 14:38:12 +00:00 (Migrated from github.com)
Review

Missing changelog links

This update removes the reference links at the bottom ([Unreleased], [0.1.0]) but doesn’t add replacements for the new headings ([0.2.0-alpha], [0.1.0-alpha]). As written, ## [0.2.0-alpha] and ## [0.1.0-alpha] will render as dead reference links.

Either keep the headings unlinked (## 0.2.0-alpha) or re-add the reference definitions for the new versions.

Prompt To Fix With AI
This is a comment left during a code review.
Path: CHANGELOG.md
Line: 19:21

Comment:
**Missing changelog links**

This update removes the reference links at the bottom (`[Unreleased]`, `[0.1.0]`) but doesn’t add replacements for the new headings (`[0.2.0-alpha]`, `[0.1.0-alpha]`). As written, `## [0.2.0-alpha]` and `## [0.1.0-alpha]` will render as dead reference links.

Either keep the headings unlinked (`## 0.2.0-alpha`) or re-add the reference definitions for the new versions.

How can I resolve this? If you propose a fix, please make it concise.
**Missing changelog links** This update removes the reference links at the bottom (`[Unreleased]`, `[0.1.0]`) but doesn’t add replacements for the new headings (`[0.2.0-alpha]`, `[0.1.0-alpha]`). As written, `## [0.2.0-alpha]` and `## [0.1.0-alpha]` will render as dead reference links. Either keep the headings unlinked (`## 0.2.0-alpha`) or re-add the reference definitions for the new versions. <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: CHANGELOG.md Line: 19:21 Comment: **Missing changelog links** This update removes the reference links at the bottom (`[Unreleased]`, `[0.1.0]`) but doesn’t add replacements for the new headings (`[0.2.0-alpha]`, `[0.1.0-alpha]`). As written, `## [0.2.0-alpha]` and `## [0.1.0-alpha]` will render as dead reference links. Either keep the headings unlinked (`## 0.2.0-alpha`) or re-add the reference definitions for the new versions. How can I resolve this? If you propose a fix, please make it concise. ````` </details>
uz1mani commented 2026-02-11 14:44:10 +00:00 (Migrated from github.com)
Review

Issue: The version headings [0.2.0-alpha] and [0.1.0-alpha] had no reference definitions at the bottom, so they rendered as dead links.
Fix: Re-added the reference link block: [Unreleased], [0.2.0-alpha], and [0.1.0-alpha] with the correct GitHub compare/release URLs.
Why: Keeps the changelog consistent with Keep a Changelog and makes the version headings clickable to the right diffs and release.

Issue: The version headings [0.2.0-alpha] and [0.1.0-alpha] had no reference definitions at the bottom, so they rendered as dead links. Fix: Re-added the reference link block: [Unreleased], [0.2.0-alpha], and [0.1.0-alpha] with the correct GitHub compare/release URLs. Why: Keeps the changelog consistent with Keep a Changelog and makes the version headings clickable to the right diffs and release.
[0.1.0-alpha]: https://github.com/ciphera-net/pulse/releases/tag/v0.1.0-alpha

View File

@@ -89,6 +89,9 @@ export default function ScriptSetupBlock({
<code className="text-xs text-neutral-900 dark:text-white break-all font-mono block pr-10"> <code className="text-xs text-neutral-900 dark:text-white break-all font-mono block pr-10">
{`<script defer data-domain="${site.domain}" data-api="${API_URL}" src="${APP_URL}/script.js"></script>`} {`<script defer data-domain="${site.domain}" data-api="${API_URL}" src="${APP_URL}/script.js"></script>`}
</code> </code>
<p className="mt-2 text-xs text-neutral-500 dark:text-neutral-400">
Default: cross-tab (localStorage). Optional: <code className="rounded px-1 bg-neutral-200 dark:bg-neutral-700">data-storage=&quot;session&quot;</code> to opt out (per-tab, ephemeral). Optional: <code className="rounded px-1 bg-neutral-200 dark:bg-neutral-700">data-storage-ttl=&quot;48&quot;</code> to set expiry in hours (default: 24).
</p>
greptile-apps[bot] commented 2026-02-11 13:04:49 +00:00 (Migrated from github.com)
Review

Documentation says data-storage="session" is "Optional" but according to the implementation, this should be the default. If localStorage is now the default, this text should clarify that data-storage="session" opts OUT of cross-tab tracking.

Prompt To Fix With AI
This is a comment left during a code review.
Path: components/sites/ScriptSetupBlock.tsx
Line: 92:94

Comment:
Documentation says `data-storage="session"` is "Optional" but according to the implementation, this should be the default. If localStorage is now the default, this text should clarify that `data-storage="session"` opts OUT of cross-tab tracking.

How can I resolve this? If you propose a fix, please make it concise.
Documentation says `data-storage="session"` is "Optional" but according to the implementation, this should be the default. If localStorage is now the default, this text should clarify that `data-storage="session"` opts OUT of cross-tab tracking. <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: components/sites/ScriptSetupBlock.tsx Line: 92:94 Comment: Documentation says `data-storage="session"` is "Optional" but according to the implementation, this should be the default. If localStorage is now the default, this text should clarify that `data-storage="session"` opts OUT of cross-tab tracking. How can I resolve this? If you propose a fix, please make it concise. ````` </details>
uz1mani commented 2026-02-11 13:13:00 +00:00 (Migrated from github.com)
Review

Issue: Copy said “Optional: data-storage="session"” without stating that the default is cross-tab and that session opts out.
Fix: Copy updated to: “Default: cross-tab (localStorage). Optional: data-storage="session" to opt out (per-tab, ephemeral).”
Why: Makes it explicit that the default is cross-tab and that data-storage="session" opts out of it.

Issue: Copy said “Optional: data-storage=\"session\"” without stating that the default is cross-tab and that session opts out. Fix: Copy updated to: “Default: cross-tab (localStorage). Optional: data-storage=\"session\" to opt out (per-tab, ephemeral).” Why: Makes it explicit that the default is cross-tab and that data-storage="session" opts out of it.
greptile-apps[bot] commented 2026-02-11 13:35:14 +00:00 (Migrated from github.com)
Review

Missing documentation for data-storage-ttl attribute. The script supports an optional TTL parameter (default 24 hours) but this isn't mentioned here. Consider adding: "Optional: data-storage-ttl=\"48\" to set expiry in hours (default: 24)."

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: components/sites/ScriptSetupBlock.tsx
Line: 92:94

Comment:
Missing documentation for `data-storage-ttl` attribute. The script supports an optional TTL parameter (default 24 hours) but this isn't mentioned here. Consider adding: "Optional: `data-storage-ttl=\"48\"` to set expiry in hours (default: 24)."

<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.
Missing documentation for `data-storage-ttl` attribute. The script supports an optional TTL parameter (default 24 hours) but this isn't mentioned here. Consider adding: "Optional: `data-storage-ttl=\"48\"` to set expiry in hours (default: 24)." <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: components/sites/ScriptSetupBlock.tsx Line: 92:94 Comment: Missing documentation for `data-storage-ttl` attribute. The script supports an optional TTL parameter (default 24 hours) but this isn't mentioned here. Consider adding: "Optional: `data-storage-ttl=\"48\"` to set expiry in hours (default: 24)." <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:34:22 +00:00 (Migrated from github.com)
Review

Fix: Already added on line 93: “Optional: data-storage-ttl="48" to set expiry in hours (default: 24).”
Why: So users know they can change the cross-tab visitor ID expiry.
If the comment was from a branch that doesn’t have this yet, merge or cherry-pick the commit that added it, or re-apply that sentence in the same paragraph.

Fix: Already added on line 93: “Optional: data-storage-ttl=\"48\" to set expiry in hours (default: 24).” Why: So users know they can change the cross-tab visitor ID expiry. If the comment was from a branch that doesn’t have this yet, merge or cherry-pick the commit that added it, or re-apply that sentence in the same paragraph.
<button <button
type="button" type="button"
onClick={copyScript} onClick={copyScript}

View File

@@ -1,6 +1,8 @@
/** /**
greptile-apps[bot] commented 2026-02-11 13:04:45 +00:00 (Migrated from github.com)
Review

Default behavior changed from sessionStorage to localStorage, contradicting PR description which states "keep sessionStorage as default". Comments say "Default: cross-tab visitor ID (localStorage)" but PR says "Default remains sessionStorage".

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

Comment:
Default behavior changed from sessionStorage to localStorage, contradicting PR description which states "keep sessionStorage as default". Comments say "Default: cross-tab visitor ID (localStorage)" but PR says "Default remains sessionStorage".

How can I resolve this? If you propose a fix, please make it concise.
Default behavior changed from sessionStorage to localStorage, contradicting PR description which states "keep sessionStorage as default". Comments say "Default: cross-tab visitor ID (localStorage)" but PR says "Default remains sessionStorage". <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: public/script.js Line: 4:5 Comment: Default behavior changed from sessionStorage to localStorage, contradicting PR description which states "keep sessionStorage as default". Comments say "Default: cross-tab visitor ID (localStorage)" but PR says "Default remains sessionStorage". How can I resolve this? If you propose a fix, please make it concise. ````` </details>
uz1mani commented 2026-02-11 13:12:32 +00:00 (Migrated from github.com)
Review

Issue: Default was documented as sessionStorage while the PR said “keep sessionStorage as default.”
Fix: Default is cross-tab (localStorage). File header now says: “Default: cross-tab visitor ID (localStorage), optional data-storage-ttl in hours. Optional: data-storage="session" for per-tab (ephemeral) counting.”
Why: Product decision is to use cross-tab as the default; docs and code are aligned with that.

Issue: Default was documented as sessionStorage while the PR said “keep sessionStorage as default.” Fix: Default is cross-tab (localStorage). File header now says: “Default: cross-tab visitor ID (localStorage), optional data-storage-ttl in hours. Optional: data-storage=\"session\" for per-tab (ephemeral) counting.” Why: Product decision is to use cross-tab as the default; docs and code are aligned with that.
* Pulse - Privacy-First Tracking Script * Pulse - Privacy-First Tracking Script
* Lightweight, no cookies, GDPR compliant * Lightweight, no cookies, GDPR compliant.
* Default: cross-tab visitor ID (localStorage), optional data-storage-ttl in hours.
* Optional: data-storage="session" for per-tab (ephemeral) counting.
*/ */
(function() { (function() {
@@ -19,6 +21,11 @@
greptile-apps[bot] commented 2026-02-11 13:27:08 +00:00 (Migrated from github.com)
Review

parseFloat with two arguments (second argument is radix) is incorrect - parseFloat only accepts one argument. The radix parameter is for parseInt, not parseFloat.

  const ttlHours = storageMode === 'local' ? parseFloat(script.getAttribute('data-storage-ttl') || '24') : 0;
Prompt To Fix With AI
This is a comment left during a code review.
Path: public/script.js
Line: 27:27

Comment:
`parseFloat` with two arguments (second argument is radix) is incorrect - `parseFloat` only accepts one argument. The radix parameter is for `parseInt`, not `parseFloat`.

```suggestion
  const ttlHours = storageMode === 'local' ? parseFloat(script.getAttribute('data-storage-ttl') || '24') : 0;
```

How can I resolve this? If you propose a fix, please make it concise.
`parseFloat` with two arguments (second argument is radix) is incorrect - `parseFloat` only accepts one argument. The radix parameter is for `parseInt`, not `parseFloat`. ```suggestion const ttlHours = storageMode === 'local' ? parseFloat(script.getAttribute('data-storage-ttl') || '24') : 0; ``` <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: public/script.js Line: 27:27 Comment: `parseFloat` with two arguments (second argument is radix) is incorrect - `parseFloat` only accepts one argument. The radix parameter is for `parseInt`, not `parseFloat`. ```suggestion const ttlHours = storageMode === 'local' ? parseFloat(script.getAttribute('data-storage-ttl') || '24') : 0; ``` How can I resolve this? If you propose a fix, please make it concise. ````` </details>
uz1mani commented 2026-02-11 13:29:33 +00:00 (Migrated from github.com)
Review

Issue: parseFloat was called with two arguments; only parseInt accepts a radix.
Fix: Removed the second argument so the call is parseFloat(script.getAttribute('data-storage-ttl') || '24').
Why: Correct use of the API.

Issue: parseFloat was called with two arguments; only parseInt accepts a radix. Fix: Removed the second argument so the call is parseFloat(script.getAttribute('data-storage-ttl') || '24'). Why: Correct use of the API.
const domain = script.getAttribute('data-domain'); const domain = script.getAttribute('data-domain');
const apiUrl = script.getAttribute('data-api') || 'https://pulse-api.ciphera.net'; const apiUrl = script.getAttribute('data-api') || 'https://pulse-api.ciphera.net';
// * Visitor ID storage: "local" (default, cross-tab) or "session" (ephemeral per-tab)
const storageMode = (script.getAttribute('data-storage') || 'local').toLowerCase() === 'session' ? 'session' : 'local';
// * 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') : 0;
const ttlMs = ttlHours > 0 ? ttlHours * 60 * 60 * 1000 : 0;
// * Performance Monitoring (Core Web Vitals) State // * Performance Monitoring (Core Web Vitals) State
let currentEventId = null; let currentEventId = null;
@@ -102,25 +109,100 @@
} }
greptile-apps[bot] commented 2026-02-11 13:04:47 +00:00 (Migrated from github.com)
Review

Comment states "Default: ephemeral (sessionStorage, per-tab)" but the actual default in line 25 is 'local' (localStorage). This is a contradiction - either the comment or the code is incorrect.

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

Comment:
Comment states "Default: ephemeral (sessionStorage, per-tab)" but the actual default in line 25 is `'local'` (localStorage). This is a contradiction - either the comment or the code is incorrect.

How can I resolve this? If you propose a fix, please make it concise.
Comment states "Default: ephemeral (sessionStorage, per-tab)" but the actual default in line 25 is `'local'` (localStorage). This is a contradiction - either the comment or the code is incorrect. <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: public/script.js Line: 119:120 Comment: Comment states "Default: ephemeral (sessionStorage, per-tab)" but the actual default in line 25 is `'local'` (localStorage). This is a contradiction - either the comment or the code is incorrect. How can I resolve this? If you propose a fix, please make it concise. ````` </details>
uz1mani commented 2026-02-11 13:12:41 +00:00 (Migrated from github.com)
Review

Issue: Comment said “Default: ephemeral (sessionStorage, per-tab)” but the code default was 'local'.
Fix: Comment updated to: “Default: persistent (localStorage, cross-tab), optional TTL in hours. With data-storage="session": ephemeral (sessionStorage, per-tab).” Default in code remains 'local'.
Why: Comment and code now both describe the cross-tab default.

Issue: Comment said “Default: ephemeral (sessionStorage, per-tab)” but the code default was 'local'. Fix: Comment updated to: “Default: persistent (localStorage, cross-tab), optional TTL in hours. With data-storage=\"session\": ephemeral (sessionStorage, per-tab).” Default in code remains 'local'. Why: Comment and code now both describe the cross-tab default.
greptile-apps[bot] commented 2026-02-11 13:40:18 +00:00 (Migrated from github.com)
Review

regenerating ID after all re-reads means two tabs opening simultaneously will create different IDs even when they shouldn't - the second generateId() call creates a new ID rather than reusing the one from line 146, causing both tabs to write their own unique IDs

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

Comment:
regenerating ID after all re-reads means two tabs opening simultaneously will create different IDs even when they shouldn't - the second `generateId()` call creates a new ID rather than reusing the one from line 146, causing both tabs to write their own unique IDs

How can I resolve this? If you propose a fix, please make it concise.
regenerating ID after all re-reads means two tabs opening simultaneously will create different IDs even when they shouldn't - the second `generateId()` call creates a new ID rather than reusing the one from line 146, causing both tabs to write their own unique IDs <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: public/script.js Line: 176:176 Comment: regenerating ID after all re-reads means two tabs opening simultaneously will create different IDs even when they shouldn't - the second `generateId()` call creates a new ID rather than reusing the one from line 146, causing both tabs to write their own unique IDs How can I resolve this? If you propose a fix, please make it concise. ````` </details>
uz1mani commented 2026-02-11 14:31:41 +00:00 (Migrated from github.com)
Review

Issue: Regenerating the ID before the write meant two tabs could each get a new ID and both write, so we could end up with two different IDs and duplicate counts.
Fix: Removed the second generateId(); we now use the ID from line 146 when writing.
Why: Avoids introducing a second ID in the write path so we don’t make the simultaneous-open case worse.

Issue: Regenerating the ID before the write meant two tabs could each get a new ID and both write, so we could end up with two different IDs and duplicate counts. Fix: Removed the second generateId(); we now use the ID from line 146 when writing. Why: Avoids introducing a second ID in the write path so we don’t make the simultaneous-open case worse.
}); });
// * Memory cache for session ID (fallback if sessionStorage is unavailable) // * Memory cache for session ID (fallback if storage is unavailable)
let cachedSessionId = null; let cachedSessionId = null;
// * Generate ephemeral session ID (not persistent) function generateId() {
return Math.random().toString(36).substring(2, 15) + Math.random().toString(36).substring(2, 15);
}
// * Returns session/visitor ID. Default: persistent (localStorage, cross-tab), optional TTL in hours.
// * With data-storage="session": ephemeral (sessionStorage, per-tab).
function getSessionId() { function getSessionId() {
if (cachedSessionId) { if (cachedSessionId) {
return cachedSessionId; return cachedSessionId;
} }
// * Use a static key for session storage to ensure consistency across pages
const key = 'ciphera_session_id'; const key = 'ciphera_session_id';
// * Legacy key support for migration (strip whitespace just in case)
const legacyKey = 'plausible_session_' + (domain ? domain.trim() : ''); const legacyKey = 'plausible_session_' + (domain ? domain.trim() : '');
if (storageMode === 'local') {
try { try {
// * Try to get existing session ID const raw = localStorage.getItem(key);
cachedSessionId = sessionStorage.getItem(key); if (raw) {
try {
const parsed = JSON.parse(raw);
if (parsed && typeof parsed.id === 'string') {
const hasValidCreated = typeof parsed.created === 'number';
const expired = ttlMs > 0 && (!hasValidCreated || (Date.now() - parsed.created > ttlMs));
if (!expired) {
cachedSessionId = parsed.id;
return cachedSessionId;
}
}
} catch (e) {
// * Invalid JSON: migrate legacy plain-string ID to { id, created } format
if (typeof raw === 'string' && raw.trim().length > 0) {
cachedSessionId = raw.trim();
try {
localStorage.setItem(key, JSON.stringify({ id: cachedSessionId, created: Date.now() }));
} catch (e2) {}
return cachedSessionId;
}
}
}
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 {
greptile-apps[bot] commented 2026-02-11 14:48:36 +00:00 (Migrated from github.com)
Review

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

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.
var parsedAgain = JSON.parse(rawAgain);
if (parsedAgain && typeof parsedAgain.id === 'string') {
var hasValidCreatedAgain = typeof parsedAgain.created === 'number';
var expiredAgain = ttlMs > 0 && (!hasValidCreatedAgain || (Date.now() - parsedAgain.created > ttlMs));
if (!expiredAgain) {
cachedSessionId = parsedAgain.id;
return cachedSessionId;
}
}
} catch (e2) {
if (typeof rawAgain === 'string' && rawAgain.trim().length > 0) {
cachedSessionId = rawAgain.trim();
return cachedSessionId;
}
}
}
// * Final re-read immediately before write to avoid overwriting a fresher ID from another tab
var rawBeforeWrite = localStorage.getItem(key);
if (rawBeforeWrite) {
try {
var parsedBefore = JSON.parse(rawBeforeWrite);
if (parsedBefore && typeof parsedBefore.id === 'string') {
var hasValidCreatedBefore = typeof parsedBefore.created === 'number';
var expBefore = ttlMs > 0 && (!hasValidCreatedBefore || (Date.now() - parsedBefore.created > ttlMs));
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.
if (!expBefore) {
cachedSessionId = parsedBefore.id;
return cachedSessionId;
}
}
} catch (e3) {
if (typeof rawBeforeWrite === 'string' && rawBeforeWrite.trim().length > 0) {
cachedSessionId = rawBeforeWrite.trim();
return cachedSessionId;
}
}
}
// * Best-effort only: another tab could write between here and setItem; without locks perfect sync is not achievable
localStorage.setItem(key, JSON.stringify({ id: cachedSessionId, created: Date.now() }));
} catch (e) {
cachedSessionId = generateId();
}
return cachedSessionId;
greptile-apps[bot] commented 2026-02-11 13:04:48 +00:00 (Migrated from github.com)
Review

Race condition fix has logic gap: if the ID in rawAgain is expired, code falls through to localStorage.setItem on line 161 using cachedSessionId from line 146, but doesn't check if yet another tab wrote a fresh ID between lines 148-161. Consider re-reading one more time right before the write.

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

Comment:
Race condition fix has logic gap: if the ID in `rawAgain` is expired, code falls through to `localStorage.setItem` on line 161 using `cachedSessionId` from line 146, but doesn't check if yet another tab wrote a fresh ID between lines 148-161. Consider re-reading one more time right before the write.

How can I resolve this? If you propose a fix, please make it concise.
Race condition fix has logic gap: if the ID in `rawAgain` is expired, code falls through to `localStorage.setItem` on line 161 using `cachedSessionId` from line 146, but doesn't check if yet another tab wrote a fresh ID between lines 148-161. Consider re-reading one more time right before the write. <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: public/script.js Line: 148:160 Comment: Race condition fix has logic gap: if the ID in `rawAgain` is expired, code falls through to `localStorage.setItem` on line 161 using `cachedSessionId` from line 146, but doesn't check if yet another tab wrote a fresh ID between lines 148-161. Consider re-reading one more time right before the write. How can I resolve this? If you propose a fix, please make it concise. ````` </details>
uz1mani commented 2026-02-11 13:12:49 +00:00 (Migrated from github.com)
Review

Issue: Race: we could overwrite a newer ID from another tab between the re-read and the write.
Fix: Added a final re-read of localStorage.getItem(key) immediately before localStorage.setItem. If that read returns a valid, non-expired ID, we use it and return without writing.
Why: Avoids overwriting a fresher ID written by another tab.

Issue: Race: we could overwrite a newer ID from another tab between the re-read and the write. Fix: Added a final re-read of localStorage.getItem(key) immediately before localStorage.setItem. If that read returns a valid, non-expired ID, we use it and return without writing. Why: Avoids overwriting a fresher ID written by another tab.
}
// * If not found in new key, try legacy key and migrate // * data-storage="session": session storage (ephemeral, per-tab)
try {
cachedSessionId = sessionStorage.getItem(key);
if (!cachedSessionId && legacyKey) { if (!cachedSessionId && legacyKey) {
cachedSessionId = sessionStorage.getItem(legacyKey); cachedSessionId = sessionStorage.getItem(legacyKey);
if (cachedSessionId) { if (cachedSessionId) {
@@ -133,7 +215,7 @@
} }
if (!cachedSessionId) { if (!cachedSessionId) {
cachedSessionId = Math.random().toString(36).substring(2, 15) + Math.random().toString(36).substring(2, 15); cachedSessionId = generateId();
try { try {
sessionStorage.setItem(key, cachedSessionId); sessionStorage.setItem(key, cachedSessionId);
} catch (e) { } catch (e) {