Repository navigation
Measure the activation funnel and separate agent-driven forks - #942
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (18)
🚧 Files skipped from review as they are similar to previous changes (10)
📝 WalkthroughWalkthroughThe change adds persisted activation milestones, fork-actor attribution, activation aggregates to admin insights, and dashboard visualizations for activation funnels and fork sources. It also updates migration tracking, account data targets, runtime instrumentation, schemas, and tests. ChangesActivation insights
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PackageRuntime
participant ActivationTracking
participant ActivationDatabase
participant AdminInsights
participant InsightsDashboard
PackageRuntime->>ActivationTracking: record successful package run
ActivationTracking->>ActivationDatabase: store milestone and query run count
ActivationTracking->>ActivationDatabase: store package_activated after second success
AdminInsights->>ActivationDatabase: query milestones and fork actor counts
AdminInsights->>InsightsDashboard: return activation aggregates
InsightsDashboard->>InsightsDashboard: render funnel and fork-source charts
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
kody-bot
marked this pull request as ready for review
July 25, 2026 20:26
Activation is defined as a package that ran successfully at least twice — the point where a user has something working unattended rather than something they got running once during setup. Most funnel steps were already derivable: signup and verification from `users`, agent connection from `mcp_agent_sessions`, first fork from `community_forks`. Two were not, and this adds exactly those. Run-derived milestones need durable storage because `package_runtime_runs` is retention-pruned, so a milestone not captured when it happens cannot be recovered later. The migration backfills what still survives. Fork actor matters because a person clicking install and that person's agent calling `community_fork` were the same row. They are not the same activation signal — we have at least one 5-star community rating left by a user's agent without the user knowing, so agent activity can inflate these numbers if it is not separated. Adds an activation funnel and fork-actor breakdown to /admin/insights. Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Backfilled milestones carry no verification date to measure from, so the median can be null while users have genuinely activated. The subtitle read "Nobody has activated yet" in that case, contradicting the funnel directly below it. Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/worker/src/app/admin-insights-data.ts (1)
213-213: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMigration needed to handle stale
admin-insights:v2cache entries foractivation.
loadAdminInsightsDatacaches the full payload under the unchangedadmin-insights:v2key, whileAdminInsightsLoaderData.activationis now required and populated byqueryActivation(). Entries cached before this deploy will deserialize withoutactivationunless the staleness handling/checkValuemigration or cache-key bump handles format incompatibilities.🤖 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 `@packages/worker/src/app/admin-insights-data.ts` at line 213, Update loadAdminInsightsData and its admin-insights:v2 cache validation or versioning so previously cached payloads missing the required AdminInsightsLoaderData.activation field are treated as stale and regenerated via queryActivation(db). Preserve valid current-format cache entries while ensuring incompatible legacy entries are not returned.
🤖 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.
Inline comments:
In `@packages/worker/client/routes/admin-insights.tsx`:
- Around line 140-188: Add a stable, unique React key to the outer div returned
by activation.steps.map in renderActivationFunnel, using the step identifier
already available on each entry, while leaving the funnel rendering unchanged.
In `@packages/worker/migrations/0093-activation-milestones.sql`:
- Around line 45-48: Update the package_run_succeeded backfill query in
migration 0093 to select the earliest successful package_runtime_runs row per
user and persist that row’s package_id instead of NULL. Preserve the existing
user grouping and first-success timestamp semantics while ensuring the selected
package_id comes from the same earliest run.
In `@packages/worker/src/community/community-flow-test-schema.ts`:
- Line 112: Add a best-effort additive ALTER for the actor column to the
community_forks schema migration, alongside the existing additiveAlters entries.
Match the column definition and constraints used by the CREATE TABLE schema,
while tolerating the already-exists case so existing and newly created databases
both remain compatible.
---
Nitpick comments:
In `@packages/worker/src/app/admin-insights-data.ts`:
- Line 213: Update loadAdminInsightsData and its admin-insights:v2 cache
validation or versioning so previously cached payloads missing the required
AdminInsightsLoaderData.activation field are treated as stale and regenerated
via queryActivation(db). Preserve valid current-format cache entries while
ensuring incompatible legacy entries are not returned.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e6fdfea1-f1c3-466f-a3b7-5638040cabb9
📒 Files selected for processing (18)
packages/worker/client/routes/admin-insights.tsxpackages/worker/migrations/0093-activation-milestones.sqlpackages/worker/src/app/account-data-targets.tspackages/worker/src/app/admin-insights-data.node.test.tspackages/worker/src/app/admin-insights-data.tspackages/worker/src/app/loader-data.tspackages/worker/src/community/community-activity-service.node.test.tspackages/worker/src/community/community-flow-test-schema.tspackages/worker/src/community/install.tspackages/worker/src/community/repo.tspackages/worker/src/community/service.tspackages/worker/src/community/types.tspackages/worker/src/mcp/capabilities/community/fork.tspackages/worker/src/package-runtime/package-runtime-debug.tspackages/worker/src/usage/activation.node.test.tspackages/worker/src/usage/activation.tstools/check-migrations.node.test.tstools/migration-ledger.json
Comment on lines
+140
to
+188
| function renderActivationFunnel(activation: AdminInsightsActivation) { | ||
| const total = activation.steps[0]?.users ?? 0 | ||
| return ( | ||
| <div mix={css({ display: 'grid', gap: spacing.sm })}> | ||
| {activation.steps.map((entry) => { | ||
| const share = total > 0 ? entry.users / total : 0 | ||
| return ( | ||
| <div mix={css({ display: 'grid', gap: spacing.xs })}> | ||
| <div | ||
| mix={css({ | ||
| display: 'flex', | ||
| justifyContent: 'space-between', | ||
| gap: spacing.sm, | ||
| fontSize: typography.fontSize.sm, | ||
| color: colors.text, | ||
| })} | ||
| > | ||
| <span>{activationStepLabels[entry.step]}</span> | ||
| <span mix={css({ color: colors.textMuted })}> | ||
| {formatIntegerNumber(entry.users)} · {formatPercentShare(share)} | ||
| </span> | ||
| </div> | ||
| <div | ||
| role="img" | ||
| aria-label={`${activationStepLabels[entry.step]}: ${entry.users} users, ${formatPercentShare(share)} of signups`} | ||
| mix={css({ | ||
| height: '10px', | ||
| borderRadius: '999px', | ||
| background: colors.border, | ||
| overflow: 'hidden', | ||
| })} | ||
| > | ||
| <div | ||
| mix={css({ | ||
| height: '100%', | ||
| width: `${Math.round(share * 100)}%`, | ||
| background: | ||
| entry.step === 'package_activated' | ||
| ? chartColor.emerald | ||
| : chartColor.blue, | ||
| })} | ||
| /> | ||
| </div> | ||
| </div> | ||
| ) | ||
| })} | ||
| </div> | ||
| ) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Missing key on mapped funnel rows.
activation.steps.map(...) renders a <div> per step without a key, unlike the DonutChart slices elsewhere in this file which set key={slice.label}.
🔧 Proposed fix
{activation.steps.map((entry) => {
const share = total > 0 ? entry.users / total : 0
return (
- <div mix={css({ display: 'grid', gap: spacing.xs })}>
+ <div key={entry.step} mix={css({ display: 'grid', gap: spacing.xs })}>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function renderActivationFunnel(activation: AdminInsightsActivation) { | |
| const total = activation.steps[0]?.users ?? 0 | |
| return ( | |
| <div mix={css({ display: 'grid', gap: spacing.sm })}> | |
| {activation.steps.map((entry) => { | |
| const share = total > 0 ? entry.users / total : 0 | |
| return ( | |
| <div mix={css({ display: 'grid', gap: spacing.xs })}> | |
| <div | |
| mix={css({ | |
| display: 'flex', | |
| justifyContent: 'space-between', | |
| gap: spacing.sm, | |
| fontSize: typography.fontSize.sm, | |
| color: colors.text, | |
| })} | |
| > | |
| <span>{activationStepLabels[entry.step]}</span> | |
| <span mix={css({ color: colors.textMuted })}> | |
| {formatIntegerNumber(entry.users)} · {formatPercentShare(share)} | |
| </span> | |
| </div> | |
| <div | |
| role="img" | |
| aria-label={`${activationStepLabels[entry.step]}: ${entry.users} users, ${formatPercentShare(share)} of signups`} | |
| mix={css({ | |
| height: '10px', | |
| borderRadius: '999px', | |
| background: colors.border, | |
| overflow: 'hidden', | |
| })} | |
| > | |
| <div | |
| mix={css({ | |
| height: '100%', | |
| width: `${Math.round(share * 100)}%`, | |
| background: | |
| entry.step === 'package_activated' | |
| ? chartColor.emerald | |
| : chartColor.blue, | |
| })} | |
| /> | |
| </div> | |
| </div> | |
| ) | |
| })} | |
| </div> | |
| ) | |
| } | |
| function renderActivationFunnel(activation: AdminInsightsActivation) { | |
| const total = activation.steps[0]?.users ?? 0 | |
| return ( | |
| <div mix={css({ display: 'grid', gap: spacing.sm })}> | |
| {activation.steps.map((entry) => { | |
| const share = total > 0 ? entry.users / total : 0 | |
| return ( | |
| <div key={entry.step} mix={css({ display: 'grid', gap: spacing.xs })}> | |
| <div | |
| mix={css({ | |
| display: 'flex', | |
| justifyContent: 'space-between', | |
| gap: spacing.sm, | |
| fontSize: typography.fontSize.sm, | |
| color: colors.text, | |
| })} | |
| > | |
| <span>{activationStepLabels[entry.step]}</span> | |
| <span mix={css({ color: colors.textMuted })}> | |
| {formatIntegerNumber(entry.users)} · {formatPercentShare(share)} | |
| </span> | |
| </div> | |
| <div | |
| role="img" | |
| aria-label={`${activationStepLabels[entry.step]}: ${entry.users} users, ${formatPercentShare(share)} of signups`} | |
| mix={css({ | |
| height: '10px', | |
| borderRadius: '999px', | |
| background: colors.border, | |
| overflow: 'hidden', | |
| })} | |
| > | |
| <div | |
| mix={css({ | |
| height: '100%', | |
| width: `${Math.round(share * 100)}%`, | |
| background: | |
| entry.step === 'package_activated' | |
| ? chartColor.emerald | |
| : chartColor.blue, | |
| })} | |
| /> | |
| </div> | |
| </div> | |
| ) | |
| })} | |
| </div> | |
| ) | |
| } |
🤖 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 `@packages/worker/client/routes/admin-insights.tsx` around lines 140 - 188, Add
a stable, unique React key to the outer div returned by activation.steps.map in
renderActivationFunnel, using the step identifier already available on each
entry, while leaving the funnel rendering unchanged.
Comment on lines
+45
to
+48
| SELECT user_id, 'package_run_succeeded', MIN(started_at), NULL | ||
| FROM package_runtime_runs | ||
| WHERE status = 'success' | ||
| GROUP BY user_id; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Persist the backfilled first-success package ID.
This writes NULL for every historical package_run_succeeded record, permanently losing the package attribution that this table is meant to retain. Select the earliest successful run row per user, including its package_id.
Proposed fix
-SELECT user_id, 'package_run_succeeded', MIN(started_at), NULL
-FROM package_runtime_runs
-WHERE status = 'success'
-GROUP BY user_id;
+SELECT user_id, 'package_run_succeeded', started_at, package_id
+FROM (
+ SELECT
+ user_id,
+ package_id,
+ started_at,
+ ROW_NUMBER() OVER (
+ PARTITION BY user_id ORDER BY started_at, package_id
+ ) AS rn
+ FROM package_runtime_runs
+ WHERE status = 'success'
+)
+WHERE rn = 1;🤖 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 `@packages/worker/migrations/0093-activation-milestones.sql` around lines 45 -
48, Update the package_run_succeeded backfill query in migration 0093 to select
the earliest successful package_runtime_runs row per user and persist that row’s
package_id instead of NULL. Preserve the existing user grouping and
first-success timestamp semantics while ensuring the selected package_id comes
from the same earliest run.
| listing_kody_id TEXT, | ||
| adopted_at TEXT, | ||
| adoption_note TEXT, | ||
| actor TEXT CHECK (actor IS NULL OR actor IN ('human', 'agent')), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Upgrade existing community_forks schemas with actor.
CREATE TABLE IF NOT EXISTS will not add this column to an existing database, and additiveAlters omits it. Since new inserts always include actor, pre-existing schemas will fail with “no column named actor.” Add a best-effort ALTER TABLE community_forks ADD COLUMN actor ... alongside the other additive alters.
🤖 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 `@packages/worker/src/community/community-flow-test-schema.ts` at line 112, Add
a best-effort additive ALTER for the actor column to the community_forks schema
migration, alongside the existing additiveAlters entries. Match the column
definition and constraints used by the CREATE TABLE schema, while tolerating the
already-exists case so existing and newly created databases both remain
compatible.
cursor
Bot
force-pushed
the
cursor/activation-instrumentation-34ba
branch
from
July 25, 2026 20:57
65de95a to
2a744ec
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2a744ec. Configure here.
| const row = await env.APP_DB.prepare(countSuccessfulRunsStatement) | ||
| .bind(input.userId, input.packageId) | ||
| .first<{ n: number }>() | ||
| if (Number(row?.n ?? 0) < 2) return |
Contributor
There was a problem hiding this comment.
Milestone before success count check
Medium Severity
recordSuccessfulPackageRun inserts the package_run_succeeded milestone before it counts successful rows in package_runtime_runs. finishPackageRuntimeRun still calls it when status is success even if the preceding status UPDATE failed, so a user can be counted as “Package ran once” with zero persisted successful runs, skewing the activation funnel.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 2a744ec. Configure here.
Contributor
|
🔎 Preview deployed: https://kody-pr-942.kody-a99.workers.dev Worker: Mocks:
|
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
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.
We agreed activation is a package that ran successfully at least twice — the point where someone has something working unattended, rather than something they got running once during setup. This makes that measurable.
Baseline is perishable: every onboarding change from here on is unfalsifiable without a before. That's why this lands first.
What needed new storage, and what didn't
Most funnel steps were already derivable, so they're read where they live rather than duplicated:
usersmcp_agent_sessionscommunity_forksTwo things were not derivable.
Run-derived milestones.
package_runtime_runsis retention-pruned (packages/worker/src/app/retention.ts), so a milestone not captured when it happens cannot be recovered.user_activation_milestonesrecords them once, on the way past. The migration backfills from the runs that still survive — that under-counts, and a partial baseline beats none.Fork actor. A person clicking install and that person's agent calling
community_forkwere the same row. They are not the same activation signal, and we have a concrete reason to care: at least one 5-star community rating was left by a user's agent without the user knowing. Agent activity will inflate these numbers if it isn't separated.community_forks.actoris now set by both call paths —install.tsdefaults tohuman, the MCP capability passesagent. Existing rows stay NULL because their origin is genuinely unknown.Activation is counted per package, not per user: two different packages that each ran once is a user still in setup, not a user with something working.
Dashboard
Seeded local data. The funnel is scaled against signups so drop-off is the thing you see, and the median is measured from email verification rather than signup so invite lag doesn't pollute it.
Manual testing caught one bug worth calling out: the subtitle read "Nobody has activated yet" while the funnel directly below showed two activated users. Backfilled milestones have no verification date to measure from, so a null median doesn't mean nobody activated. Fixed in the second commit, with a regression test.
Validation
npm run validatepasses clean —format:check,lint,typecheck, 1290 unit tests, Playwright E2E, MCP E2E,primitives:check,migrations:checkall exit 0. Migration0093applies cleanly against a from-scratch local D1 (all 99 migrations).The repo's schema-coverage guardrails did their job here and required
user_activation_milestonesto be registered inaccountUserDataTargets, so the new table is covered by account deletion and export like every other user-owned table.Note:
search-hidden-packagesandmeta-list-capabilitiesintermittently hit the 5s vitest timeout when validate runs alongside a dev server and browser. Both pass in isolation and in a quiet run; unrelated to this change, but worth knowing if CI is ever noisy on those two.Follow-ups not in this PR
Time-to-activation is only measurable going forward for users who verify after this deploys. And
agent_connectedcounts anyone with an MCP session row, which is a connection rather than a successful first call — worth tightening if that step turns out to be the drop-off the feedback suggests it is.Summary by CodeRabbit
New Features
Bug Fixes