Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 47 additions & 5 deletions apps/backend/src/services/story-template-validation.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,59 @@
import { findUnreferencedStoryFilters, validateSqlFilterTemplate } from '@nao/shared/sql-template';
import { getStoryFiltersFromCode } from '@nao/shared/story-segments';
import { extractQueryIds, getStoryFiltersFromCode } from '@nao/shared/story-segments';
import { QueryIdSchema } from '@nao/shared/tools';

import { env } from '../env';
import * as storyQueries from '../queries/story.queries';
import * as executeSqlQueries from '../queries/execute-sql.queries';

type SqlQueryMap = Record<string, { sqlQuery: string; databaseId?: string }>;

export async function getStoryTemplateWarnings(chatId: string, code: string): Promise<string[]> {
if (!env.BETA_STORY_FILTERS_ENABLED) {
return [];
const warnings: string[] = [];
const { wellFormedIds, malformedWarnings } = partitionReferencedQueryIds(code);
warnings.push(...malformedWarnings);

const sqlQueries =
wellFormedIds.size > 0 ? await executeSqlQueries.getLatestSqlQueriesByIds(chatId, wellFormedIds) : {};

warnings.push(...getMissingQueryWarnings(wellFormedIds, sqlQueries));

if (env.BETA_STORY_FILTERS_ENABLED) {
warnings.push(...getFilterWarnings(code, sqlQueries));
}

return warnings;
}

function partitionReferencedQueryIds(code: string): { wellFormedIds: Set<string>; malformedWarnings: string[] } {
const wellFormedIds = new Set<string>();
const malformedWarnings: string[] = [];
for (const queryId of extractQueryIds(code)) {
if (QueryIdSchema.safeParse(queryId).success) {
wellFormedIds.add(queryId);
} else {
malformedWarnings.push(
`Story references query_id "${queryId}", which is not a valid query id. Use the exact id returned in an execute_sql tool output (the "id" field, which looks like "query_..."); a chart/table/map block with an invalid query_id renders empty.`,
);
}
}
return { wellFormedIds, malformedWarnings };
}

function getMissingQueryWarnings(wellFormedIds: Set<string>, sqlQueries: SqlQueryMap): string[] {
const warnings: string[] = [];
for (const queryId of wellFormedIds) {
if (!sqlQueries[queryId]) {
warnings.push(
`Story references query_id "${queryId}", which was not produced by any execute_sql call in this chat. Run execute_sql first and use the exact id returned in its output (the "id" field); a chart/table/map block with an unknown query_id renders empty.`,
);
}
}
return warnings;
}

function getFilterWarnings(code: string, sqlQueries: SqlQueryMap): string[] {
const filters = getStoryFiltersFromCode(code);
const knownFilterIds = filters.map((filter) => filter.id);
const sqlQueries = await storyQueries.getSqlQueriesFromCode(chatId, code);
const warnings: string[] = [];

for (const duplicateId of findDuplicateFilterIds(knownFilterIds)) {
Expand Down
12 changes: 8 additions & 4 deletions apps/shared/src/story-segments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -683,10 +683,14 @@ function extractSeriesFromRawAttrs(attrString: string): ParsedChartBlock['series

export function extractQueryIds(code: string): Set<string> {
const ids = new Set<string>();
const regex = /<(?:chart|table|map)\s+[^>]*?\bquery_id\s*=\s*"([^"]+)"/g;
let match: RegExpExecArray | null;
while ((match = regex.exec(code)) !== null) {
ids.add(match[1]);
for (const tagRegex of [chartTagRegex('g'), tableTagRegex('g'), mapTagRegex('g')]) {
let match: RegExpExecArray | null;
while ((match = tagRegex.exec(code)) !== null) {
const { query_id } = parseChartAttributes(match[1]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The rewritten extractQueryIds now routes every tag through parseChartAttributes, whose attribute regex requires name= with no surrounding whitespace and a quoted value. The previous regex accepted query_id\s*=\s*"..." (optional whitespace around = and before the quote) and also matched query ids in tags that never close with >. For references written as <chart query_id = "q_1" ...> or an unclosed <chart query_id="q_1", the old code still recorded q_1 (and would warn on it), while the new code silently drops it, so those references bypass the new validator. Generated tags use query_id="..." so it is an edge case, but it is a coverage regression in the very validator this PR adds.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/shared/src/story-segments.ts, line 689:

<comment>The rewritten `extractQueryIds` now routes every tag through `parseChartAttributes`, whose attribute regex requires `name=` with no surrounding whitespace and a quoted value. The previous regex accepted `query_id\s*=\s*"..."` (optional whitespace around `=` and before the quote) and also matched query ids in tags that never close with `>`. For references written as `<chart query_id = "q_1" ...>` or an unclosed `<chart query_id="q_1"`, the old code still recorded `q_1` (and would warn on it), while the new code silently drops it, so those references bypass the new validator. Generated tags use `query_id="..."` so it is an edge case, but it is a coverage regression in the very validator this PR adds.</comment>

<file context>
@@ -683,10 +683,14 @@ function extractSeriesFromRawAttrs(attrString: string): ParsedChartBlock['series
+	for (const tagRegex of [chartTagRegex('g'), tableTagRegex('g'), mapTagRegex('g')]) {
+		let match: RegExpExecArray | null;
+		while ((match = tagRegex.exec(code)) !== null) {
+			const { query_id } = parseChartAttributes(match[1]);
+			if (query_id) {
+				ids.add(query_id);
</file context>

if (query_id) {
ids.add(query_id);
}
}
}
return ids;
}
Expand Down
1 change: 1 addition & 0 deletions apps/shared/src/tools/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export * as list from './list';
export * as loadSkill from './load-skill';
export * as mcpCall from './mcp-call';
export * as mcpConnect from './mcp-connect';
export { QueryIdSchema } from './query-id';
export * as readFile from './read';
export * as readQueryResult from './read-query-result';
export * as searchFiles from './search';
Expand Down
Loading