INS-286: Use PostHog to track template page visits and downloads#146
Conversation
WalkthroughThe PR adds analytics event emission for marketplace template downloads. When a template downloads successfully, the CLI now captures a ChangesMarketplace template download analytics
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
CarmenDou
changed the title
Greptile SummaryThis PR adds a
Confidence Score: 4/5The production change is minimal and low-risk; the analytics call is correctly gated and consistent with existing patterns. The new captureEvent call is correctly placed inside the if (downloaded) guard and matches the style of every other call site in the file. The only gap is in the test: the regex window was widened to pass but no assertion was added to verify the new event is itself inside the guard, leaving that invariant untested going forward. src/commands/create.marketplace.test.ts — the guard-assertion gap for the new event Important Files Changed
Sequence DiagramsequenceDiagram
participant CLI as create command
participant GH as downloadGitHubTemplate
participant PH as PostHog (captureEvent)
participant MW as Marketplace API (reportMarketplaceDownload)
CLI->>GH: downloadGitHubTemplate(slug, config, json)
GH-->>CLI: downloaded (true/false)
alt "downloaded === true"
CLI->>PH: "captureEvent(orgId, 'marketplace_template_downloaded', { template: slug })"
CLI-->>MW: void reportMarketplaceDownload(slug) [fire-and-forget]
end
Reviews (1): Last reviewed commit: "feat(posthog): track marketplace_templat..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // the marketplace's install count. | ||
| expect(createSource).toMatch(/const downloaded = await downloadGitHubTemplate/); | ||
| expect(createSource).toMatch(/if \(downloaded\)[\s\S]{0,80}reportMarketplaceDownload/); | ||
| expect(createSource).toMatch(/if \(downloaded\)[\s\S]{0,200}reportMarketplaceDownload/); |
There was a problem hiding this comment.
captureEvent call against the downloaded boolean
The existing test was widened to {0,200} so reportMarketplaceDownload still passes, but there is no corresponding assertion that captureEvent(orgId, 'marketplace_template_downloaded', …) is also inside the if (downloaded) block. If the captureEvent call were accidentally moved outside that guard, the test suite would not catch it, silently recording phantom analytics events for failed downloads — the exact failure mode the surrounding comment warns about.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/commands/create.marketplace.test.ts (1)
96-96: 💤 Low valueConsider tightening the regex bound to 150 characters.
The change from
{0,80}to{0,200}accommodates the newcaptureEventcall, but 200 is generous. The actual intervening code (captureEvent + reportMarketplaceDownload) is roughly 120-150 characters. Using{0,150}would provide adequate margin while keeping the structural assertion more precise.🎯 Proposed tighter bound
- expect(createSource).toMatch(/if \(downloaded\)[\s\S]{0,200}reportMarketplaceDownload/); + expect(createSource).toMatch(/if \(downloaded\)[\s\S]{0,150}reportMarketplaceDownload/);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/create.marketplace.test.ts` at line 96, The test's regex in the assertion on createSource is too loose ({0,200}); narrow the character bound to {0,150} to better match the actual intervening code (captureEvent followed by reportMarketplaceDownload). Update the expectation that references createSource (the line using expect(createSource).toMatch(...)) so the pattern uses /if \(downloaded\)[\s\S]{0,150}reportMarketplaceDownload/ to keep the structural check precise while still allowing the new captureEvent call.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/commands/create.marketplace.test.ts`:
- Line 96: The test's regex in the assertion on createSource is too loose
({0,200}); narrow the character bound to {0,150} to better match the actual
intervening code (captureEvent followed by reportMarketplaceDownload). Update
the expectation that references createSource (the line using
expect(createSource).toMatch(...)) so the pattern uses /if
\(downloaded\)[\s\S]{0,150}reportMarketplaceDownload/ to keep the structural
check precise while still allowing the new captureEvent call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 95ca8973-1fb3-4aad-8e96-b42d195a56d6
📒 Files selected for processing (2)
src/commands/create.marketplace.test.tssrc/commands/create.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
jwfing
left a comment
jwfing
left a comment
There was a problem hiding this comment.
Code Review — INS-286: PostHog marketplace_template_downloaded tracking
Summary: Small, focused addition that emits a PostHog event on successful marketplace template download, correctly guarded by the existing downloaded boolean, with a matching doc-comment correction. No blocking issues.
Requirements context
No /docs/superpowers/ directory exists in this repo — assessing against the PR description and the old doc comment that read "PostHog is intentionally not used (per spec §6.3)". The PR description is clear: emit marketplace_template_downloaded after a successful download, keep the DB counter, update docs to reflect PostHog as the analytics source of truth.
Findings
Critical
(none)
Suggestion
Software Engineering — missing dedicated test for the new event (src/commands/create.marketplace.test.ts)
The test file uses structural source-text assertions as its established pattern — there are already tests verifying the presence and shape of captureEvent(orgId, 'template_selected', ...) (lines 99–107) and the counter guard (lines 91–97). The PR adds a new observable behavior (a new captureEvent call with a specific event name and property), but only widens the existing counter-guard regex ({0,80} → {0,200}) rather than adding a new assertion.
Following the existing pattern, a complementary test like the following would pin the new behavior and prevent silent removal:
it('emits marketplace_template_downloaded inside the downloaded guard', () => {
expect(createSource).toMatch(
/if \(downloaded\)[\s\S]{0,200}captureEvent\(orgId, 'marketplace_template_downloaded'/,
);
// verify template property is forwarded
const callIdx = createSource.indexOf("captureEvent(orgId, 'marketplace_template_downloaded'");
const closingParen = createSource.indexOf(');', callIdx);
expect(createSource.slice(callIdx, closingParen)).toMatch(/template.*opts\.marketplace/);
});The insforge-development skill requires "every new behavior has at least one test"; the widened regex in the existing test happens to keep passing, but it no longer directly asserts the new call exists.
Information
src/commands/create.ts:748–756(doc comment): The correction from "PostHog is intentionally not used (per spec §6.3)" to "PostHog is the source of truth for download analytics" is correct and important — the old note would have actively misled future readers. Good housekeeping.- Event ordering (
create.ts:385–388):captureEventfires beforevoid reportMarketplaceDownload(...). Both are fire-and-forget and either order works, but firing analytics first (before the counter) is a reasonable default — a failed counter call cannot suppress the analytics event. orgIdas distinct ID (create.ts:385):orgIdis resolved in step 1 of the action (line 194–216) and is always set before the download block (step 7) executes — no null-identity risk here.
Verdict
approved (informational — human approval required via the approve flow)
Zero Critical findings. One Suggestion to add an explicit structural test for the new event, consistent with the test patterns already present in create.marketplace.test.ts.
2 participants
Summary by cubic
Track marketplace template downloads in PostHog by emitting marketplace_template_downloaded after a successful download, including orgId and template slug. Kept the marketplace counter (fire-and-forget), clarified docs that PostHog is the analytics source of truth, and updated the test regex to allow the new event call.
Written for commit 63db66f. Summary will update on new commits.
Review in cubic
Note
Track
marketplace_template_downloadedPostHog event on successful marketplace template downloadWhen
insforge createis invoked with a valid--marketplaceslug and the GitHub template download succeeds, amarketplace_template_downloadedPostHog event is emitted with the template slug as a property. This fires before the existingreportMarketplaceDownloadDB counter call, which is now documented as backing the on-page download count rather than analytics.Macroscope summarized 63db66f.
Summary by CodeRabbit
New Features
Tests
Documentation