feat(rpc): measure saved conversion journeys by cohort - #741
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Greptile SummaryThis PR adds bounded cohort filtering to saved goal and funnel analytics while preserving the saved definition separately from the measured definition.
Confidence Score: 4/5The cohort behavior appears sound, but the explicit repository type-safety and formatting requirements must be satisfied before merging. No behavioral or security failure remains; the accepted findings are confined to concrete repository-rule violations in the newly added and modified tests. Files Needing Attention: packages/ai/src/ai/tools/cohort-read.test.ts, packages/rpc/src/lib/analytics-cohort.integration.test.ts, packages/rpc/src/routers/analytics-measurement.test.ts, apps/insights/src/investigation-flow.test.ts Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[AI analytics tool] --> B[Resolve authorized website]
B --> C[Goal or funnel RPC]
C --> D[Load saved definition]
D --> E[Combine saved filters with read-only cohort]
E --> F[Build cohort-specific cache key]
F --> G[Query ClickHouse analytics]
G --> H[Return measurement and counts]
D --> I[Return savedDefinition separately]
H --> J[Insights investigation]
I --> J
J --> K[Validate repairs against saved definition]
Reviews (1): Last reviewed commit: "feat(rpc): measure saved conversion jour..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 10 files
Architecture diagram
sequenceDiagram
participant Agent as Intelligence Agent
participant FunnelTool as Funnel Tools
participant GoalTool as Goal Tools
participant RPC as RPC Layer
participant Cache as Analytics Cache
participant DB as Database
participant CH as ClickHouse
Note over Agent,CH: Cohort-Based Saved Funnel/Goal Analytics Flow
Agent->>FunnelTool: get_funnel_analytics(funnelId, dates, cohort)
Agent->>GoalTool: get_goal_analytics(goalId, dates, cohort)
Note over FunnelTool,GoalTool: Validate cohort schema (browser/device/country/campaign only, no tenant/step selectors)
FunnelTool->>RPC: callRPCProcedure("funnels", "getAnalytics", {funnelId, websiteId, cohort})
GoalTool->>RPC: callRPCProcedure("goals", "getAnalytics", {goalId, websiteId, cohort})
Note over RPC: withWebsiteRead middleware validates tenant access
alt Inaccessible website
RPC-->>FunnelTool: Error: "not in this workspace"
RPC-->>GoalTool: Error: "not in this workspace"
else Valid tenant access
RPC->>DB: Load saved funnel/goal definition from DB
DB-->>RPC: Saved definition (filters, steps)
RPC->>RPC: Build measurement definition
Note over RPC: Combine saved filters + cohort filters (ANDed)
RPC->>RPC: Cache key based on full measurement JSON
alt Cache hit
Cache-->>RPC: Cached analytics result
else Cache miss
RPC->>CH: processFunnelAnalytics(steps, combinedFilters, params)
RPC->>CH: processGoalAnalytics(steps, combinedFilters, params, entrants)
CH-->>RPC: Analytics metrics (entrants, completions, rate)
RPC->>Cache: Store result with cohort-aware key
end
RPC-->>FunnelTool: {analytics, measurement, savedDefinition, cohort}
RPC-->>GoalTool: {analytics, measurement, savedDefinition, cohort}
end
FunnelTool-->>Agent: Metrics with cohort applied
GoalTool-->>Agent: Metrics with cohort applied
Note over Agent: Validate measurement matches saved definition ID/website
Agent->>Agent: Compare cohorts/periods via parallel calls
opt Agent needs saved definition restored
Agent->>Agent: Use savedDefinition field (without cohort filters)
end
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
|
The latest updates on your projects. Learn more about Unkey Deploy
|
There was a problem hiding this comment.
0 issues found across 5 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Adds optional read-only cohort segmentation to funnel and goal analytics tools and RPC, reusing the authenticated path and restricting the new schema to context fields. Bounded feature; no schema or breaking changes.
Re-trigger cubic
1 participant
Saved funnel analytics could not answer whether a completion decline was concentrated among visitors entering from a particular browser, device, country or campaign. The native tools now accept a bounded read-only cohort and apply it through the existing authenticated goal/funnel query path, preserving the saved definition separately from the measured filters.
Eight fresh synthetic model invocations were audited: three baseline investigations could not measure the browser-specific ordered completions; all five candidate invocations identified Safari 100→20/500 with Chrome 80/500 unchanged, in two model turns each. Two extra candidate invocations were an orchestration mistake and are included, not selected away; their accepted drafts and token usage survive, but two overwritten timing wrappers do not. These are capability results, not proof of automatic discovery, causality or production latency.
Validation: nine tests against an isolated ClickHouse 25.10 instance cover ordered events, saved-filter intersection, tenant collisions, changing later-step context, cohort unions/exclusions, source breakdowns and goal counts. Native tool, RPC/cache and investigation repair-provenance regressions pass. An independent review found a test mock leak; replacing the global RPC mock with a restored scoped spy makes the new tests and existing dry-run/cancellation tests pass together. Root lint, all 33 workspace typechecks, and the full repository test suite pass. The first pre-push attempt also included an unshipped experimental test with a global mock; isolating that experiment from the shipping suite removed its interference. CI lint, types, tests, dashboard E2E, CodeQL and service health checks pass.
Entry cohorts identify visitors by their first-step context; subsequent steps may occur in another browser/session. Missing context and overlapping cohorts are not proof of zero or a partition of all visitors. No saved definitions, database schema or agent strategy change.
Scope: one conversion-analytics read capability. No dependencies. Known overlap: #680 changes goals bulkAnalytics; this changes individual getAnalytics and shared measurement provenance. The broader investigation-discovery experiment will land separately and be refreshed after this PR. AI-assisted maintainer contribution, independently reviewed.
Review resolution: the native forwarding test now uses SDK-inferred spy calls; fixture branches are explicit and new non-null assertions are removed. The warehouse insertion boundary retains unknown JSON values, which matches the ClickHouse transport rather than inventing another event schema. Test names/assertions distinguish the browser-only population from its intersection with the saved country filter, rejected websites assert zero RPC calls, and per-test mock defaults prevent contamination. Link analytics keeps its original non-cohort contract and explicitly rejects the new cohort input; the two native cohort endpoints return measured provenance. This avoids expanding an unrelated endpoint without population metadata.