First version of filters#1250
Conversation
🧹 Preview RemovedThe preview deployment for this PR has been cleaned up. |
There was a problem hiding this comment.
3 issues found across 49 files
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/frontend/src/components/mcp-app/story-app-view.tsx">
<violation number="1" location="apps/frontend/src/components/mcp-app/story-app-view.tsx:86">
P2: MCP story views will never show or apply story filters: this `StoryTabbedContent` invocation omits `filterApi`, which disables `useStoryFilters` entirely. Provide an MCP-capable filtering API (or keep this view on the non-filtered renderer) so filter blocks are not silently ignored.</violation>
</file>
<file name="apps/backend/src/agents/tools/execute-sql.ts">
<violation number="1" location="apps/backend/src/agents/tools/execute-sql.ts:127">
P1: Editing a query creates a second persisted `execute_sql` result with the same ID, so later chart/data lookups can read the stale duplicate because query-ID lookups have no ordering. Persist the replacement only once, or make query-ID retrieval/update select a deterministic canonical/latest part.</violation>
</file>
<file name="apps/shared/src/sql-template.ts">
<violation number="1" location="apps/shared/src/sql-template.ts:120">
P2: Search values containing `%`, `_`, or `\` do not reliably match literal text on SQL engines that require an explicit `ESCAPE` clause: `escapeLikePattern` adds backslashes, but the rendered SQL fragment cannot establish that those backslashes are escapes. The search template contract could use a dialect-aware escaping strategy or require/render a matching `ESCAPE` clause.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
| execute: executeQuery, | ||
| execute: async (input, context) => { | ||
| if (input.query_id) { | ||
| return updateExistingQuery({ ...input, query_id: input.query_id }, context); |
There was a problem hiding this comment.
P1: Editing a query creates a second persisted execute_sql result with the same ID, so later chart/data lookups can read the stale duplicate because query-ID lookups have no ordering. Persist the replacement only once, or make query-ID retrieval/update select a deterministic canonical/latest part.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/backend/src/agents/tools/execute-sql.ts, line 127:
<comment>Editing a query creates a second persisted `execute_sql` result with the same ID, so later chart/data lookups can read the stale duplicate because query-ID lookups have no ordering. Persist the replacement only once, or make query-ID retrieval/update select a deterministic canonical/latest part.</comment>
<file context>
@@ -75,11 +82,51 @@ async function executeAppDbQuery(sqlQuery: string, context: ToolContext): Promis
- execute: executeQuery,
+ execute: async (input, context) => {
+ if (input.query_id) {
+ return updateExistingQuery({ ...input, query_id: input.query_id }, context);
+ }
+ return executeQuery(input, context);
</file context>
There was a problem hiding this comment.
1 issue found across 47 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/frontend/src/components/side-panel/story-chart-query.tsx">
<violation number="1" location="apps/frontend/src/components/side-panel/story-chart-query.tsx:14">
P3: After changing a filter, this can label SQL rendered from the new selection as “Executed query” while the chart still shows debounced or fallback prior data. Feed the SQL view the same debounced/effective selections used for chart execution, or label it as a pending preview.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| chatId: source.api.kind === 'owned' ? source.api.chatId : '', | ||
| storySlug: source.api.kind === 'owned' ? source.api.storySlug : '', | ||
| queryId, | ||
| selections: source.selections, |
There was a problem hiding this comment.
P3: After changing a filter, this can label SQL rendered from the new selection as “Executed query” while the chart still shows debounced or fallback prior data. Feed the SQL view the same debounced/effective selections used for chart execution, or label it as a pending preview.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/frontend/src/components/side-panel/story-chart-query.tsx, line 14:
<comment>After changing a filter, this can label SQL rendered from the new selection as “Executed query” while the chart still shows debounced or fallback prior data. Feed the SQL view the same debounced/effective selections used for chart execution, or label it as a pending preview.</comment>
<file context>
@@ -0,0 +1,75 @@
+ chatId: source.api.kind === 'owned' ? source.api.chatId : '',
+ storySlug: source.api.kind === 'owned' ? source.api.storySlug : '',
+ queryId,
+ selections: source.selections,
+ }),
+ enabled: source.api.kind === 'owned',
</file context>
There was a problem hiding this comment.
1 issue found across 56 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/backend/src/services/story-filters.ts">
<violation number="1" location="apps/backend/src/services/story-filters.ts:76">
P2: When story filters are disabled, both `getQuerySql` endpoints still expose template/rendered filter SQL because neither caller applies `assertStoryFiltersEnabled()`. Gate these endpoints like the adjacent filter endpoints so the beta flag consistently disables the feature.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| return queryData; | ||
| } | ||
|
|
||
| export async function getStoryQuerySql( |
There was a problem hiding this comment.
P2: When story filters are disabled, both getQuerySql endpoints still expose template/rendered filter SQL because neither caller applies assertStoryFiltersEnabled(). Gate these endpoints like the adjacent filter endpoints so the beta flag consistently disables the feature.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/backend/src/services/story-filters.ts, line 76:
<comment>When story filters are disabled, both `getQuerySql` endpoints still expose template/rendered filter SQL because neither caller applies `assertStoryFiltersEnabled()`. Gate these endpoints like the adjacent filter endpoints so the beta flag consistently disables the feature.</comment>
<file context>
@@ -59,25 +59,48 @@ export async function getFilteredStoryQueryData(
return queryData;
}
+export async function getStoryQuerySql(
+ chatId: string,
+ storySlug: string,
</file context>
🚀 Preview Deployment
Preview will be automatically removed when this PR is closed. |
Uh oh!
There was an error while loading. Please reload this page.