fix(ai): reject ignored analytics filter scopes - #723
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: 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 |
![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 8 files
Architecture diagram
sequenceDiagram
participant Model as AI Model
participant Discover as discover_query_types Tool
participant GetData as get_data Tool
participant Compiler as SimpleQueryBuilder
participant TraitFilter as Trait Filters Resolver
participant DB as Analytics Database
Note over Model,DB: Query Discovery & Validation Flow
Model->>Discover: discover_query_types(search)
Discover->>Discover: Enumerate QueryBuilders
Discover-->>Model: Builder contracts (allowedFilters, operators, outputFields, defaultOrder, requiredFilters)
Model->>GetData: get_data({queries: [{type, filters, orderBy}]})
GetData->>GetData: Resolve website context & dates
GetData->>GetData: Validate query schema (strict filter objects)
alt Filter has target or having
GetData-->>Model: Error - reject invalid filter scope
else Filter is trait field
GetData->>TraitFilter: resolveRequestTraitFilters()
TraitFilter->>TraitFilter: Check for target/having on trait filters
alt Trait filter has target/having
TraitFilter-->>GetData: Throw TraitFilterError
GetData-->>Model: Error - trait filters must select rows
else Valid trait filter
TraitFilter->>DB: Resolve trait segment
DB-->>TraitFilter: Segment cohort
TraitFilter-->>GetData: Row-level trait filters
end
else Standard row filter
GetData->>Compiler: compile() with filters
Compiler->>Compiler: Validate filter targets against CTEs
alt Invalid target or having on custom SQL
Compiler-->>GetData: Throw "Filter target not permitted" / "Having filters not supported"
GetData-->>Model: Public error (sanitized)
else Valid filter scope
Compiler->>Compiler: Build SQL with WHERE clause
Compiler-->>GetData: Compiled query + params
GetData->>DB: Execute SQL
DB-->>GetData: Query rows (up to 1000)
GetData->>GetData: Truncate to 20 rows, track rowCount
GetData-->>Model: Result with filters, dates, returnedRows, rowCount, truncated flag
end
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
|
421d5ec to
80a5214
Compare
izadoesdev
left a comment
izadoesdev
left a comment
There was a problem hiding this comment.
Reviewed query compilation, trait resolution, tool validation, result scope and discovery metadata. Unsupported filter targets and HAVING cannot silently broaden an agent cohort. Custom SQL discovery now distinguishes undocumented ordering/output columns from a known unordered aggregate. Recent errors and session metrics have verified explicit metadata; unannotated custom builders remain explicitly unknown.
The existing batch-executor signature test caught a metadata column-order mismatch during review; it was corrected before pushing. Focused tests, the full 27-task pre-push suite, root lint and all 33 type/build tasks pass. Rebased on current staging after #724 without conflicts.
📄 Knowledge review✏️ Suggested updates1 page suggestion needs review.
📝 Query Filtering by User Traits@@ -303,6 +303,18 @@
***
+### Invalid filter scopes
+
+Trait filters can only be used to select rows in the query. Passing a trait filter with `target` or `having` scope options throws:
+
+```
+Trait filters must select rows, without target or having.
+```
+
+The `target` scope is reserved for filtering within a specific CTE (common table expression), and `having` scopes filter aggregated results — neither is supported for trait filtering. This validation happens in the query compiler before trait segment resolution [[6]](https://github.com/databuddy-analytics/Databuddy/pull/723).
+
+***
+
### Summary of error messages
| Condition | Error message |
@@ -311,6 +323,7 @@
| Builder does not allow `profile_id` | `"Trait filters are not supported for {name}"` |
| Segment exceeds 10,000 profiles | `"Trait segment exceeds 10000 profiles — narrow the filter."` |
| Unsupported operator | `"Trait filters do not support the {op} operator."` |
+| Trait filter uses `target` or `having` scope | `"Trait filters must select rows, without target or having."` |
| `resolveTraitSegment` throws any other error | The error message is forwarded as-is |
## Implementation Details
|
Greptile SummaryThis PR tightens model-facing analytics filters, rejects unsupported compiler scopes, adds builder discovery metadata, and returns query-scope and row-count information with
Confidence Score: 4/5The PR is not yet safe to merge because discovery can direct callers to apply filters that an uptime builder silently ignores. The scope rejection and result metadata are consistent with their intended contracts, but the new discovery contract exposes global filters for Files Needing Attention: packages/ai/src/ai/tools/discover-query-types.ts Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
D[discover_query_types] --> C[Builder contract]
C --> M[Model constructs get_data request]
M --> V[Validate filter field and scope]
V --> Q[Compile named query]
Q --> S[Custom or standard SQL]
S --> R[Rows plus scope and truncation metadata]
C -. overstated global filters .-> U[uptime_time_series]
U -. filterConditions omitted .-> R
Reviews (1): Last reviewed commit: "fix(ai): disclose incomplete query disco..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| function listAllTypes(): DiscoveredType[] { | ||
| function listAllTypes() { | ||
| return Object.entries(QueryBuilders).map(([name, config]) => ({ | ||
| allowedFilters: allowedFilterFields(config), |
There was a problem hiding this comment.
Discovery advertises every global analytics filter for every builder, but uptime_time_series does not consume filterConditions. A model following this contract can submit an advertised filter such as country; validation accepts it, but the custom SQL applies only site and timestamp predicates. The result therefore contains unfiltered uptime data even though the requested scope appears to have been applied.
Knowledge Base Used: Analytics query engine
1 participant
Fresh investigations supplied event/error filter targets that the query compiler silently discarded, so tool results described a narrower cohort than the SQL actually selected. Remove unsupported scope options from the model-facing filter schema, reject invalid CTE/HAVING scopes in the compiler, and expose each builder's accepted selectors, outputs, and default order. Results now carry their filter/date scope and explicitly distinguish query rows from a complete population.
Validation: 348 focused query/tool tests, 100 batch-executor tests, and 229 insights tests passed. Repository lint and all 33 typecheck/build tasks passed. Tests use synthetic data. Scope is query correctness and discoverability; no storage migrations, production cleanup, or model-policy changes. No dependencies or known ownership overlaps.
Summary by cubic
Rejects unsupported analytics filter scopes so tool results always match the SQL cohort, and exposes builder contracts so models can construct valid queries.
havingselectors are rejected with clear public errors instead of being silently discarded, including for trait filters.discover_query_typesnow returns each builder's allowed and required filters, operators, output fields, and default order, and flags builders with undocumented ordering or output schema so models omitorderByinstead of guessing.get_dataresults include the applied filter/date scope and distinguish returned rows from the query row count.Written for commit 80a5214. Summary will update on new commits.