feat(code): add Discuss button to inbox reports#2241
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 33678e7055
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Opens a chat session pre-loaded with report context and auto-submits the first message so users can talk through a report with the agent without going through the TaskInput screen manually. Falls back to the standard TaskInput with the prompt pre-filled if preconditions (repo, auth, online) don't resolve within 2.5s.
andrewm4894
force-pushed
the
inbox-discuss-button
branch
from
016c739 to
c075c7c
Compare
# Conflicts: # apps/code/src/renderer/features/inbox/components/detail/ReportDetailPane.tsx
- Add 'discuss' to InboxReportActionType with optional has_question flag - Fire INBOX_REPORT_ACTION from the Discuss handler (popover + button) - Defer trpcClient load in MarkdownRenderer link onClick so UserMessage.test doesn't fail at module init without an electronTRPC global
Add optional question_text field (truncated to 500 chars) to InboxReportActionProperties so we can see what users are asking when they submit Discuss via the popover.
| function isPostHogCodeDeeplink(href: string | undefined): href is string { | ||
| if (!href) return false; | ||
| try { | ||
| const protocol = new URL(href).protocol; | ||
| return protocol === "posthog-code:" || protocol === "posthog-code-dev:"; | ||
| } catch { | ||
| return false; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
This should not live here, maybe we already have something for this
There was a problem hiding this comment.
Good catch — there was already a getDeeplinkProtocol helper in apps/code/src/shared/deeplink.ts, so I moved isPostHogCodeDeeplink there alongside it and updated the protocol literals to use the existing constants. MarkdownRenderer now just imports it. Fixed in 75662a8.
|
|
||
| navigateToInbox: () => { | ||
| navigate({ type: "inbox" }); | ||
| track(ANALYTICS_EVENTS.INBOX_VIEWED); |
There was a problem hiding this comment.
Why are we removing this event?
There was a problem hiding this comment.
Not removed, just relocated — when I merged main into this branch, the work from #2228 (feat(inbox): instrument analytics for engagement events) had already redefined INBOX_VIEWED to require a rich InboxViewedProperties payload (report counts, filters, etc.). The bare track(ANALYTICS_EVENTS.INBOX_VIEWED) call here no longer typechecks, and the new structured fire happens inside InboxSignalsTab.tsx once the inbox actually renders with data — see InboxSignalsTab.tsx:478-506. So we still get the event, just with proper props and from a more accurate spot in the lifecycle.
| // the user can submit manually once they pick a repo. | ||
| useEffect(() => { | ||
| if (!isAutoSubmitting) return; | ||
| const timer = setTimeout(() => { | ||
| if (!hasAutoSubmittedRef.current) { | ||
| setIsAutoSubmitting(false); | ||
| } | ||
| }, 2500); | ||
| return () => clearTimeout(timer); | ||
| }, [isAutoSubmitting]); |
There was a problem hiding this comment.
This can easily take longer than 2.5s. I'm sure there's a better solution than relying on a timeout :/
There was a problem hiding this comment.
Gone in 9615f39 — the new useDiscussReport hook calls taskService.createTask directly so there's nothing to time out against. While creation runs the user sees a toast.loading ("Starting discussion...") and the Discuss button shows an inline spinner; on success we navigate, on failure we surface a toast and stay on the inbox.
| const { isCreatingTask, canSubmitBase, canSubmit, handleSubmit } = | ||
| useTaskCreation({ | ||
| editorRef, | ||
| selectedDirectory, | ||
| selectedRepository: selectedCloudRepository, | ||
| githubUserIntegrationId: selectedGithubUserIntegrationId, | ||
| workspaceMode: effectiveWorkspaceMode, | ||
| branch: branchForTaskCreation, | ||
| editorIsEmpty, | ||
| adapter, | ||
| executionMode: currentExecutionMode, | ||
| model: currentModel, | ||
| reasoningLevel: currentReasoningLevel, | ||
| onTaskCreated, | ||
| environmentId: selectedEnvironment, | ||
| sandboxEnvironmentId: | ||
| effectiveWorkspaceMode === "cloud" && selectedCloudEnvId | ||
| ? selectedCloudEnvId | ||
| : undefined, | ||
| signalReportId: activeReportAssociation?.reportId, | ||
| }); | ||
|
|
||
| // Reset auto-submit state on each fresh navigation. We key off | ||
| // `prefillRequestKey` (a fresh UUID per navigation) so back-to-back Discuss | ||
| // clicks each get their own auto-submit attempt even when `autoSubmit` | ||
| // itself doesn't change reference. | ||
| useEffect(() => { | ||
| if (!prefillRequestKey) return; | ||
| hasAutoSubmittedRef.current = false; | ||
| setIsAutoSubmitting(!!autoSubmit); | ||
| }, [prefillRequestKey, autoSubmit]); | ||
|
|
||
| // Fire the first message automatically once preconditions are satisfied. | ||
| // Uses `contentOverride` so we don't race the editor hydrating from the | ||
| // draft store — `handleSubmit` only requires `canSubmitBase` in that path. | ||
| useEffect(() => { | ||
| if (!isAutoSubmitting || hasAutoSubmittedRef.current) return; | ||
| if (!canSubmitBase || !isTaskConfigReady || !initialPrompt) return; | ||
| hasAutoSubmittedRef.current = true; | ||
| void handleSubmit({ | ||
| segments: [{ type: "text", text: initialPrompt }], | ||
| }).then((ok) => { | ||
| if (!ok) { | ||
| setIsAutoSubmitting(false); | ||
| return; | ||
| } | ||
| const draftActions = useDraftStore.getState().actions; | ||
| draftActions.clearPendingContent(sessionId); | ||
| draftActions.setDraft(sessionId, null); | ||
| editorRef.current?.clear(); | ||
| }); | ||
| }, [ | ||
| isAutoSubmitting, | ||
| canSubmitBase, | ||
| isTaskConfigReady, | ||
| handleSubmit, | ||
| initialPrompt, | ||
| sessionId, | ||
| ]); | ||
|
|
||
| // If preconditions never resolve (no repo, offline, etc.), fall back to the | ||
| // normal UI after a short grace period — the prompt stays in the editor so | ||
| // the user can submit manually once they pick a repo. | ||
| useEffect(() => { | ||
| if (!isAutoSubmitting) return; | ||
| const timer = setTimeout(() => { | ||
| if (!hasAutoSubmittedRef.current) { | ||
| setIsAutoSubmitting(false); | ||
| } | ||
| }, 2500); | ||
| return () => clearTimeout(timer); | ||
| }, [isAutoSubmitting]); | ||
|
|
There was a problem hiding this comment.
This whole thing is doing way too much IMO:
- three effects
- canSubmitBase as a workaround..?
- 2.5s timeout
- lots of new state
Maybe it's better if you just directly call taskService.createTask from the Discuss component instead
There was a problem hiding this comment.
Refactored as suggested in 9615f39. The Discuss button now goes straight through a small useDiscussReport hook that calls taskService.createTask directly — no TaskInput round-trip. That dissolves all of the things you flagged: the three effects, the canSubmitBase workaround, the 2.5s timeout, the loading placeholder, plus the matching transient state in navigationStore (autoSubmit, initialExecutionMode) and the editor-bypass changes in useTaskCreation. While the task is being created the Discuss button shows an in-place spinner and a toast.loading (same pattern as connectivityToast.ts); on success we jump to the task detail page, on failure a toast surfaces and we stay on the inbox. Net diff: ~125 lines removed.
There was a problem hiding this comment.
hopefully a bit simpler now - just uses a toast and flow all still feels same as a user - tested it locally
| has_question?: boolean; | ||
| // The first question text the user typed before hitting Discuss. Truncated to | ||
| // 500 chars to keep event payloads bounded. | ||
| question_text?: string; |
There was a problem hiding this comment.
I don't think this should go inside the event right? And if we're truncating it, why even include it
There was a problem hiding this comment.
would like to keep this one - the motivation is mostly analytics ergonomics on a brand-new feature. Even truncated, having the first question text on the event makes it really easy to get a quick feel for the kinds of follow-ons people want without having to join against task/chat data each time. The full prompt is already going to the backend via the created task, so the truncated snippet on the event doesn't really add new PII exposure on top of that. Happy to revisit and drop it once Discuss is more mature and we have a clearer view of what's worth keeping long-term.
(and when we have some sort of opt out layer too for things like this for users)
| const prompt = trimmedQuestion | ||
| ? `Discuss PostHog inbox report ${report.id} ([inbox item](${reportLink})). Use the inbox MCP tools to fetch the report, then answer this first: ${trimmedQuestion}` | ||
| : `Discuss PostHog inbox report ${report.id} ([inbox item](${reportLink})). Use the inbox MCP tools to fetch the report, then give me a brief readout and ask what I want to dig into.`; |
There was a problem hiding this comment.
Let's extract this to a builder
There was a problem hiding this comment.
Done — extracted to apps/code/src/renderer/features/inbox/utils/buildDiscussReportPrompt.ts with its own unit tests in 32fe704. ReportDetailPane now just calls buildDiscussReportPrompt({ reportId, question, isDevBuild }) so all the prompt copy lives in one place.
Move the helper out of MarkdownRenderer so other renderer surfaces can reuse it instead of redefining the protocol check.
Pull the inline prompt template out of ReportDetailPane into a buildDiscussReportPrompt util with its own tests, so the Discuss prompt copy lives in one place and is easy to tweak.
Replace the auto-submit dance that routed Discuss clicks through TaskInput (loading placeholder, three effects, 2.5s fallback timeout, canSubmitBase workaround) with a dedicated useDiscussReport hook that calls taskService.createTask straight from the inbox detail pane. The Discuss button now shows an in-place spinner plus a loading toast while the task is created; on success we jump to the task detail page, on failure we surface a toast and stay on the inbox. TaskInput drops the autoSubmit / initialExecutionMode props and the associated render branch, navigationStore drops the matching transient view fields, and useTaskCreation reverts to requiring a mounted editor.
| onClick={(event) => { | ||
| if (!isDeeplink || !href) return; | ||
| event.preventDefault(); | ||
| // Lazy-load the tRPC client so tests that render markdown don't need | ||
| // an electronTRPC global at module init time. | ||
| void import("@renderer/trpc/client").then(({ trpcClient }) => | ||
| trpcClient.os.openExternal.mutate({ url: href }), | ||
| ); | ||
| }} |
There was a problem hiding this comment.
Any way we can avoid this inline import pattern?
There was a problem hiding this comment.
Done in cfe9107. Inline import is gone — back to a top-level import { trpcClient } like every other call site.
The reason it was lazy in the first place was that UserMessage.test.tsx was failing at module init because trpc/client.ts calls ipcLink() which needs window.electronTRPC, and MarkdownRenderer was the first thing in UserMessage's import graph to reach it. Fixed properly by adding vi.mock("@renderer/trpc/client", ...) to the test — same pattern 14 other test files already use.
…ge test
The lazy `import("@renderer/trpc/client")` inside the link onClick was
working around UserMessage.test.tsx failing at module init (trpc/client.ts
calls ipcLink() which needs window.electronTRPC). Top-level import matches
every other call site in the renderer; the test gets the same vi.mock
pattern other test files use.
2 participants
Problem
Inbox reports have Dismiss and Create PR actions, but no lightweight way to talk through a report with the agent before deciding what to do. Manually opening a new task, referencing the report, choosing the right mode, and submitting is too much friction for “let me think about this with you.”
Changes
Kick off discussion and optionally add a question or prompt:
Then a chat will open in auto mode and use the mcp to try answer/respond to your initial prompt:
posthog-code://andposthog-code-dev://deeplinks.How did you test this?
pnpm --filter code typecheckpnpm typecheck.Publish to changelog?
no
Small, frontend-focused workflow improvement — candidate for stamphog AI-assisted approval.