diff --git a/.agents/skills/databuddy-internal/SKILL.md b/.agents/skills/databuddy-internal/SKILL.md index 85aa6231e..9de7b684a 100644 --- a/.agents/skills/databuddy-internal/SKILL.md +++ b/.agents/skills/databuddy-internal/SKILL.md @@ -53,6 +53,7 @@ Keep additions **minimal**: one bullet, a new `rg` hint, or a routing note—eno - Replies beneath delivered Slack investigations must resolve the delivery and enter the existing durable reply/resume path; never route them through generic Slack chat or relevance scoring. - One-off insight previews must preserve the real signal entity and use customer-facing product output. Never hand-write Slack copy from eval metadata or expose evaluation and suppression mechanics. - Agent ClickHouse SQL must use the canonical analytics.events schema: `client_id`, `time`, `path`, `event_name`, and pageviews as `event_name = 'screen_view'`; never `website_id`, `created_at`, `page_path`, `event_type`, or `pageview`. +- Agent `get_data` filters select rows; do not expose SQL CTE `target` or `having` as generic event/error scopes. Query discovery supplies accepted selectors, and result row counts describe the query output rather than a complete population. - Slack agent expected stops such as exhausted Databunny credits should throw `DatabuddyAgentUserError` from `@databuddy/ai/agent/errors`; Slack surfaces those messages directly and reserves the generic reconnect copy for real infrastructure failures. - Slack Docker builds use `bun build --compile --bytecode`; keep `apps/slack/src/index.ts` bootstrapping inside an async `main()` instead of top-level `await`, which can fail during compile even when typecheck passes. - Insights Docker builds also use `bun build --compile --bytecode`; keep `apps/insights/src/index.ts` startup work inside async functions instead of top-level `await`. @@ -74,6 +75,7 @@ Keep additions **minimal**: one bullet, a new `rg` hint, or a routing note—eno - `packages/auth`: Better Auth setup, permissions, organization access - `packages/env`: shared URL, public, and boolean environment helpers - `packages/shared`: shared types, flags, analytics schemas, utilities +- Analytics query builders live in `packages/ai/src/query`; there is no standalone `packages/query` directory. Tests are excluded from root Biome checks, so format changed test blocks explicitly. - `packages/sdk`: published analytics SDK for React, Vue, and Node - `packages/tracker`: internal tracker script build and release package - `packages/encryption`, `packages/notifications`, `packages/cache`, `packages/redis`, `packages/services`, `packages/validation`, `packages/api-keys`: shared infra and domain packages diff --git a/packages/ai/src/ai/tools/discover-query-types.test.ts b/packages/ai/src/ai/tools/discover-query-types.test.ts new file mode 100644 index 000000000..fea637ece --- /dev/null +++ b/packages/ai/src/ai/tools/discover-query-types.test.ts @@ -0,0 +1,51 @@ +import { expect, test } from "bun:test"; +import { discoverQueryTypesTool } from "./discover-query-types"; + +test("discovery exposes custom error context and its effective ordering", async () => { + const result = await discoverQueryTypesTool.execute?.( + { search: "recent_errors" }, + { toolCallId: "discovery", messages: [] } + ); + expect(result).toMatchObject({ + types: [ + expect.objectContaining({ + name: "recent_errors", + defaultOrder: "timestamp DESC", + outputFields: expect.arrayContaining([ + { name: "timestamp", type: "datetime" }, + { name: "message", type: "string" }, + ]), + }), + ], + }); +}); + +test("discovery distinguishes an unordered aggregate from undocumented custom SQL", async () => { + const result = await discoverQueryTypesTool.execute?.( + { search: "session_metrics" }, + { toolCallId: "discovery", messages: [] } + ); + expect(result).toMatchObject({ + types: [ + expect.objectContaining({ + name: "session_metrics", + defaultOrder: null, + outputFields: expect.arrayContaining([ + { name: "avg_session_duration", type: "number", unit: "seconds" }, + ]), + }), + ], + }); + const unknown = await discoverQueryTypesTool.execute?.( + { search: "session_list" }, + { toolCallId: "discovery", messages: [] } + ); + expect(unknown).toMatchObject({ + types: [ + expect.objectContaining({ + defaultOrder: "Built-in ordering is undocumented; omit orderBy.", + outputFields: null, + }), + ], + }); +}); diff --git a/packages/ai/src/ai/tools/discover-query-types.ts b/packages/ai/src/ai/tools/discover-query-types.ts index a3064da55..8440d41ae 100644 --- a/packages/ai/src/ai/tools/discover-query-types.ts +++ b/packages/ai/src/ai/tools/discover-query-types.ts @@ -1,19 +1,23 @@ import { tool } from "ai"; import { z } from "zod"; import { QueryBuilders } from "../../query/builders"; +import { allowedFilterFields } from "../../query/simple-builder"; -interface DiscoveredType { - category: string; - description: string; - name: string; - tags: string[]; -} - -function listAllTypes(): DiscoveredType[] { +function listAllTypes() { return Object.entries(QueryBuilders).map(([name, config]) => ({ + allowedFilters: allowedFilterFields(config), + allowedFilterOperators: config.allowedFilterOperators, name, category: config.meta?.category ?? "Uncategorized", + defaultOrder: config.customSql + ? config.meta?.default_order === undefined + ? "Built-in ordering is undocumented; omit orderBy." + : config.meta.default_order + : (config.orderBy ?? null), description: config.meta?.description ?? "", + outputFields: config.meta?.output_fields ?? config.fields ?? null, + requiredFilters: config.requiredFilters ?? [], + requiredAnyFilter: config.requiredAnyFilter ?? [], tags: config.meta?.tags ?? [], })); } @@ -22,7 +26,8 @@ const ALL_TYPES = listAllTypes(); const CATEGORIES = [...new Set(ALL_TYPES.map((t) => t.category))].sort(); export const discoverQueryTypesTool = tool({ - description: `List the analytics query builders available to get_data, filtered by category and/or keyword. Call this when you don't know which builder fits the user's ask (especially for breakdowns by a dimension you haven't used before — try category="Performance" or search="device"). Returns name, category, description, and tags. Cheap to call (no I/O); the result lets you pick the right type before calling get_data.`, + description: + "List the analytics query builders available to get_data, filtered by category and/or keyword. Call this when you need the right builder or its input contract. Returns allowed filters and operators, required selectors, output fields, and default order alongside the description. Null outputFields means undocumented, not an empty result schema. Custom SQL may have undocumented built-in ordering; omit orderBy to retain it. Cheap to call (no I/O).", inputSchema: z.object({ category: z .enum([CATEGORIES[0] ?? "Summary", ...CATEGORIES.slice(1)] as [ diff --git a/packages/ai/src/ai/tools/get-data.test.ts b/packages/ai/src/ai/tools/get-data.test.ts new file mode 100644 index 000000000..ef8be18ab --- /dev/null +++ b/packages/ai/src/ai/tools/get-data.test.ts @@ -0,0 +1,110 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { asSchema } from "ai"; +import { SimpleQueryBuilder } from "../../query/simple-builder"; +import { discoverQueryTypesTool } from "./discover-query-types"; +import { getDataTool } from "./get-data"; + +const options = { + toolCallId: "query-contract-test", + messages: [], + experimental_context: { + currentDateTime: "2026-09-05T00:00:00Z", + websiteId: "site-test", + websiteDomain: "example.com", + timezone: "UTC", + }, +}; + +afterEach(() => vi.restoreAllMocks()); + +describe("analytics tool contract", () => { + it.each([ + { target: "event" }, + { having: false }, + ])("rejects unsupported filter scope instead of silently stripping it: %o", async (scope) => { + const schema = asSchema(getDataTool.inputSchema); + if (!schema.validate) throw new Error("Missing tool schema validator"); + const result = await schema.validate({ + queries: [ + { + type: "custom_events_by_path", + filters: [ + { + field: "event_name", + op: "eq", + value: "activation_completed", + ...scope, + }, + ], + }, + ], + }); + expect(result.success).toBe(false); + }); + + it("exposes the exact continuation selector contract before the model queries", async () => { + if (!discoverQueryTypesTool.execute) + throw new Error("Missing discovery tool"); + const result = await discoverQueryTypesTool.execute( + { search: "error_route_continuation_comparison" }, + options + ); + expect(result).toMatchObject({ + types: [ + { + name: "error_route_continuation_comparison", + requiredAnyFilter: ["message", "path"], + allowedFilterOperators: { message: ["eq"], path: ["eq"] }, + }, + ], + }); + }); + + it("returns the measured scope and distinguishes a truncated result from its query row count", async () => { + const execute = vi + .spyOn(SimpleQueryBuilder.prototype, "execute") + .mockImplementation(function () { + const compiled = this.compile(); + expect(compiled.params).toMatchObject({ f0: "activation_completed" }); + expect(compiled.sql).toContain("event_name = {f0:String}"); + return Promise.resolve( + Array.from({ length: 25 }, (_, index) => ({ + name: `/step-${index}`, + total_events: 1, + })) + ); + }); + if (!getDataTool.execute) throw new Error("Missing data tool"); + const result = await getDataTool.execute( + { + queries: [ + { + type: "custom_events_by_path", + from: "2026-08-29", + to: "2026-09-04", + filters: [ + { field: "event_name", op: "eq", value: "activation_completed" }, + ], + }, + ], + }, + options + ); + expect(result).toMatchObject({ + results: { + custom_events_by_path: { + websiteId: "site-test", + from: "2026-08-29", + to: "2026-09-04", + filters: [ + { field: "event_name", op: "eq", value: "activation_completed" }, + ], + returnedRows: 20, + rowCount: 25, + truncated: true, + }, + }, + }); + expect(execute).toHaveBeenCalledOnce(); + }); +}); diff --git a/packages/ai/src/ai/tools/get-data.ts b/packages/ai/src/ai/tools/get-data.ts index 8f97d8747..39754fc7b 100644 --- a/packages/ai/src/ai/tools/get-data.ts +++ b/packages/ai/src/ai/tools/get-data.ts @@ -11,7 +11,11 @@ import { } from "../../query"; import { shiftDate, todayInTimeZone } from "../../query/date-utils"; import type { QueryRequest } from "../../query/types"; -import { getAppContext, resolveToolWebsite, toolDateRangeError } from "./utils"; +import { + getAppContext, + resolveToolWebsite, + toolDateRangeError, +} from "./utils/context"; type QueryType = Extract; const QUERY_TYPES = Object.keys(QueryBuilders) as [QueryType, ...QueryType[]]; @@ -30,7 +34,7 @@ const queryItemSchema = z.object({ timeUnit: z.enum(["minute", "hour", "day", "week", "month"]).optional(), filters: z .array( - z.object({ + z.strictObject({ field: z .string() .describe( @@ -50,13 +54,16 @@ const queryItemSchema = z.object({ z.number(), z.array(z.union([z.string(), z.number()])), ]), - target: z.string().optional(), - having: z.boolean().optional(), }) ) .optional(), groupBy: z.array(z.string()).optional(), - orderBy: z.string().optional(), + orderBy: z + .string() + .optional() + .describe( + "Omit to use the builder's default order. Otherwise use an output column followed by ASC or DESC, e.g. 'errors DESC'; never use names such as count_desc. discover_query_types lists output fields and default order." + ), limit: z.number().min(1).max(1000).optional(), timezone: z.string().optional(), }); @@ -66,9 +73,12 @@ type QueryItem = z.infer; interface QueryItemResult { data: unknown[]; error?: string; + filters?: QueryItem["filters"]; + from?: string; returnedRows?: number; rowCount: number; summary?: string; + to?: string; truncated?: boolean; type: string; websiteId?: string; @@ -145,7 +155,7 @@ function resolveDates( export const getDataTool = tool({ description: - "Run analytics query builders for explicit data questions. Batch 1-10 queries per call. Use preset (last_7d/last_30d/...) or from+to dates. Each query may target a specific website via websiteId; omit to use the workspace default. When truncated is true, data contains only returnedRows examples from rowCount query rows; never aggregate or generalize that sample.", + "Run analytics query builders for explicit data questions. Batch 1-10 queries per call. Use preset (last_7d/last_30d/...) or from+to dates. Each query may target a specific website via websiteId; omit to use the workspace default. Filters select rows: never supply target or having. discover_query_types lists allowed and required filters. Results include at most 20 rows; rowCount is the number of query rows, not the whole population. Query limits may exclude more rows even when truncated is false. Never infer absence, totals, or completeness from a ranked list; query the exact subject or use an aggregate builder.", inputSchema: z.object({ queries: z .array(queryItemSchema) @@ -196,7 +206,7 @@ export const getDataTool = tool({ from, to, timeUnit: item.timeUnit, - filters: item.filters as QueryRequest["filters"], + filters: item.filters, groupBy: item.groupBy, orderBy: item.orderBy, limit: item.limit, @@ -214,6 +224,9 @@ export const getDataTool = tool({ return { type: item.type, websiteId, + filters: item.filters ?? [], + from, + to, summary: buildResultSummary( item.type, from, diff --git a/packages/ai/src/query/builders/errors.ts b/packages/ai/src/query/builders/errors.ts index 55f7f17cb..a89877c47 100644 --- a/packages/ai/src/query/builders/errors.ts +++ b/packages/ai/src/query/builders/errors.ts @@ -6,6 +6,26 @@ import type { SimpleQueryConfig } from "../types"; export const ErrorsBuilders: Record = { recent_errors: { meta: { + default_order: "timestamp DESC", + output_fields: [ + { name: "message", type: "string" }, + { name: "stack", type: "string" }, + { name: "path", type: "string" }, + { name: "anonymous_id", type: "string" }, + { name: "session_id", type: "string" }, + { name: "timestamp", type: "datetime" }, + { name: "filename", type: "string" }, + { name: "lineno", type: "number" }, + { name: "colno", type: "number" }, + { name: "error_type", type: "string" }, + { name: "browser_name", type: "string" }, + { name: "browser_version", type: "string" }, + { name: "os_name", type: "string" }, + { name: "os_version", type: "string" }, + { name: "device_type", type: "string" }, + { name: "country", type: "string" }, + { name: "region", type: "string" }, + ], description: "Recent JS errors with full context: message, stack (capped at 1500 chars), path, error_type, browser, OS, device, country. For aggregates use error_summary / errors_by_type / errors_by_page.", category: "Errors", diff --git a/packages/ai/src/query/builders/sessions.ts b/packages/ai/src/query/builders/sessions.ts index c16318277..debf38719 100644 --- a/packages/ai/src/query/builders/sessions.ts +++ b/packages/ai/src/query/builders/sessions.ts @@ -11,6 +11,13 @@ function inclusiveEndDate(endDate: string): string { export const SessionsBuilders: Record = { session_metrics: { meta: { + default_order: null, + output_fields: [ + { name: "total_sessions", type: "number", unit: "sessions" }, + { name: "avg_session_duration", type: "number", unit: "seconds" }, + { name: "bounce_rate", type: "number", unit: "percent" }, + { name: "total_events", type: "number", unit: "events" }, + ], description: "Aggregate session statistics including total sessions, avg duration, and pages per session.", category: "Sessions", diff --git a/packages/ai/src/query/simple-builder.test.ts b/packages/ai/src/query/simple-builder.test.ts index 40abd85e5..6a205af71 100644 --- a/packages/ai/src/query/simple-builder.test.ts +++ b/packages/ai/src/query/simple-builder.test.ts @@ -320,17 +320,42 @@ describe("SimpleQueryBuilder.compile", () => { expect(params.f0).toBe("US"); }); - it("skips target-scoped filters from the outer WHERE clause", () => { + it("rejects a filter targeting a missing CTE instead of returning broader data", () => { const filters: Filter[] = [ { field: "country", op: "eq", value: "US" }, { field: "path", op: "eq", value: "/checkout", target: "my_cte" }, ]; - const { sql } = compile({}, { filters }); + expect(() => compile({}, { filters })).toThrow( + "Filter target 'my_cte' is not permitted" + ); + }); - const whereClause = whereClauseOf(sql); - expect(whereClause).toContain("country = {f0:String}"); - expect(whereClause).not.toMatch(/\bpath\b/); + it("applies a configured CTE selector inside that CTE", () => { + const { sql, params } = compile( + { with: [{ name: "selected", table: "analytics.events", fields: ["country"] }], from: "selected", groupBy: ["country"] }, + { filters: [{ field: "country", op: "eq", value: "US", target: "selected" }] } + ); + expect(sql).toContain("country = {f"); + expect(sql.indexOf("country = {f")).toBeLessThan(sql.lastIndexOf("FROM selected")); + expect(Object.values(params)).toContain("US"); + }); + + it.each(["custom_events_by_path", "error_frequency", "errors_by_page"])( + "rejects an invented target in %s before compiling unfiltered SQL", + (type) => { + const config = QueryBuilders[type]; + if (!config) throw new Error("Missing builder"); + expect(() => compileBuilder(type, config, { + filters: [{ field: type === "custom_events_by_path" ? "event_name" : "message", op: "eq", value: "synthetic-event", target: "event" }], + })).toThrow("Filter target 'event' is not permitted"); + } + ); + + it("does not silently discard a HAVING selector in custom SQL", () => { + expect(() => compileBuilder("custom_events_by_path", QueryBuilders.custom_events_by_path, { + filters: [{field: "total_events", op: "eq", value: 10, having: true}], + })).toThrow("Having filters are not supported"); }); it("allows a configured required filter when present", () => { diff --git a/packages/ai/src/query/simple-builder.ts b/packages/ai/src/query/simple-builder.ts index c54e61b02..ce6917a79 100644 --- a/packages/ai/src/query/simple-builder.ts +++ b/packages/ai/src/query/simple-builder.ts @@ -670,6 +670,23 @@ export class SimpleQueryBuilder { } compile(): CompiledQuery { + for (const filter of this.request.filters ?? []) { + if ( + filter.target && + (this.config.customSql || + filter.having || + !this.config.with?.some((cte) => cte.name === filter.target)) + ) { + throw new Error( + `Filter target '${filter.target}' is not permitted for ${this.request.type}. Omit target to filter the selected rows; target is only supported for a configured CTE.` + ); + } + if (filter.having && this.config.customSql) { + throw new Error( + `Having filters are not supported for ${this.request.type}. Use a row filter instead.` + ); + } + } this.validateRequiredFilters(); if (this.config.customSql) { diff --git a/packages/ai/src/query/trait-filters.test.ts b/packages/ai/src/query/trait-filters.test.ts index b8c485d37..81c488df3 100644 --- a/packages/ai/src/query/trait-filters.test.ts +++ b/packages/ai/src/query/trait-filters.test.ts @@ -90,6 +90,16 @@ describe("resolveRequestTraitFilters", () => { expect(mockResolveTraitSegment).not.toHaveBeenCalled(); }); + it.each([{ target: "event" }, { having: true }])( + "rejects scoped trait selectors before resolving a different cohort: %o", + async (scope) => { + await expect(resolveRequestTraitFilters(makeRequest({ + filters: [{field: "trait:plan", op: "eq", value: "pro", ...scope}], + }))).rejects.toThrow("Trait filters must select rows"); + expect(mockResolveTraitSegment).not.toHaveBeenCalled(); + } + ); + }); describe("invalidFilterFieldError", () => { @@ -149,6 +159,9 @@ describe("publicQueryErrorMessage", () => { expect( publicQueryErrorMessage("Missing required filter: 'session_id'.") ).toBe("Missing required filter: 'session_id'."); + expect(publicQueryErrorMessage("Filter target 'event' is not permitted for error_frequency.")).toBe("Filter target 'event' is not permitted for error_frequency."); + expect(publicQueryErrorMessage("Having filters are not supported for custom_events_by_path.")).toBe("Having filters are not supported for custom_events_by_path."); + expect(publicQueryErrorMessage("error_route_continuation_comparison requires one scalar message or path equality filter")).toBe("error_route_continuation_comparison requires one scalar message or path equality filter"); }); it("hides raw backend and compiler errors", () => { diff --git a/packages/ai/src/query/trait-filters.ts b/packages/ai/src/query/trait-filters.ts index 79cddb785..a0901a974 100644 --- a/packages/ai/src/query/trait-filters.ts +++ b/packages/ai/src/query/trait-filters.ts @@ -13,11 +13,14 @@ export const SANITIZED_QUERY_ERROR = "Query failed"; const PUBLIC_QUERY_ERROR_PATTERNS = [ /^Unknown query type:/, /^Filter on field '[^']+' is not permitted/, + /^Filter target '[^']+' is not permitted/, + /^Having filters are not supported for [a-z_]+\./, /^Operator '[^']+' is not permitted for filter '[^']+'\./, /^Grouping by '[^']+' is not permitted/, /^Ordering by '[^']+' is not permitted/, /^Missing required filters?:/, /^error_customer_impact requires exactly one scalar message or path equality filter$/, + /^error_route_continuation_comparison requires one scalar message or path equality filter$/, /^Trait filters /, /^Trait segment exceeds /, /^[a-z_]+ filter is required for [a-z_]+ query$/, @@ -86,6 +89,11 @@ export async function resolveRequestTraitFilters( } const filters = request.filters ?? []; const traitFilters = filters.filter((f) => isTraitFilterField(f.field)); + if (traitFilters.some((filter) => filter.target || filter.having)) { + throw new TraitFilterError( + "Trait filters must select rows, without target or having." + ); + } const segment = await resolveTraitSegment( request.projectId, traitFilters as TraitFilter[] diff --git a/packages/ai/src/query/types.ts b/packages/ai/src/query/types.ts index 90eb52467..22be5e607 100644 --- a/packages/ai/src/query/types.ts +++ b/packages/ai/src/query/types.ts @@ -35,6 +35,8 @@ export type VisualizationType = export interface QueryBuilderMeta { category?: string; + /** Effective ordering inside custom SQL; null means no ordering. */ + default_order?: string | null; default_visualization?: VisualizationType; deprecated?: boolean; description: string;