Skip to content

fix: new user detection race condition + telemetry gaps - #445

Merged
anandgupta42 merged 5 commits into
mainfrom
fix/new-user-detection
Mar 24, 2026
Merged

fix: new user detection race condition + telemetry gaps#445
anandgupta42 merged 5 commits into
mainfrom
fix/new-user-detection

Conversation

@anandgupta42

@anandgupta42anandgupta42 commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes 3 bugs in the "new user" detection system and 2 telemetry gaps.

UI Fixes:

  • Race condition (home.tsx): isFirstTimeUser was evaluating session.length === 0 before sessions loaded, causing beginner UI to flash on every startup for all users. Now guards on sync.status !== "loading".
  • Non-reactive Tips (tips.tsx): Tip pool was a plain variable, not reactive. Beginner tip selected during the flash persisted for the entire session. Now uses createMemo.

Telemetry Fixes (privacy-safe):

  • first_launch event (welcome.ts): Fires once after install. Contains only version string + is_upgrade boolean. No PII.
  • machine_id fallback (telemetry/index.ts): Anonymous users now get their random UUID as ai.user.id instead of "". This is a privacy improvement — stops grouping all anonymous users as one mega-user.

Documentation:

  • telemetry.md: Added first_launch to event table, "New User Identification" section, "Data Retention" section
  • security-faq.md: Added "How does Altimate Code identify users?" and "What happens on first launch?" Q&As

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Documentation update

Issue for this PR

Closes#444

How did you verify your code works?

  • Upstream marker check passes
  • Typecheck passes (turbo: 5/5 successful)
  • UI changes are reactive Solid.js patterns — createMemo guards on sync.status
  • Telemetry changes are additive (new event type, fallback value) — no existing behavior changed
  • All telemetry respects ALTIMATE_TELEMETRY_DISABLED opt-out

Checklist

  • My code follows the project's coding standards
  • I have performed a self-review of my code
  • I have added altimate_change markers to upstream-shared files
  • I have updated documentation (telemetry.md, security-faq.md)
  • My changes generate no new warnings

Summary by CodeRabbit

  • Documentation

    • Added comprehensive privacy documentation explaining how user identifiers are generated and transmitted for analytics, including first-launch telemetry behavior with confirmed PII exclusion.
    • Updated telemetry documentation with data retention policies and privacy contact information.
    • Enhanced chart library guidance for multi-tab dashboard implementations.
  • Bug Fixes

    • Improved first-time user detection to properly synchronize with session data before determining status.
  • Tests

    • Extended telemetry event validation test coverage.

anandgupta42and others added 2 commits March 23, 2026 22:07
… icon semantics, field validation, pre-delivery checklist
Six improvements derived from session learnings — all general, none task-specific:
- component-guide: lazy chart initialization pattern for multi-tab dashboards
(Chart.js/Recharts/Nivo all render blank in display:none containers)
- component-guide: data-code separation for programmatic HTML generation
(f-string + JS curly braces cause silent parse failures)
- SKILL.md Design Principles: dynamic color safety rule for external/brand colors
- SKILL.md Design Principles: icon semantics check
- SKILL.md Anti-Patterns: warn against filtering on unvalidated data fields
- SKILL.md: pre-delivery checklist (tabs, fields, contrast, icons, tooltips, mobile)
UI Fixes:
- Guard `isFirstTimeUser` on sync status — don't show beginner UI
while sessions are loading (prevents flash on every startup)
- Make Tips component reactive — tip pool now updates when
`isFirstTime` changes (was locked at render time)
Telemetry Fixes (privacy-safe):
- Add `first_launch` event — fires once after install, contains only
version string and is_upgrade boolean. No PII. Opt-out-able.
- Use machine_id as ai.user.id fallback — IMPROVES privacy by
giving each anonymous user a distinct random UUID instead of
grouping all non-logged-in users under empty string ""
Documentation:
- telemetry.md: added `first_launch` to event table, new "New User
Identification" section, "Data Retention" section
- security-faq.md: added "How does Altimate Code identify users?"
and "What happens on first launch?" sections
All telemetry changes respect existing ALTIMATE_TELEMETRY_DISABLED
opt-out. No PII is ever sent — machine_id is crypto.randomUUID(),
email is SHA-256 hashed.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@claudeclaudeBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review.

Tip: disable this comment in your organization's Code Review settings.

@coderabbitai

coderabbitaiBot commented Mar 24, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: d93e4f19-55b3-4be5-8a62-37ca7800fbc1

📥 Commits

Reviewing files that changed from the base of the PR and between 80b5734 and 2e784dd.

📒 Files selected for processing (2)
  • .opencode/skills/data-viz/references/component-guide.md
  • packages/opencode/src/cli/welcome.ts
✅ Files skipped from review due to trivial changes (1)
  • .opencode/skills/data-viz/references/component-guide.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/opencode/src/cli/welcome.ts

📝 Walkthrough

Walkthrough

This PR adds first-launch telemetry tracking, enhances user identification for analytics, and improves first-time user detection in the CLI TUI. It introduces a new first_launch event type, implements machine-ID-based fallback identity for anonymous users, refactors the Tips component with memoization, and updates documentation on telemetry, user identification, and data retention policies.

Changes

Cohort / File(s)Summary
Documentation — Telemetry & Security
docs/docs/reference/security-faq.md, docs/docs/reference/telemetry.md, .opencode/skills/data-viz/references/component-guide.md
Added FAQ sections explaining SHA-256 email hashing for logged-in users and random UUID persistence for anonymous users; documented first_launch event payload (version + is_upgrade flag, no PII); clarified data retention via Azure Application Insights; updated Nivo ResizeObserver behavior notes.
Telemetry Core Implementation
packages/opencode/src/altimate/telemetry/index.ts
Extended Telemetry.Event union with first_launch event type (timestamp, session_id, version, is_upgrade); updated Application Insights user ID tagging to use userEmail || machineId || "" fallback instead of always userEmail.
Telemetry Tests
packages/opencode/test/telemetry/telemetry.test.ts
Added first_launch, skill_created, skill_installed, skill_removed to validated event types (37 total); added test for first_launch tracking; reformatted string literals.
TUI Components & Onboarding
packages/opencode/src/cli/cmd/tui/component/tips.tsx, packages/opencode/src/cli/cmd/tui/routes/home.tsx, packages/opencode/src/cli/welcome.ts
Refactored Tips component to memoize random tip selection via createMemo; delayed first-time user detection until session load completes (return undefined while syncing); added Telemetry.track() call in welcome banner; changed upgrade detection from version comparison to machine-id file existence.

Sequence Diagram(s)

sequenceDiagram
participant CLI as CLI Start
participant Welcome as Welcome Banner
participant Telemetry as Telemetry
participant AppInsights as App Insights
CLI->>Welcome: showWelcomeBannerIfNeeded()
Welcome->>Welcome: Check ~/.altimate/.first_launch_shown
Welcome->>Welcome: Detect isUpgrade via machine-id existence
Welcome->>Telemetry: track({ type: "first_launch", version, is_upgrade })
Telemetry->>Telemetry: Resolve userEmail || machineId || ""
Telemetry->>AppInsights: Send event with ai.user.id fallback
AppInsights-->>Telemetry: Acknowledged
Telemetry-->>Welcome: Complete
alt isUpgrade = false
Welcome-->>CLI: Show welcome banner
else isUpgrade = true
Welcome-->>CLI: Skip (return)
end
Loading
sequenceDiagram
participant TUI as TUI Home Route
participant Sync as Session Sync
participant FirstTime as First-Time Detector
participant Tips as Tips Component
TUI->>Sync: Initialize session data fetch
Sync-->>TUI: sync.status = "loading"
TUI->>FirstTime: isFirstTimeUser() — status loading?
FirstTime-->>TUI: Return undefined (defer)
TUI->>TUI: Skip onboarding hint (when !== true)
Sync-->>TUI: sync.status = "success", sync.data.session = [...]
TUI->>FirstTime: isFirstTimeUser() — check session.length === 0
FirstTime-->>TUI: Return true or false
alt session.length === 0
TUI->>Tips: Show Tips with isFirstTime={true}
Tips->>Tips: createMemo selects from BEGINNER_TIPS pool
Tips-->>TUI: Render beginner tips
else
TUI->>Tips: Show Tips with isFirstTime={false}
Tips->>Tips: createMemo selects from TIPS pool
Tips-->>TUI: Render regular tips
end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • mdesmet

Poem

🐰 A bunny's ode to tracking well:
First-launch events now ring the bell,
Machine IDs hide behind the curtain,
Tips that memoize—now that's certain! 🎉
With privacy squared and UX shined,
This telemetry's one of a kind. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check nameStatusExplanationResolution
Linked Issues check⚠️ WarningThe PR claims to close #444, but the changes focus entirely on new user detection race conditions and telemetry gaps (home.tsx, tips.tsx, welcome.ts, telemetry/index.ts), not on data-viz skill description length, formatting, or system prompt directives that are the actual objectives of #444.Verify whether issue #444 actually requires new user detection and telemetry fixes, or whether this PR incorrectly references #444. The code changes do not address data-viz skill documentation or system prompt improvements mentioned in #444's objectives.
Docstring Coverage⚠️ WarningDocstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
Out of Scope Changes check❓ InconclusiveChanges to component-guide.md (Nivo ResizeObserver clarification) and security-faq.md/telemetry.md documentation appear unrelated to the stated objectives of fixing user detection race conditions or closing #444's data-viz skill issues, suggesting potential out-of-scope modifications.Clarify why Nivo documentation and expanded security/telemetry FAQ sections are included. If these are necessary for the PR context, explain their relevance; otherwise, consider moving them to a separate PR focused on documentation improvements.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly summarizes the main fixes: a race condition in new user detection and telemetry gaps, matching the primary changes in home.tsx, welcome.ts, and telemetry/index.ts.
Description check✅ PassedThe description provides detailed context on UI fixes, telemetry improvements, documentation updates, and verification steps. While it exceeds the template structure (which requests Summary, Test Plan, Checklist), it fully covers all necessary information with clear explanations.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/new-user-detection

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.opencode/skills/data-viz/references/component-guide.md:
- Line 425: Update the inaccurate description for Nivo's Responsive* components:
replace the claim that ResizeObserver "never re-fires on show" with text
describing the actual lifecycle (e.g., "uses ResizeObserver — initially measures
as 0×0 when hidden, then re-measures and re-renders correctly when the container
becomes visible"); reference Nivo's measurement hooks (useMeasure /
useDimensions in `@nivo/core`) to clarify that the ResizeObserver triggers a new
measurement when visibility/size changes so the chart is rendered with correct
dimensions.
In `@packages/opencode/src/cli/welcome.ts`:
- Around line 45-55: The is_upgrade flag is computed incorrectly (isUpgrade =
installedVersion === currentVersion) and cannot distinguish fresh installs from
upgrades; change the logic in the CLI startup path that sets isUpgrade (the
variables installedVersion and currentVersion and the Telemetry.track call) to
detect whether a previous marker/version existed: read the previous version
before you overwrite the marker (or check marker existence), set is_upgrade =
true only when a previousVersion exists AND previousVersion !== currentVersion,
and set is_upgrade = false for first-time installs (no previousVersion present);
then pass that corrected boolean into Telemetry.track.
🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 860aa2ff-e725-4b50-9fb0-119a736d9193

📥 Commits

Reviewing files that changed from the base of the PR and between 12ed190 and e664d78.

📒 Files selected for processing (8)
  • .opencode/skills/data-viz/SKILL.md
  • .opencode/skills/data-viz/references/component-guide.md
  • docs/docs/reference/security-faq.md
  • docs/docs/reference/telemetry.md
  • packages/opencode/src/altimate/telemetry/index.ts
  • packages/opencode/src/cli/cmd/tui/component/tips.tsx
  • packages/opencode/src/cli/cmd/tui/routes/home.tsx
  • packages/opencode/src/cli/welcome.ts

Comment thread.opencode/skills/data-viz/references/component-guide.md Outdated
Comment threadpackages/opencode/src/cli/welcome.ts Outdated
- use `~/.altimate/machine-id` existence for robust `is_upgrade` flag
- fix 3-state logic in `isFirstTimeUser` memo to prevent suppressed beginner UI
- prevent tip re-randomization on prop change in `tips.tsx`
- add missing `first_launch` event to telemetry tests
- remove unused import
@anandgupta42

Copy link
Copy Markdown
ContributorAuthor

Multi-Model Code Review — 6 Models, 1 Round Convergence

Verdict: REQUEST CHANGESALL FIXED
Panel: Claude, GPT 5.2 Codex, Gemini 3.1 Pro, Kimi K2.5, MiniMax M2.5, GLM-5

All findings below were identified during review and fixed in 80b5734.


Major Issues (Fixed)

1. is_upgrade telemetry field was always true for release buildswelcome.ts:45

postinstall.mjs always writes the current version to .installed-version, so installedVersion === currentVersion was always true — making the field useless for distinguishing fresh installs from upgrades.

Fix: Now uses ~/.altimate/machine-id existence as proxy. File only exists after first Telemetry.init(), so fs.existsSync correctly detects upgrades vs fresh installs.

Flagged by: GPT 5.2 Codex, Gemini 3.1 Pro, Kimi K2.5, GLM-5

2. sync.status guard could permanently suppress beginner UIhome.tsx:42-46

Returning false while loading meant that if non-blocking requests (MCP/LSP) stalled at "partial" status, beginner UI would never appear for new users.

Fix: Now returns undefined during loading (3-state). JSX uses isFirstTimeUser() === true to avoid rendering either state prematurely.

Flagged by: GPT 5.2 Codex, Gemini 3.1 Pro

Minor Issues (Fixed)

3. Tips re-randomized when isFirstTime changedtips.tsx:53-56

Math.random() inside createMemo re-evaluated on every dependency change, causing the displayed tip to jump when loading completed.

Fix: Random value now captured once at mount time, reused inside the memo.

Flagged by: Claude, Gemini 3.1 Pro, GLM-5

4. Missing first_launch in telemetry test suitetelemetry.test.ts

Fix: Added "first_launch" to event type enumeration test + dedicated acceptance test.

Flagged by: GPT 5.2 Codex, Gemini 3.1 Pro, Kimi K2.5, MiniMax M2.5

NIT (Fixed)

5. Unused createSignal importtips.tsx:1

Fix: Removed.

Flagged by: Kimi K2.5


Finding Attribution

IssueOriginType
is_upgrade always true for release buildsGPT 5.2, Gemini 3.1, Kimi K2.5, GLM-5Consensus
sync.status guard suppresses beginner UIGPT 5.2, Gemini 3.1Consensus
Tips re-randomize on prop changeClaude, Gemini 3.1, GLM-5Consensus
Missing first_launch test coverageGPT 5.2, Gemini 3.1, Kimi K2.5, MiniMax M2.5Consensus
Unused createSignal importKimi K2.5Unique

Reviewed by 6 models. Convergence: 1 round, unanimous agreement. All findings resolved in 80b5734.

anandgupta42and others added 2 commits March 24, 2026 13:42
- Correct Nivo `Responsive*` behavior: `ResizeObserver` does re-fire
when container becomes visible, not "never re-fires on show"
- Add `altimate_change` marker around `installedVersion` banner line
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@anandgupta42
anandgupta42 merged commit 3022614 into mainMar 24, 2026
14 checks passed
@anandgupta42
anandgupta42 deleted the fix/new-user-detection branch March 25, 2026 02:14
kulvirgit pushed a commit that referenced this pull request Mar 30, 2026
* skill(data-viz): add lazy init, data-code separation, color contrast, icon semantics, field validation, pre-delivery checklist
Six improvements derived from session learnings — all general, none task-specific:
- component-guide: lazy chart initialization pattern for multi-tab dashboards
(Chart.js/Recharts/Nivo all render blank in display:none containers)
- component-guide: data-code separation for programmatic HTML generation
(f-string + JS curly braces cause silent parse failures)
- SKILL.md Design Principles: dynamic color safety rule for external/brand colors
- SKILL.md Design Principles: icon semantics check
- SKILL.md Anti-Patterns: warn against filtering on unvalidated data fields
- SKILL.md: pre-delivery checklist (tabs, fields, contrast, icons, tooltips, mobile)
* fix: new user detection race condition + telemetry gaps
UI Fixes:
- Guard `isFirstTimeUser` on sync status — don't show beginner UI
while sessions are loading (prevents flash on every startup)
- Make Tips component reactive — tip pool now updates when
`isFirstTime` changes (was locked at render time)
Telemetry Fixes (privacy-safe):
- Add `first_launch` event — fires once after install, contains only
version string and is_upgrade boolean. No PII. Opt-out-able.
- Use machine_id as ai.user.id fallback — IMPROVES privacy by
giving each anonymous user a distinct random UUID instead of
grouping all non-logged-in users under empty string ""
Documentation:
- telemetry.md: added `first_launch` to event table, new "New User
Identification" section, "Data Retention" section
- security-faq.md: added "How does Altimate Code identify users?"
and "What happens on first launch?" sections
All telemetry changes respect existing ALTIMATE_TELEMETRY_DISABLED
opt-out. No PII is ever sent — machine_id is crypto.randomUUID(),
email is SHA-256 hashed.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix: address code review feedback on new user detection and telemetry
- use `~/.altimate/machine-id` existence for robust `is_upgrade` flag
- fix 3-state logic in `isFirstTimeUser` memo to prevent suppressed beginner UI
- prevent tip re-randomization on prop change in `tips.tsx`
- add missing `first_launch` event to telemetry tests
- remove unused import
* fix: address CodeRabbit review — Nivo description + marker guard
- Correct Nivo `Responsive*` behavior: `ResizeObserver` does re-fire
when container becomes visible, not "never re-fires on show"
- Add `altimate_change` marker around `installedVersion` banner line
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@anandgupta42