Release: revenue attribution, users LTV + email lookup, feedback pipeline, slack agent reliability and chart images#541
Release: revenue attribution, users LTV + email lookup, feedback pipeline, slack agent reliability and chart images#541izadoesdev wants to merge 136 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
|
The latest updates on your projects. Learn more about Unkey Deploy
|
Greptile SummaryThis PR centralizes AI query trait-filter resolution into a new
Confidence Score: 4/5The core refactor is sound — trait resolution is cleanly centralized, index bookkeeping in executeBatch is correct, and the session guard fix prevents accidental sign-outs. The two flagged items are edge-case inefficiencies that do not break the happy path. The unknown-type guard in resolveRequestTraitFilters lets an unnecessary identity-service round-trip happen before an unknown query type fails downstream, and the raw ClickHouse error strings now reach the LLM context without any sanitization pass. Neither causes incorrect data or broken flows on valid inputs, but both deserve a follow-up before the code sees heavy use with novel query types or noisy ClickHouse errors. packages/ai/src/query/trait-filters.ts (unknown-type guard) and packages/ai/src/ai/mcp/agent-tools.ts (raw error forwarding) are worth a second look. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant AI as AI Agent / Tool
participant MCP as MCP buildBatchQueryRequests
participant EX as executeBatch
participant TF as resolveRequestTraitFilters
participant ID as Identity Service (resolveTraitSegment)
participant CH as ClickHouse
AI->>MCP: "queries[] with trait:<key> filters"
MCP->>MCP: invalidFilterFieldError() per query
MCP-->>AI: invalid[] (bad field) + requests[] (ok)
AI->>EX: executeBatch(requests)
loop per request (parallel)
EX->>TF: resolveRequestTraitFilters(req)
alt has trait filters
TF->>ID: resolveTraitSegment(projectId, traitFilters)
ID-->>TF: profile_id[]
TF-->>EX: req with profile_id filter
else no trait filters
TF-->>EX: original req (no-op)
end
end
EX->>CH: union query (successful requests)
CH-->>EX: rows
EX-->>AI: BatchResult[] (with errors for failures)
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant AI as AI Agent / Tool
participant MCP as MCP buildBatchQueryRequests
participant EX as executeBatch
participant TF as resolveRequestTraitFilters
participant ID as Identity Service (resolveTraitSegment)
participant CH as ClickHouse
AI->>MCP: "queries[] with trait:<key> filters"
MCP->>MCP: invalidFilterFieldError() per query
MCP-->>AI: invalid[] (bad field) + requests[] (ok)
AI->>EX: executeBatch(requests)
loop per request (parallel)
EX->>TF: resolveRequestTraitFilters(req)
alt has trait filters
TF->>ID: resolveTraitSegment(projectId, traitFilters)
ID-->>TF: profile_id[]
TF-->>EX: req with profile_id filter
else no trait filters
TF-->>EX: original req (no-op)
end
end
EX->>CH: union query (successful requests)
CH-->>EX: rows
EX-->>AI: BatchResult[] (with errors for failures)
Reviews (1): Last reviewed commit: "chore(staging): merge main" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const config = QueryBuilders[request.type]; | ||
| if (config && !isFilterFieldAllowed(config, "profile_id")) { | ||
| throw new TraitFilterError( | ||
| `Trait filters are not supported for ${request.type}. Query types that support them accept a profile_id filter.` | ||
| ); | ||
| } |
There was a problem hiding this comment.
config is undefined (unknown query type), the guard if (config && !isFilterFieldAllowed(config, "profile_id")) is a no-op — config is falsy so the throw is skipped. The function then calls resolveTraitSegment (a network round-trip to the identity service) before the query ultimately fails downstream with an "unknown type" error. The guard should also reject when the config is missing entirely.
| const config = QueryBuilders[request.type]; | |
| if (config && !isFilterFieldAllowed(config, "profile_id")) { | |
| throw new TraitFilterError( | |
| `Trait filters are not supported for ${request.type}. Query types that support them accept a profile_id filter.` | |
| ); | |
| } | |
| const config = QueryBuilders[request.type]; | |
| if (!config || !isFilterFieldAllowed(config, "profile_id")) { | |
| throw new TraitFilterError( | |
| `Trait filters are not supported for ${request.type}. Query types that support them accept a profile_id filter.` | |
| ); | |
| } |
| data: r.data, | ||
| rowCount: r.data.length, | ||
| ...(r.error && { error: "Query failed" }), | ||
| ...(r.error && { error: r.error }), |
There was a problem hiding this comment.
"Query failed" to r.error means ClickHouse-level error messages (which can include table names, column names, and query fragments) are forwarded verbatim to the LLM context. For filter-validation errors this is intentional and useful, but a transient ClickHouse failure or a malformed query could expose internal schema details. Consider forwarding r.error as-is for known structured errors (e.g. TraitFilterError messages or the filter-field error format) and falling back to a sanitized string for raw database errors.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 521 files
Confidence score: 2/5
- In
apps/basket/src/hooks/auth.ts,_resolveOwnerIdnow appears to fail hard on member-query errors instead of returningnull, which can make owner lookup failures take down the whole auth lookup path and cause avoidable sign-in/access regressions—restore the previous fallback behavior (capture +null) before merging. - In
apps/api/src/http/errors.ts, production responses now unconditionally include raw Elysia validation messages, which can bypass the existingexposeStructuredsanitization intent and leak internal validation detail to clients—reapply the production gating so only sanitized fields are exposed. - In
apps/dashboard/components/layout/organization-selector.tsx, switching orgs callsqueryClient.clear(), which can wipe unrelated cache and pending mutations (billing/session/flags) and introduce cross-page state loss or flaky UX after an org switch—replace this with targeted invalidation/removal for org-scoped queries. - In
apps/docs/public/pricing.md, the dropped(Scale)label and removedScale vs Enterprisecross-reference can mislead customers if entitlement configs still usescale, increasing support and sales confusion—restore the tier naming/cross-reference to match current product configuration before release.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/api/src/http/errors.ts">
<violation number="1" location="apps/api/src/http/errors.ts:83">
P2: Raw Elysia validation messages are exposed unconditionally in production responses, bypassing the existing `exposeStructured` sanitization gate that protects `why`, `fix`, and `link`.
In production, `exposeStructured` is `false` for `ValidationError` (it is not an `EvlogError`), so the top-level `error` field safely falls back to "Invalid request". However, the `details` array containing up to 20 raw `issue.summary` / `issue.message` strings is still included. Those messages can leak schema field names, custom validation logic, header/cookie expectations, or reflected input text that the `getSafeErrorMessage` policy is meant to suppress.
Consider gating `details` behind the same `exposeStructured` check, or introducing an explicit allowlist/gate for which validation messages are safe to expose publicly.</violation>
</file>
<file name="apps/basket/src/hooks/auth.ts">
<violation number="1" location="apps/basket/src/hooks/auth.ts:56">
P1: Owner lookup failures are now fatal to the entire website auth lookup, which is a regression for ingest resilience. `_resolveOwnerId` previously returned `null` on member-query failures (after `captureError`); now it throws `websiteLookupUnavailable`, and that error propagates through `getWebsiteByIdWithOwnerCached` and `getWebsiteByIdV2`. Since `ownerId` is typed as nullable (`string | null`) and all callers treat it as optional enrichment—not a hard auth gate—`ownerId` resolution should remain a soft failure. Keeping it soft means a transient member-table issue won't take down `/track` and `/identify` requests for otherwise-valid websites. Consider returning `null` from `_resolveOwnerId` on non-EvlogError DB failures so the website lookup still succeeds, or at least catching the owner-resolution error inside `getWebsiteByIdWithOwnerCached` and returning the website with `ownerId: null` so the optional enrichment doesn't block the auth path.</violation>
</file>
<file name="apps/dashboard/components/layout/organization-selector.tsx">
<violation number="1" location="apps/dashboard/components/layout/organization-selector.tsx:164">
P2: Organization switch now uses `queryClient.clear()`, which wipes the entire React Query cache including billing state, session metadata, feature flags, and any pending mutations. The previous code was more targeted: it removed only org-scoped query keys and explicitly invalidated the active-organization query so `useOrganizationsContext` promptly picked up the new org. Global `clear()` makes that refresh implicit and risks transient empty-state flicker during the redirect to `/websites`. Prefer restoring scoped invalidation — remove org-scoped keys and explicitly invalidate the active-organization query instead.</violation>
</file>
<file name="apps/docs/public/pricing.md">
<violation number="1" location="apps/docs/public/pricing.md:35">
P2: The product-limits table header lost the `(Scale)` annotation and the definitions section dropped the `Scale vs Enterprise` cross-reference. If internal tier IDs and entitlement configs still use `scale` — which the codebase suggests via `pricing/_pricing/estimator-scale.ts` — this removal removes an important breadcrumb for keeping docs and code aligned. Future edits to either side can drift silently. Consider keeping a visible reminder (e.g., restoring `Enterprise (Scale)` in the header or a short footnote) so reviewers know the internal plan id is still `scale`.</violation>
</file>
Note: This PR contains a large number of files. cubic only reviews up to 200 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
| message: "Failed to fetch workspace owner", | ||
| organizationId, | ||
| }); | ||
| throw basketErrors.websiteLookupUnavailable(); |
There was a problem hiding this comment.
P1: Owner lookup failures are now fatal to the entire website auth lookup, which is a regression for ingest resilience. _resolveOwnerId previously returned null on member-query failures (after captureError); now it throws websiteLookupUnavailable, and that error propagates through getWebsiteByIdWithOwnerCached and getWebsiteByIdV2. Since ownerId is typed as nullable (string | null) and all callers treat it as optional enrichment—not a hard auth gate—ownerId resolution should remain a soft failure. Keeping it soft means a transient member-table issue won't take down /track and /identify requests for otherwise-valid websites. Consider returning null from _resolveOwnerId on non-EvlogError DB failures so the website lookup still succeeds, or at least catching the owner-resolution error inside getWebsiteByIdWithOwnerCached and returning the website with ownerId: null so the optional enrichment doesn't block the auth path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/basket/src/hooks/auth.ts, line 56:
<comment>Owner lookup failures are now fatal to the entire website auth lookup, which is a regression for ingest resilience. `_resolveOwnerId` previously returned `null` on member-query failures (after `captureError`); now it throws `websiteLookupUnavailable`, and that error propagates through `getWebsiteByIdWithOwnerCached` and `getWebsiteByIdV2`. Since `ownerId` is typed as nullable (`string | null`) and all callers treat it as optional enrichment—not a hard auth gate—`ownerId` resolution should remain a soft failure. Keeping it soft means a transient member-table issue won't take down `/track` and `/identify` requests for otherwise-valid websites. Consider returning `null` from `_resolveOwnerId` on non-EvlogError DB failures so the website lookup still succeeds, or at least catching the owner-resolution error inside `getWebsiteByIdWithOwnerCached` and returning the website with `ownerId: null` so the optional enrichment doesn't block the auth path.</comment>
<file context>
@@ -45,10 +46,14 @@ function _resolveOwnerId(
message: "Failed to fetch workspace owner",
organizationId,
});
+ throw basketErrors.websiteLookupUnavailable();
}
</file context>
| Checkout **Enterprise** maps to **Scale** entitlements in-app (same column below). | ||
|
|
||
| | | Free | Hobby | Pro | Enterprise (Scale) | | ||
| | | Free | Hobby | Pro | Enterprise | |
There was a problem hiding this comment.
P2: The product-limits table header lost the (Scale) annotation and the definitions section dropped the Scale vs Enterprise cross-reference. If internal tier IDs and entitlement configs still use scale — which the codebase suggests via pricing/_pricing/estimator-scale.ts — this removal removes an important breadcrumb for keeping docs and code aligned. Future edits to either side can drift silently. Consider keeping a visible reminder (e.g., restoring Enterprise (Scale) in the header or a short footnote) so reviewers know the internal plan id is still scale.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/docs/public/pricing.md, line 35:
<comment>The product-limits table header lost the `(Scale)` annotation and the definitions section dropped the `Scale vs Enterprise` cross-reference. If internal tier IDs and entitlement configs still use `scale` — which the codebase suggests via `pricing/_pricing/estimator-scale.ts` — this removal removes an important breadcrumb for keeping docs and code aligned. Future edits to either side can drift silently. Consider keeping a visible reminder (e.g., restoring `Enterprise (Scale)` in the header or a short footnote) so reviewers know the internal plan id is still `scale`.</comment>
<file context>
@@ -32,45 +32,33 @@ Overage = events **above** the monthly included amount. Cumulative overage is ch
-Checkout **Enterprise** maps to **Scale** entitlements in-app (same column below).
-
-| | Free | Hobby | Pro | Enterprise (Scale) |
+| | Free | Hobby | Pro | Enterprise |
| --- | --- | --- | --- | --- |
| Funnels | 1 | 5 | 50 | Unlimited |
</file context>
| | | Free | Hobby | Pro | Enterprise | | |
| +| | Free | Hobby | Pro | Enterprise (Scale) | |
There was a problem hiding this comment.
0 issues found across 7 files (changes from recent commits).
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 51 unresolved issues from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/rpc/src/routers/insights.ts">
<violation number="1" location="packages/rpc/src/routers/insights.ts:337">
P2: The `.catch(() => false)` wrapper around `withWorkspace` in `getById` and `related` swallows every rejection, including database and infrastructure errors. After reviewing `resolveWorkspace` in `packages/rpc/src/procedures/with-workspace.ts`, it calls `getMemberRole` and `context.getBilling()` which can throw on DB/connectivity failures. When that happens the endpoint will return `{ success: true, insight: null }` (or an empty array) instead of a real error, masking outages and bypassing telemetry.
Preserve the anti-enumeration behavior for actual permission denials, but rethrow unexpected/infrastructure errors so they surface properly. A simple approach is to inspect the rejection and only swallow the expected `rpcError.forbidden` / `rpcError.unauthorized` kinds.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 46 unresolved issues from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 22 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/dashboard/lib/safe-callback.ts">
<violation number="1" location="apps/dashboard/lib/safe-callback.ts:1">
P1: `safeCallbackPath` can be bypassed via URL-parser-stripped whitespace, leading to an open redirect. A value like `/\t/attacker.example` passes the `startsWith("/")` and `!startsWith("//")` checks because the tab character sits between the slashes. When the browser or Next.js router parses the URL later, the tab is stripped (per the WHATWG URL Standard) and the path becomes `//attacker.example`, which is a protocol-relative external URL. Consider rejecting tab, LF, and CR characters explicitly before the prefix test, or parsing the string against a known base origin with `new URL()` and verifying the resulting pathname has not collapsed into a protocol-relative form.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| @@ -0,0 +1,14 @@ | |||
| export function safeCallbackPath( | |||
There was a problem hiding this comment.
P1: safeCallbackPath can be bypassed via URL-parser-stripped whitespace, leading to an open redirect. A value like /\t/attacker.example passes the startsWith("/") and !startsWith("//") checks because the tab character sits between the slashes. When the browser or Next.js router parses the URL later, the tab is stripped (per the WHATWG URL Standard) and the path becomes //attacker.example, which is a protocol-relative external URL. Consider rejecting tab, LF, and CR characters explicitly before the prefix test, or parsing the string against a known base origin with new URL() and verifying the resulting pathname has not collapsed into a protocol-relative form.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/lib/safe-callback.ts, line 1:
<comment>`safeCallbackPath` can be bypassed via URL-parser-stripped whitespace, leading to an open redirect. A value like `/\t/attacker.example` passes the `startsWith("/")` and `!startsWith("//")` checks because the tab character sits between the slashes. When the browser or Next.js router parses the URL later, the tab is stripped (per the WHATWG URL Standard) and the path becomes `//attacker.example`, which is a protocol-relative external URL. Consider rejecting tab, LF, and CR characters explicitly before the prefix test, or parsing the string against a known base origin with `new URL()` and verifying the resulting pathname has not collapsed into a protocol-relative form.</comment>
<file context>
@@ -0,0 +1,14 @@
+export function safeCallbackPath(
+ callback: string | null | undefined,
+ fallback = "/websites"
</file context>
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 22 unresolved issues from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
12 issues found across 91 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/insights/src/resolution.ts">
<violation number="1" location="apps/insights/src/resolution.ts:23">
P2: Active custom-event findings now bypass recovery tracking and become stale after the TTL despite an ongoing `custom_event:*` signal. Keep `custom_event_spike` in a transient family (and match its signals) so active findings remain open and disappeared signals recover.</violation>
</file>
<file name="apps/insights/src/detection.ts">
<violation number="1" location="apps/insights/src/detection.ts:651">
P2: Custom-event anomalies will no longer produce Findings: `custom_events_discovery` was removed from this detection pass and no replacement detector emits `custom_event:*` signals. Retain that family or move its logic into the new engine so event additions, volume shifts, and disappearances remain detectable.</violation>
</file>
<file name="packages/ai/src/ai/insights/product-context.ts">
<violation number="1" location="packages/ai/src/ai/insights/product-context.ts:131">
P3: Malformed or legacy funnel step data can make product-evidence collection throw at `step.name`, because this cast validates only the array, not its elements. Filter to non-null object steps before mapping.</violation>
</file>
<file name="apps/insights/src/delivery.ts">
<violation number="1" location="apps/insights/src/delivery.ts:440">
P2: Rate-limited Slack deliveries with `Retry-After` above five seconds retry too early and exhaust all three attempts. Preserve Slack’s requested delay rather than capping it so the durable effect can succeed after the limit window.</violation>
</file>
<file name="packages/ai/src/ai/insights/validate.ts">
<violation number="1" location="packages/ai/src/ai/insights/validate.ts:784">
P1: The deterministic decision validator is not connected to a production investigation flow, so this release will keep using existing behavior and never generate these validated insights. Wire `validateInvestigationDecision` into the investigation result path before returning/persisting a decision.</violation>
</file>
<file name="apps/insights/src/jobs.ts">
<violation number="1" location="apps/insights/src/jobs.ts:261">
P2: Concurrent delivery can run the same website generation twice because this predicate accepts an already `running` item, so both workers receive a row and proceed. Use an atomic claim/lease transition that only one worker can acquire before calling `generateWebsiteInsights`.</violation>
</file>
<file name="packages/ai/package.json">
<violation number="1" location="packages/ai/package.json:21">
P3: Export path `./insights/evidence-reader` maps to a source file that wasn't renamed to match. The file is still at `src/ai/tools/insights-agent-tools.ts` while every other export here follows the pattern where the path key matches the actual file location (`./insights/fetch-context` → `src/ai/insights/fetch-context.ts`, etc.). Someone searching for `evidence-reader` by filename or looking in `src/ai/insights/` won't find it. Consider renaming the source file to `evidence-reader.ts` and moving it to `src/ai/insights/` to match the export path and align with the convention.</violation>
</file>
<file name="package.json">
<violation number="1" location="package.json:67">
P2: `eval:insights` is missing `dotenv --` prefix, inconsistent with all other `eval*` scripts. This means `.env` variables won't be loaded before the script runs, so any dependencies requiring env vars (e.g., DB connections via `investigateWebsiteWithSources`) may fail or misbehave. Add `dotenv --` before `bun run` to match the project convention and ensure env variables are available.</violation>
</file>
<file name="apps/insights/src/generation.ts">
<violation number="1" location="apps/insights/src/generation.ts:756">
P1: Partial detector scans can still close an open finding when the selected signal returns `monitor` or `not_a_problem`. Gate `retiredSignalKey` on `analysis.detectionComplete` too, so incomplete scans remain non-destructive.</violation>
</file>
<file name="apps/insights/src/index.ts">
<violation number="1" location="apps/insights/src/index.ts:25">
P2: `readBooleanEnv` changes the default when `INSIGHTS_WORKER_ENABLED` is unset: old code defaulted to `true` (enabled), new code defaults to `false` (disabled). If any environment relies on the implicit default rather than setting this var explicitly, the worker will silently stop running after deploy. All documented configs (`.env.example`, selfhost, CI) do set it explicitly, so this is mainly a risk for unconfigured or ad-hoc deployments. Consider a separate `readBooleanEnv('...', true)` overload or verify the env var is set in every deployment that needs the worker.</violation>
</file>
<file name="apps/insights/src/funnel-detection.test.ts">
<violation number="1" location="apps/insights/src/funnel-detection.test.ts:615">
P2: The `aborts sibling workers when one definition fails fatally` test creates unhandled promise rejections. Workers that are blocked on `await blocked` resume after `release?.()` and throw `signal.reason` on promises that were part of the already-settled `Promise.all` in `mapWithConcurrency`. These throws become unhandled rejections that could cause the test to fail or produce flaky results depending on how `bun:test` collects unhandled rejections. Consider using the abort signal to short-circuit the `work` function instead, or guard the worker loop with a check that skips work on settled `Promise.all`.</violation>
</file>
<file name="packages/ai/src/query/builders/vitals.ts">
<violation number="1" location="packages/ai/src/query/builders/vitals.ts:142">
P2: Vitals overview percentiles can become inaccurate under traffic spikes because the deterministic sampler key can repeat across many rows. Using a higher-cardinality determinator (include identity/context columns) would keep deterministic output without the duplicate-key bias.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| return generatedInsightSchema.parse(insight); | ||
| } | ||
|
|
||
| export function validateInvestigationDecision( |
There was a problem hiding this comment.
P1: The deterministic decision validator is not connected to a production investigation flow, so this release will keep using existing behavior and never generate these validated insights. Wire validateInvestigationDecision into the investigation result path before returning/persisting a decision.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/ai/insights/validate.ts, line 784:
<comment>The deterministic decision validator is not connected to a production investigation flow, so this release will keep using existing behavior and never generate these validated insights. Wire `validateInvestigationDecision` into the investigation result path before returning/persisting a decision.</comment>
<file context>
@@ -348,257 +680,272 @@ function toGeneratedInsight(
-
-export function validateInvestigationSubmission(
- input: ValidateInvestigationInput
+export function validateInvestigationDecision(
+ input: unknown
): InvestigationValidationResult {
</file context>
| retiredSignalKey: retiredSignalKeyForOutcome({ | ||
| disposition: analysis.decision?.disposition, | ||
| hasInsight: analysis.insight !== null, | ||
| signalKey: analysis.signal?.signalKey, | ||
| }), |
There was a problem hiding this comment.
P1: Partial detector scans can still close an open finding when the selected signal returns monitor or not_a_problem. Gate retiredSignalKey on analysis.detectionComplete too, so incomplete scans remain non-destructive.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/insights/src/generation.ts, line 756:
<comment>Partial detector scans can still close an open finding when the selected signal returns `monitor` or `not_a_problem`. Gate `retiredSignalKey` on `analysis.detectionComplete` too, so incomplete scans remain non-destructive.</comment>
<file context>
@@ -694,152 +673,127 @@ export async function generateWebsiteInsights(
detectedSignals: analysis.detectedSignals,
- canRecover: analysis.status !== "no_data",
+ canRecover: analysis.status !== "no_data" && analysis.detectionComplete,
+ retiredSignalKey: retiredSignalKeyForOutcome({
+ disposition: analysis.decision?.disposition,
+ hasInsight: analysis.insight !== null,
</file context>
| retiredSignalKey: retiredSignalKeyForOutcome({ | |
| disposition: analysis.decision?.disposition, | |
| hasInsight: analysis.insight !== null, | |
| signalKey: analysis.signal?.signalKey, | |
| }), | |
| retiredSignalKey: analysis.detectionComplete | |
| ? retiredSignalKeyForOutcome({ | |
| disposition: analysis.decision?.disposition, | |
| hasInsight: analysis.insight !== null, | |
| signalKey: analysis.signal?.signalKey, | |
| }) | |
| : undefined, |
| @@ -16,26 +20,20 @@ type InsightFamily = | |||
| | "vitals" | |||
| | "traffic" | |||
| | "engagement" | |||
| | "conversion" | |||
| | "custom_event"; | |||
| | "conversion"; | |||
There was a problem hiding this comment.
P2: Active custom-event findings now bypass recovery tracking and become stale after the TTL despite an ongoing custom_event:* signal. Keep custom_event_spike in a transient family (and match its signals) so active findings remain open and disappeared signals recover.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/insights/src/resolution.ts, line 23:
<comment>Active custom-event findings now bypass recovery tracking and become stale after the TTL despite an ongoing `custom_event:*` signal. Keep `custom_event_spike` in a transient family (and match its signals) so active findings remain open and disappeared signals recover.</comment>
<file context>
@@ -18,26 +20,20 @@ type InsightFamily =
| "engagement"
- | "conversion"
- | "custom_event";
+ | "conversion";
const TRANSIENT_TYPE_FAMILY: Record<string, InsightFamily> = {
</file context>
| query("vitals_overview", previousFrom, previousTo), | ||
| query("custom_events_discovery", currentFrom, currentTo), | ||
| query("custom_events_discovery", previousFrom, previousTo), | ||
| const [summary, errors, revenue, vitals] = await Promise.all([ |
There was a problem hiding this comment.
P2: Custom-event anomalies will no longer produce Findings: custom_events_discovery was removed from this detection pass and no replacement detector emits custom_event:* signals. Retain that family or move its logic into the new engine so event additions, volume shifts, and disappearances remain detectable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/insights/src/detection.ts, line 651:
<comment>Custom-event anomalies will no longer produce Findings: `custom_events_discovery` was removed from this detection pass and no replacement detector emits `custom_event:*` signals. Retain that family or move its logic into the new engine so event additions, volume shifts, and disappearances remain detectable.</comment>
<file context>
@@ -482,33 +643,57 @@ async function detectWow(
- query("vitals_overview", previousFrom, previousTo),
- query("custom_events_discovery", currentFrom, currentTo),
- query("custom_events_discovery", previousFrom, previousTo),
+ const [summary, errors, revenue, vitals] = await Promise.all([
+ readDetectorFamily({
+ abortSignal,
</file context>
| ? seconds * 1000 | ||
| : SLACK_RATE_LIMIT_FALLBACK_MS; | ||
| return ( | ||
| Math.min(requested, SLACK_RATE_LIMIT_MAX_WAIT_MS) + |
There was a problem hiding this comment.
P2: Rate-limited Slack deliveries with Retry-After above five seconds retry too early and exhaust all three attempts. Preserve Slack’s requested delay rather than capping it so the durable effect can succeed after the limit window.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/insights/src/delivery.ts, line 440:
<comment>Rate-limited Slack deliveries with `Retry-After` above five seconds retry too early and exhaust all three attempts. Preserve Slack’s requested delay rather than capping it so the durable effect can succeed after the limit window.</comment>
<file context>
@@ -409,138 +424,208 @@ export function buildThreadBlocks(
+ ? seconds * 1000
+ : SLACK_RATE_LIMIT_FALLBACK_MS;
+ return (
+ Math.min(requested, SLACK_RATE_LIMIT_MAX_WAIT_MS) +
+ Math.floor(random() * 250)
+ );
</file context>
| @@ -21,7 +22,7 @@ const environment = | |||
| process.env.APP_ENV ?? | |||
| process.env.RAILWAY_ENVIRONMENT_NAME ?? | |||
| (process.env.NODE_ENV === "development" ? "development" : "production"); | |||
| const workerEnabled = process.env.INSIGHTS_WORKER_ENABLED !== "false"; | |||
| const workerEnabled = readBooleanEnv("INSIGHTS_WORKER_ENABLED"); | |||
There was a problem hiding this comment.
P2: readBooleanEnv changes the default when INSIGHTS_WORKER_ENABLED is unset: old code defaulted to true (enabled), new code defaults to false (disabled). If any environment relies on the implicit default rather than setting this var explicitly, the worker will silently stop running after deploy. All documented configs (.env.example, selfhost, CI) do set it explicitly, so this is mainly a risk for unconfigured or ad-hoc deployments. Consider a separate readBooleanEnv('...', true) overload or verify the env var is set in every deployment that needs the worker.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/insights/src/index.ts, line 25:
<comment>`readBooleanEnv` changes the default when `INSIGHTS_WORKER_ENABLED` is unset: old code defaulted to `true` (enabled), new code defaults to `false` (disabled). If any environment relies on the implicit default rather than setting this var explicitly, the worker will silently stop running after deploy. All documented configs (`.env.example`, selfhost, CI) do set it explicitly, so this is mainly a risk for unconfigured or ad-hoc deployments. Consider a separate `readBooleanEnv('...', true)` overload or verify the env var is set in every deployment that needs the worker.</comment>
<file context>
@@ -21,7 +22,7 @@ const environment =
process.env.RAILWAY_ENVIRONMENT_NAME ??
(process.env.NODE_ENV === "development" ? "development" : "production");
-const workerEnabled = process.env.INSIGHTS_WORKER_ENABLED !== "false";
+const workerEnabled = readBooleanEnv("INSIGHTS_WORKER_ENABLED");
const DRAIN_TIMEOUT_MS = 10_000;
</file context>
| release = resolve; | ||
| }); | ||
|
|
||
| const detection = detectFunnelGoalSignals( |
There was a problem hiding this comment.
P2: The aborts sibling workers when one definition fails fatally test creates unhandled promise rejections. Workers that are blocked on await blocked resume after release?.() and throw signal.reason on promises that were part of the already-settled Promise.all in mapWithConcurrency. These throws become unhandled rejections that could cause the test to fail or produce flaky results depending on how bun:test collects unhandled rejections. Consider using the abort signal to short-circuit the work function instead, or guard the worker loop with a check that skips work on settled Promise.all.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/insights/src/funnel-detection.test.ts, line 615:
<comment>The `aborts sibling workers when one definition fails fatally` test creates unhandled promise rejections. Workers that are blocked on `await blocked` resume after `release?.()` and throw `signal.reason` on promises that were part of the already-settled `Promise.all` in `mapWithConcurrency`. These throws become unhandled rejections that could cause the test to fail or produce flaky results depending on how `bun:test` collects unhandled rejections. Consider using the abort signal to short-circuit the `work` function instead, or guard the worker loop with a check that skips work on settled `Promise.all`.</comment>
<file context>
@@ -175,4 +292,375 @@ describe("detectFunnelGoalSignals", () => {
+ release = resolve;
+ });
+
+ const detection = detectFunnelGoalSignals(
+ PARAMS,
+ TODAY,
</file context>
| quantilesTDigest(0.50, 0.75, 0.90, 0.95, 0.99)(metric_value) as _q, | ||
| quantilesDeterministic(0.50, 0.75, 0.90, 0.95, 0.99)( | ||
| metric_value, | ||
| cityHash64(tuple(timestamp, metric_value)) |
There was a problem hiding this comment.
P2: Vitals overview percentiles can become inaccurate under traffic spikes because the deterministic sampler key can repeat across many rows. Using a higher-cardinality determinator (include identity/context columns) would keep deterministic output without the duplicate-key bias.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/query/builders/vitals.ts, line 142:
<comment>Vitals overview percentiles can become inaccurate under traffic spikes because the deterministic sampler key can repeat across many rows. Using a higher-cardinality determinator (include identity/context columns) would keep deterministic output without the duplicate-key bias.</comment>
<file context>
@@ -137,7 +137,10 @@ export const VitalsBuilders: Record<string, SimpleQueryConfig> = {
- quantilesTDigest(0.50, 0.75, 0.90, 0.95, 0.99)(metric_value) as _q,
+ quantilesDeterministic(0.50, 0.75, 0.90, 0.95, 0.99)(
+ metric_value,
+ cityHash64(tuple(timestamp, metric_value))
+ ) as _q,
avg(metric_value) as avg_value,
</file context>
| cityHash64(tuple(timestamp, metric_value)) | |
| cityHash64(tuple(timestamp, session_id, anonymous_id, path, metric_value)) |
| const definitionSteps = Array.isArray(funnel.steps) | ||
| ? (funnel.steps as Record<string, unknown>[]) | ||
| : []; |
There was a problem hiding this comment.
P3: Malformed or legacy funnel step data can make product-evidence collection throw at step.name, because this cast validates only the array, not its elements. Filter to non-null object steps before mapping.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/ai/insights/product-context.ts, line 131:
<comment>Malformed or legacy funnel step data can make product-evidence collection throw at `step.name`, because this cast validates only the array, not its elements. Filter to non-null object steps before mapping.</comment>
<file context>
@@ -85,20 +107,46 @@ async function getFunnelsSummary(
+ const analyticsSteps = Array.isArray(analytics.steps_analytics)
+ ? (analytics.steps_analytics as Record<string, unknown>[])
+ : [];
+ const definitionSteps = Array.isArray(funnel.steps)
+ ? (funnel.steps as Record<string, unknown>[])
+ : [];
</file context>
| const definitionSteps = Array.isArray(funnel.steps) | |
| ? (funnel.steps as Record<string, unknown>[]) | |
| : []; | |
| const definitionSteps = Array.isArray(funnel.steps) | |
| ? funnel.steps.filter( | |
| (step): step is Record<string, unknown> => | |
| typeof step === "object" && step !== null | |
| ) | |
| : []; |
| @@ -18,10 +18,8 @@ | |||
| "./agents/types": "./src/ai/agents/types.ts", | |||
| "./config/context": "./src/ai/config/context.ts", | |||
| "./config/models": "./src/ai/config/models.ts", | |||
| "./insights/dedupe": "./src/ai/insights/dedupe.ts", | |||
| "./insights/evidence-reader": "./src/ai/tools/insights-agent-tools.ts", | |||
There was a problem hiding this comment.
P3: Export path ./insights/evidence-reader maps to a source file that wasn't renamed to match. The file is still at src/ai/tools/insights-agent-tools.ts while every other export here follows the pattern where the path key matches the actual file location (./insights/fetch-context → src/ai/insights/fetch-context.ts, etc.). Someone searching for evidence-reader by filename or looking in src/ai/insights/ won't find it. Consider renaming the source file to evidence-reader.ts and moving it to src/ai/insights/ to match the export path and align with the convention.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/package.json, line 21:
<comment>Export path `./insights/evidence-reader` maps to a source file that wasn't renamed to match. The file is still at `src/ai/tools/insights-agent-tools.ts` while every other export here follows the pattern where the path key matches the actual file location (`./insights/fetch-context` → `src/ai/insights/fetch-context.ts`, etc.). Someone searching for `evidence-reader` by filename or looking in `src/ai/insights/` won't find it. Consider renaming the source file to `evidence-reader.ts` and moving it to `src/ai/insights/` to match the export path and align with the convention.</comment>
<file context>
@@ -18,6 +18,7 @@
"./agents/types": "./src/ai/agents/types.ts",
"./config/context": "./src/ai/config/context.ts",
"./config/models": "./src/ai/config/models.ts",
+ "./insights/evidence-reader": "./src/ai/tools/insights-agent-tools.ts",
"./insights/fetch-context": "./src/ai/insights/fetch-context.ts",
"./insights/validate": "./src/ai/insights/validate.ts",
</file context>
1 participant
What's in this release
Users page
profile_listRevenue attribution
invoice.paid/invoice.payment_succeededhandling: recurring subscription payments now attribute to sessions and identified users. Metadata is read from the invoice,subscription_details, andparent.subscription_details(covers Autumn and newer Stripe API versions where payment intents carry no metadata)invoice.paidas a required webhook eventFeedback pipeline
website_idfkey indexed@databuddy/serviceswith source-aware Slack alertssubmit_feedbackagent tool for dashboard chat and the Slack bot, with consent-aware prompting and a single source of truth for the prompt rulesfeedback-previewchat component with send and receipt modesSlack agent
agent_runevents never reaching Axiom when a deploy landed mid-answerslack_run_timed_outtelemetry instead of sitting on "Thinking..." until the next deploy kills them@databuddy/chartspackage renders the agent's chart components (line, area, bar, stacked bar, pie, donut) to dashboard-themed PNGs server-side (ECharts SSR + resvg, LT Superior fonts, dark/light design tokens). Slack answers upload up to 3 charts into the thread and fall back to the existing text tables if rendering or upload fails. Activating uploads requires adding thefiles:writescope to the Slack appAgent & AI
list_profile_traitstool: trait key/value distribution with profile counts, guiding agents totrait:<key>segmentation before queryingget_data, MCP tools, and raw SQL paths; telemetry gated on the exported sanitized constant; error messaging keyed on error type, not input shapeDashboard & misc
has_active_subscriptiontrait dropped (redundant withplan)Custom event page path (new)
pathand basket persists it tocustom_events.path. The column existed but was never populated (0 of 144,409 custom events over the prior 30 days had a path).Hash-route pageviews (new)
screen_viewevents now include the URL hash fragment inpathwhentrackHashChangesis enabled. PreviouslybuildPagePathused pathname only and dropped the hash. This also fixes a pre-existing failing tracker E2E test.Insights becomes Findings (new)
@databuddy/ai,rpc,redis,db,shared, andevals.Misc (new)