feat: Render PostHog URLs as rich preview chips#2355
Open
Basit-Balogun10 wants to merge 1 commit into
Open
Conversation
Contributor
Prompt To Fix All With AIFix the following 4 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 4
apps/code/src/renderer/features/message-editor/utils/content.ts:81
**"Loading…" placeholder can be persisted in XML**
`chip.label` is written verbatim into the `label` attribute. If the user pastes a PostHog URL and submits the message before `resolvePostHogRefChip` fires (typical API round-trip of 100–500 ms), the label stored in the XML will be `"Feature Flag #42 - Loading..."`. Any later rendering of that message from history will show "Loading…" permanently. The GitHub path avoids this by serialising `number` and `title` separately — a missing title just produces an empty `title` attr and the label is reconstructed cleanly. PostHog chips should apply the same guard, e.g. strip the ` - Loading...` suffix (or fall back to the ID-only label) before persisting.
### Issue 2 of 4
apps/code/src/renderer/api/posthogClient.ts:2891-3070
**Repeated fetch pattern violates OnceAndOnlyOnce**
All 11 new methods share an identical 6-line body: build `urlPath`, construct a `URL`, call `this.api.fetcher.fetch`, return `null` on non-ok, cast the JSON, and return the relevant field. A private helper (e.g. `fetchProjectResource(endpoint, projectId, resourceId): Promise<unknown>`) would let each public method reduce to a one-liner field-pluck, making future endpoint additions or error-handling changes apply in one place.
### Issue 3 of 4
apps/code/src/renderer/features/message-editor/tiptap/MentionChipView.tsx:101-106
`isPostHogRef` is defined by exclusion — it is true for every chip type that is not `file`, `folder`, `command`, `error`, or a GitHub ref. Any new chip type added in the future that is not a URL-based resource would silently be treated as a clickable PostHog chip, triggering a `window.open` with a non-URL `id`. A positive definition against the known PostHog resource types is safer.
```suggestion
const POSTHOG_CHIP_TYPES = new Set<string>([
"experiment",
"insight",
"feature_flag",
"dashboard",
"recording",
"error_tracking",
"survey",
"notebook",
"cohort",
"action",
"early_access_feature",
]);
const isPostHogRef = POSTHOG_CHIP_TYPES.has(type);
```
### Issue 4 of 4
apps/code/src/renderer/api/posthogClient.ts:603-605
`getDefaultProjectId` does nothing but delegate to `getTeamId`. The one caller in `posthogChip.ts` could call `getTeamId()` directly (with its own `String()` cast), or `getDefaultProjectId` could at least drop the unnecessary `async` keyword since it is a single `return` of an already-`Promise`-returning method.
```suggestion
/** Returns the numeric team/project ID to use when a PostHog URL omits the project segment. */
getDefaultProjectId(): Promise<number> {
return this.getTeamId();
}
```
Reviews (1): Last reviewed commit: "feat: Render PostHog URLs as rich previe..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| case "cohort": | ||
| case "action": | ||
| case "early_access_feature": | ||
| return `<${chip.type} id="${escapedId}" label="${escapeXmlAttr(chip.label)}" />`; |
Contributor
There was a problem hiding this comment.
chip.label is written verbatim into the label attribute. If the user pastes a PostHog URL and submits the message before resolvePostHogRefChip fires (typical API round-trip of 100–500 ms), the label stored in the XML will be "Feature Flag #42 - Loading...". Any later rendering of that message from history will show "Loading…" permanently. The GitHub path avoids this by serialising number and title separately — a missing title just produces an empty title attr and the label is reconstructed cleanly. PostHog chips should apply the same guard, e.g. strip the - Loading... suffix (or fall back to the ID-only label) before persisting.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/renderer/features/message-editor/utils/content.ts
Line: 81
Comment:
**"Loading…" placeholder can be persisted in XML**
`chip.label` is written verbatim into the `label` attribute. If the user pastes a PostHog URL and submits the message before `resolvePostHogRefChip` fires (typical API round-trip of 100–500 ms), the label stored in the XML will be `"Feature Flag #42 - Loading..."`. Any later rendering of that message from history will show "Loading…" permanently. The GitHub path avoids this by serialising `number` and `title` separately — a missing title just produces an empty `title` attr and the label is reconstructed cleanly. PostHog chips should apply the same guard, e.g. strip the ` - Loading...` suffix (or fall back to the ID-only label) before persisting.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
2891
to
3070
| } | ||
| return (await response.json()) as SpendAnalysisResponse; | ||
| } | ||
|
|
||
| async getFeatureFlag( | ||
| projectId: string, | ||
| flagId: string, | ||
| ): Promise<{ name: string; key: string } | null> { | ||
| const urlPath = `/api/projects/${encodeURIComponent(projectId)}/feature_flags/${encodeURIComponent(flagId)}/`; | ||
| const url = new URL(`${this.api.baseUrl}${urlPath}`); | ||
| const response = await this.api.fetcher.fetch({ | ||
| method: "get", | ||
| url, | ||
| path: urlPath, | ||
| }); | ||
| if (!response.ok) return null; | ||
| const data = (await response.json()) as { name?: string; key?: string }; | ||
| return { name: data.name ?? "", key: data.key ?? "" }; | ||
| } | ||
|
|
||
| async getExperiment( | ||
| projectId: string, | ||
| experimentId: string, | ||
| ): Promise<{ name: string } | null> { | ||
| const urlPath = `/api/projects/${encodeURIComponent(projectId)}/experiments/${encodeURIComponent(experimentId)}/`; | ||
| const url = new URL(`${this.api.baseUrl}${urlPath}`); | ||
| const response = await this.api.fetcher.fetch({ | ||
| method: "get", | ||
| url, | ||
| path: urlPath, | ||
| }); | ||
| if (!response.ok) return null; | ||
| const data = (await response.json()) as { name?: string }; | ||
| return { name: data.name ?? "" }; | ||
| } | ||
|
|
||
| async getInsight( | ||
| projectId: string, | ||
| insightId: string, | ||
| ): Promise<{ name: string } | null> { | ||
| const urlPath = `/api/projects/${encodeURIComponent(projectId)}/insights/${encodeURIComponent(insightId)}/`; | ||
| const url = new URL(`${this.api.baseUrl}${urlPath}`); | ||
| const response = await this.api.fetcher.fetch({ | ||
| method: "get", | ||
| url, | ||
| path: urlPath, | ||
| }); | ||
| if (!response.ok) return null; | ||
| const data = (await response.json()) as { name?: string }; | ||
| return { name: data.name ?? "" }; | ||
| } | ||
|
|
||
| async getDashboard( | ||
| projectId: string, | ||
| dashboardId: string, | ||
| ): Promise<{ name: string } | null> { | ||
| const urlPath = `/api/projects/${encodeURIComponent(projectId)}/dashboards/${encodeURIComponent(dashboardId)}/`; | ||
| const url = new URL(`${this.api.baseUrl}${urlPath}`); | ||
| const response = await this.api.fetcher.fetch({ | ||
| method: "get", | ||
| url, | ||
| path: urlPath, | ||
| }); | ||
| if (!response.ok) return null; | ||
| const data = (await response.json()) as { name?: string }; | ||
| return { name: data.name ?? "" }; | ||
| } | ||
|
|
||
| async getErrorTrackingGroup( | ||
| projectId: string, | ||
| groupId: string, | ||
| ): Promise<{ title: string } | null> { | ||
| const urlPath = `/api/projects/${encodeURIComponent(projectId)}/error_tracking/${encodeURIComponent(groupId)}/`; | ||
| const url = new URL(`${this.api.baseUrl}${urlPath}`); | ||
| const response = await this.api.fetcher.fetch({ | ||
| method: "get", | ||
| url, | ||
| path: urlPath, | ||
| }); | ||
| if (!response.ok) return null; | ||
| const data = (await response.json()) as { title?: string }; | ||
| return { title: data.title ?? "" }; | ||
| } | ||
|
|
||
| async getRecording( | ||
| projectId: string, | ||
| recordingId: string, | ||
| ): Promise<{ name: string } | null> { | ||
| const urlPath = `/api/projects/${encodeURIComponent(projectId)}/session_recordings/${encodeURIComponent(recordingId)}/`; | ||
| const url = new URL(`${this.api.baseUrl}${urlPath}`); | ||
| const response = await this.api.fetcher.fetch({ | ||
| method: "get", | ||
| url, | ||
| path: urlPath, | ||
| }); | ||
| if (!response.ok) return null; | ||
| const data = (await response.json()) as { name?: string }; | ||
| return { name: data.name ?? "" }; | ||
| } | ||
|
|
||
| async getSurvey( | ||
| projectId: string, | ||
| surveyId: string, | ||
| ): Promise<{ name: string } | null> { | ||
| const urlPath = `/api/projects/${encodeURIComponent(projectId)}/surveys/${encodeURIComponent(surveyId)}/`; | ||
| const url = new URL(`${this.api.baseUrl}${urlPath}`); | ||
| const response = await this.api.fetcher.fetch({ | ||
| method: "get", | ||
| url, | ||
| path: urlPath, | ||
| }); | ||
| if (!response.ok) return null; | ||
| const data = (await response.json()) as { name?: string }; | ||
| return { name: data.name ?? "" }; | ||
| } | ||
|
|
||
| async getNotebook( | ||
| projectId: string, | ||
| notebookId: string, | ||
| ): Promise<{ title: string } | null> { | ||
| const urlPath = `/api/projects/${encodeURIComponent(projectId)}/notebooks/${encodeURIComponent(notebookId)}/`; | ||
| const url = new URL(`${this.api.baseUrl}${urlPath}`); | ||
| const response = await this.api.fetcher.fetch({ | ||
| method: "get", | ||
| url, | ||
| path: urlPath, | ||
| }); | ||
| if (!response.ok) return null; | ||
| const data = (await response.json()) as { title?: string }; | ||
| return { title: data.title ?? "" }; | ||
| } | ||
|
|
||
| async getCohort( | ||
| projectId: string, | ||
| cohortId: string, | ||
| ): Promise<{ name: string } | null> { | ||
| const urlPath = `/api/projects/${encodeURIComponent(projectId)}/cohorts/${encodeURIComponent(cohortId)}/`; | ||
| const url = new URL(`${this.api.baseUrl}${urlPath}`); | ||
| const response = await this.api.fetcher.fetch({ | ||
| method: "get", | ||
| url, | ||
| path: urlPath, | ||
| }); | ||
| if (!response.ok) return null; | ||
| const data = (await response.json()) as { name?: string }; | ||
| return { name: data.name ?? "" }; | ||
| } | ||
|
|
||
| async getAction( | ||
| projectId: string, | ||
| actionId: string, | ||
| ): Promise<{ name: string } | null> { | ||
| const urlPath = `/api/projects/${encodeURIComponent(projectId)}/actions/${encodeURIComponent(actionId)}/`; | ||
| const url = new URL(`${this.api.baseUrl}${urlPath}`); | ||
| const response = await this.api.fetcher.fetch({ | ||
| method: "get", | ||
| url, | ||
| path: urlPath, | ||
| }); | ||
| if (!response.ok) return null; | ||
| const data = (await response.json()) as { name?: string }; | ||
| return { name: data.name ?? "" }; | ||
| } | ||
|
|
||
| async getEarlyAccessFeature( | ||
| projectId: string, | ||
| featureId: string, | ||
| ): Promise<{ name: string } | null> { | ||
| const urlPath = `/api/projects/${encodeURIComponent(projectId)}/early_access_feature/${encodeURIComponent(featureId)}/`; | ||
| const url = new URL(`${this.api.baseUrl}${urlPath}`); | ||
| const response = await this.api.fetcher.fetch({ | ||
| method: "get", | ||
| url, | ||
| path: urlPath, | ||
| }); | ||
| if (!response.ok) return null; | ||
| const data = (await response.json()) as { name?: string }; | ||
| return { name: data.name ?? "" }; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
All 11 new methods share an identical 6-line body: build urlPath, construct a URL, call this.api.fetcher.fetch, return null on non-ok, cast the JSON, and return the relevant field. A private helper (e.g. fetchProjectResource(endpoint, projectId, resourceId): Promise<unknown>) would let each public method reduce to a one-liner field-pluck, making future endpoint additions or error-handling changes apply in one place.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/renderer/api/posthogClient.ts
Line: 2891-3070
Comment:
**Repeated fetch pattern violates OnceAndOnlyOnce**
All 11 new methods share an identical 6-line body: build `urlPath`, construct a `URL`, call `this.api.fetcher.fetch`, return `null` on non-ok, cast the JSON, and return the relevant field. A private helper (e.g. `fetchProjectResource(endpoint, projectId, resourceId): Promise<unknown>`) would let each public method reduce to a one-liner field-pluck, making future endpoint additions or error-handling changes apply in one place.
How can I resolve this? If you propose a fix, please make it concise.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!
Comment on lines
+101
to
+106
| const isPostHogRef = | ||
| type !== "file" && | ||
| type !== "folder" && | ||
| type !== "command" && | ||
| type !== "error" && | ||
| !isGithubRef; |
Contributor
There was a problem hiding this comment.
isPostHogRef is defined by exclusion — it is true for every chip type that is not file, folder, command, error, or a GitHub ref. Any new chip type added in the future that is not a URL-based resource would silently be treated as a clickable PostHog chip, triggering a window.open with a non-URL id. A positive definition against the known PostHog resource types is safer.
Suggested change
| const isPostHogRef = | |
| type !== "file" && | |
| type !== "folder" && | |
| type !== "command" && | |
| type !== "error" && | |
| !isGithubRef; | |
| const POSTHOG_CHIP_TYPES = new Set<string>([ | |
| "experiment", | |
| "insight", | |
| "feature_flag", | |
| "dashboard", | |
| "recording", | |
| "error_tracking", | |
| "survey", | |
| "notebook", | |
| "cohort", | |
| "action", | |
| "early_access_feature", | |
| ]); | |
| const isPostHogRef = POSTHOG_CHIP_TYPES.has(type); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/renderer/features/message-editor/tiptap/MentionChipView.tsx
Line: 101-106
Comment:
`isPostHogRef` is defined by exclusion — it is true for every chip type that is not `file`, `folder`, `command`, `error`, or a GitHub ref. Any new chip type added in the future that is not a URL-based resource would silently be treated as a clickable PostHog chip, triggering a `window.open` with a non-URL `id`. A positive definition against the known PostHog resource types is safer.
```suggestion
const POSTHOG_CHIP_TYPES = new Set<string>([
"experiment",
"insight",
"feature_flag",
"dashboard",
"recording",
"error_tracking",
"survey",
"notebook",
"cohort",
"action",
"early_access_feature",
]);
const isPostHogRef = POSTHOG_CHIP_TYPES.has(type);
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+603
to
+605
| async getDefaultProjectId(): Promise<number> { | ||
| return this.getTeamId(); | ||
| } |
Contributor
There was a problem hiding this comment.
getDefaultProjectId does nothing but delegate to getTeamId. The one caller in posthogChip.ts could call getTeamId() directly (with its own String() cast), or getDefaultProjectId could at least drop the unnecessary async keyword since it is a single return of an already-Promise-returning method.
Suggested change
| async getDefaultProjectId(): Promise<number> { | |
| return this.getTeamId(); | |
| } | |
| /** Returns the numeric team/project ID to use when a PostHog URL omits the project segment. */ | |
| getDefaultProjectId(): Promise<number> { | |
| return this.getTeamId(); | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/renderer/api/posthogClient.ts
Line: 603-605
Comment:
`getDefaultProjectId` does nothing but delegate to `getTeamId`. The one caller in `posthogChip.ts` could call `getTeamId()` directly (with its own `String()` cast), or `getDefaultProjectId` could at least drop the unnecessary `async` keyword since it is a single `return` of an already-`Promise`-returning method.
```suggestion
/** Returns the numeric team/project ID to use when a PostHog URL omits the project segment. */
getDefaultProjectId(): Promise<number> {
return this.getTeamId();
}
```
How can I resolve this? If you propose a fix, please make it concise.
Adds rich chip rendering for PostHog resource URLs, mirroring the
existing GitHub URL chip pattern. PostHog URLs in both agent messages
(MarkdownRenderer) and pasted into the editor (Tiptap) now render as
interactive chips with resource-type icons and resolved titles.
Key additions:
- URL parser supporting 11 resource types (feature flags, experiments,
insights, dashboards, error tracking, recordings, surveys, notebooks,
cohorts, actions, early access features)
- Both long (/project/{id}/...) and short (no project prefix) URL formats
- Async title resolution via PostHog API (shows "Loading..." placeholder,
resolves to actual resource name)
- Chips persist labels through XML round-trip (fixes raw XML showing in
user message blocks)
- Multi-URL paste support for mixed GitHub + PostHog URLs
Closes PostHog#1977
Basit-Balogun10
force-pushed
the
claude/competent-montalcini-7eba82
branch
from
5a07b0e to
f47aa5b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When PostHog resource URLs (dashboards, feature flags, insights, experiments, etc.) appear in agent responses or are pasted into the editor, they render as plain text links — unlike GitHub URLs which already get rich chip rendering with icons and resolved titles. This makes it harder to quickly identify and interact with referenced PostHog resources.
Closes #1977
Changes
Mirrors the GitHub URL → chip pipeline for PostHog Cloud URLs across both the MarkdownRenderer (agent messages) and the Tiptap editor (paste handling).
New files:
posthogUrl.ts— URL parser supporting both long (/project/{id}/...) and short (no project prefix) URL formats. Handlesus.posthog.com,eu.posthog.com, andlocalhost:8010.posthogUrl.test.ts— 31 tests covering all resource types, both URL formats, edge cases (trailing slashes, query params, fragments), and rejection cases.PostHogRefChip.tsx— Read-only chip component for agent messages with per-type Phosphor icons.posthogChip.ts— Chip builder + async title resolution via PostHog API (feature flag names, experiment names, dashboard titles, etc.)Modified files:
MarkdownRenderer.tsx— AddsSmartPostHogRefChipthat resolves resource titles viauseAuthenticatedQuery, same pattern asSmartGithubRefChip.useTiptapEditor.ts— Paste-time title resolution with "Loading..." placeholder → resolved label. Works for single URLs, multi-URL pastes, and mixed GitHub + PostHog URL pastes.content.ts— Persists chip labels in XML (<feature_flag id="..." label="..." />) so they survive the user message round-trip. Old messages without alabelattr gracefully fall back to URL-derived labels.MentionChipNode.ts,MentionChipView.tsx— ExtendedChipTypeunion and icon map for all new resource types.posthogClient.ts— 11 new API methods for fetching resource metadata, plusgetDefaultProjectId()for short URL support.Supported resource types
/feature_flags/{id}Feature Flag #619272 - beta-rollout/experiments/{id}Experiment #373424 - signup-test/insights/{id}Insight KP8iqi6E - Weekly DAU/dashboard/{id}Dashboard #944836 - Product KPIs/error_tracking/{id}Error abc-def-123 - TypeError.../replay/{id}Recording 019012ab.../surveys/{id}Survey 019d1c79... - NPS Q2/notebooks/{id}Notebook wkGd - Sprint retro/cohorts/{id}Cohort #55 - Power users/data-management/actions/{id}Action #99 - Signed up/early_access_features/{id}Early Access Feature abc-123...What's NOT matched
Org-level and non-resource PostHog URLs render as normal links (no chip, no metadata resolution):
/organization/billing/overview/settings/.../feature_flags(index/list page, no specific resource ID)/project/{id}/feature_flags?search=my-flag(search/filter page)These are intentionally excluded — chips are only shown when we can identify a discrete resource to link to.
How did you test this?
Automated:
Manual testing steps:
Agent response chips — Start a session and ask the agent to echo PostHog links. Verify they render as chips with correct icons and resolved titles (not plain links).
Single URL paste — Copy
https://us.posthog.com/feature_flags/619272and paste into the editor. Should show "Feature Flag #619272 - Loading..." then resolve.Short URL format — Paste
https://us.posthog.com/insights/KP8iqi6E(no/project/{id}/). Should parse and chip correctly.Multi-URL paste — Paste a block containing both PostHog and GitHub URLs. Both should render as chips with resolved titles.
User message round-trip — Send a message containing a PostHog chip. Chip should display correctly in the user message block (not raw XML).
Graceful fallback — Paste a PostHog URL to a resource you can't access. Should fall back to URL-derived label without errors.
Org-level URLs — Paste
https://us.posthog.com/organization/billing/overview. Should render as a normal link, NOT a chip.New resource types — Test with survey, notebook, cohort, action, and early access feature URLs.
Real PostHog URLs for testing:
https://us.posthog.com/feature_flags/619272https://us.posthog.com/experiments/373424https://us.posthog.com/insights/KP8iqi6Ehttps://us.posthog.com/dashboard/944836https://us.posthog.com/surveys/019d1c79-170c-0000-b8dc-6880403ecae9https://us.posthog.com/notebooks/wkGdPublish to changelog?
no