Skip to content

feat(desktop): track full onboarding chat messages in PostHog#6011

Merged
kodjima33 merged 1 commit into
mainfrom
worktree-fix-auth-activate-app
Mar 24, 2026
Merged

feat(desktop): track full onboarding chat messages in PostHog#6011
kodjima33 merged 1 commit into
mainfrom
worktree-fix-auth-activate-app

Conversation

@kodjima33
Copy link
Copy Markdown
Collaborator

Summary

Adds onboarding_chat_message_detailed PostHog events that capture:

  • User messages: full text sent during onboarding chat
  • AI responses: full text, tool calls used, model
  • Errors: raw error string when chat fails

Query in PostHog: filter by onboarding_chat_message_detailed and group by distinct_id to reconstruct any user's full onboarding conversation.

Test plan

  • Go through onboarding, check PostHog for onboarding_chat_message_detailed events
  • Verify user messages have role=user and AI responses have role=assistant
  • Verify tool calls appear in the tool_calls property

🤖 Generated with Claude Code

Adds onboarding_chat_message_detailed events with full message text,
tool calls, model used, and errors for debugging onboarding issues.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@kodjima33 kodjima33 merged commit 1c6548b into main Mar 24, 2026
3 checks passed
@kodjima33 kodjima33 deleted the worktree-fix-auth-activate-app branch March 24, 2026 21:35
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 24, 2026

Greptile Summary

This PR adds onboarding_chat_message_detailed PostHog events that capture the full text of every onboarding conversation turn (user messages, AI responses, and errors) for debugging and conversation reconstruction. The analytics helper is well-structured and correctly routes only to PostHog to avoid Mixpanel size limits.

Key issues found:

  • AI response text is always empty for streaming responses: The analytics call at line 2111 looks up the AI message by its original local UUID (aiMessageId), but by that point the saveMessage backend call has already replaced that UUID with the server-assigned ID. The lookup returns nil, falls back to queryResult.text, which is empty in streaming scenarios. The fix is to use the already-captured messageText variable directly.
  • Error event text field has confusing semantics: The role: "error" event sets text: trimmedText (the user's original query), not the error string — the opposite of what the PR description implies.
  • error property has no truncation cap in onboardingChatMessageDetailed, unlike how rawError is handled in the sibling chatAgentError method (capped at 500 chars).

Confidence Score: 2/5

  • Not safe to merge as-is — the AI response analytics event will silently capture empty text in the common streaming path due to a stale ID lookup.
  • The stale aiMessageId lookup is a correctness bug that defeats the primary purpose of the PR (capturing full AI responses). In the normal streaming flow, queryResult.text is empty and the message's ID has already been replaced, meaning PostHog events for AI turns will always have text: "". This doesn't break the app or cause data loss, but it means the feature is effectively non-functional for AI responses.
  • desktop/Desktop/Sources/Providers/ChatProvider.swift — the AI response analytics call at line 2111 needs to use messageText instead of re-looking up the message by its now-stale UUID.

Important Files Changed

Filename Overview
desktop/Desktop/Sources/AnalyticsManager.swift New onboardingChatMessageDetailed method correctly routes only to PostHog and truncates text to 2000 chars; minor: error property has no truncation unlike adjacent analytics methods.
desktop/Desktop/Sources/Providers/ChatProvider.swift User-message tracking is correct, but the AI-response tracking lookup uses a stale local UUID (replaced by server ID before the lookup), causing the text field to fall back to queryResult.text which is empty for streaming responses. The error event also places user query text in the text field instead of the error string.

Sequence Diagram

sequenceDiagram
    participant User
    participant ChatProvider
    participant ACPBridge
    participant Backend as APIClient
    participant PostHog

    User->>ChatProvider: sendMessage(text)
    ChatProvider->>PostHog: onboarding_chat_message_detailed(role=user, text=trimmedText)
    ChatProvider->>ACPBridge: query(prompt, ...) [streaming]
    ACPBridge-->>ChatProvider: textDelta callbacks (populate messages[aiMessageId].text)
    ACPBridge-->>ChatProvider: queryResult (queryResult.text may be empty for streaming)
    ChatProvider->>Backend: saveMessage(aiText)
    Backend-->>ChatProvider: response.id
    ChatProvider->>ChatProvider: messages[aiMessageId].id = response.id ⚠️ ID replaced
    Note over ChatProvider: aiMessageId is now stale
    ChatProvider->>PostHog: onboarding_chat_message_detailed(role=assistant, text=??)<br/>lookup by aiMessageId → nil → fallback to queryResult.text (empty)
    alt Error path
        ACPBridge-->>ChatProvider: throw error
        ChatProvider->>PostHog: onboarding_chat_message_detailed(role=error, text=trimmedText, error=rawError)
    end
Loading

Reviews (1): Last reviewed commit: "feat(desktop): track full onboarding cha..." | Re-trigger Greptile


// Track onboarding AI responses with full content and tool calls
if isOnboarding {
let aiText = messages.first(where: { $0.id == aiMessageId })?.text ?? queryResult.text
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Stale aiMessageId lookup returns nil, falling back to empty text

By the time execution reaches line 2111, the saveMessage call (lines 2082–2094) has already awaited and, on success, replaced the in-memory message's id from aiMessageId (a local UUID) to the server-assigned response.id:

if let syncIndex = messages.firstIndex(where: { $0.id == aiMessageId }) {
    messages[syncIndex].id = response.id   // ← ID is no longer aiMessageId here
    messages[syncIndex].isSynced = true
}

So messages.first(where: { $0.id == aiMessageId }) returns nil, and the code falls back to queryResult.text. For streaming responses, queryResult.text is typically empty (the streamed text lives in messages[index].text, not in queryResult.text), meaning the PostHog event is sent with text: "".

messageText (declared earlier at line 2045 and populated before the ID swap) already holds the correct final AI text. Using it directly avoids the stale lookup:

Suggested change
let aiText = messages.first(where: { $0.id == aiMessageId })?.text ?? queryResult.text
let aiText = messageText

Comment on lines +2222 to +2228
// Track onboarding errors with full context
if isOnboarding {
AnalyticsManager.shared.onboardingChatMessageDetailed(
role: "error", text: trimmedText, step: "chat",
error: rawError
)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Error event text field sends user's query, not the error

The PR description states that error events capture "raw error string when chat fails", but here text: trimmedText is the user's query that triggered the error (not the error itself). The actual error string is in the error: parameter. This means the text field in error events has the opposite semantics from what the PR description implies, and PostHog queries filtering/grouping on text will get the user input rather than the error message.

If the intent is to record the error text in the text field for uniform query patterns, the arguments should be swapped:

Suggested change
// Track onboarding errors with full context
if isOnboarding {
AnalyticsManager.shared.onboardingChatMessageDetailed(
role: "error", text: trimmedText, step: "chat",
error: rawError
)
}
AnalyticsManager.shared.onboardingChatMessageDetailed(
role: "error", text: rawError, step: "chat",
error: rawError
)

Alternatively, if keeping text as the user query is intentional, the PR description and any PostHog dashboards should clarify that text carries the user's input in the role=error case.

props["tool_calls"] = toolCalls.joined(separator: ", ")
}
if let model = model { props["model"] = model }
if let error = error { props["error"] = error }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 error field lacks truncation unlike other error properties

The text field is capped at 2000 chars (String(text.prefix(2000))), and the existing chatAgentError method truncates rawError to 500 chars. The new error property has no upper bound. While errors are usually short, a stack trace or a very verbose bridge error could be unbounded. For consistency with the rest of the analytics codebase, consider adding a cap:

Suggested change
if let error = error { props["error"] = error }
if let error = error { props["error"] = String(error.prefix(500)) }

Glucksberg pushed a commit to Glucksberg/omi-local that referenced this pull request Apr 28, 2026
…ardware#6011)

## Summary
Adds `onboarding_chat_message_detailed` PostHog events that capture:
- **User messages**: full text sent during onboarding chat
- **AI responses**: full text, tool calls used, model
- **Errors**: raw error string when chat fails

Query in PostHog: filter by `onboarding_chat_message_detailed` and group
by `distinct_id` to reconstruct any user's full onboarding conversation.

## Test plan
- [ ] Go through onboarding, check PostHog for
`onboarding_chat_message_detailed` events
- [ ] Verify user messages have `role=user` and AI responses have
`role=assistant`
- [ ] Verify tool calls appear in the `tool_calls` property

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant