diff --git a/apps/backend/src/services/live-story.ts b/apps/backend/src/services/live-story.ts index e82bb24fb..0351391be 100644 --- a/apps/backend/src/services/live-story.ts +++ b/apps/backend/src/services/live-story.ts @@ -296,11 +296,11 @@ function preservesStoryStructure(originalCode: string, candidateCode: string): b } function extractStructureTokens(code: string): string[] { - const structureRegex = new RegExp( - `]*>|<\\/grid>||`, + const tokenRegex = new RegExp( + String.raw`|<\/grid>||`, 'g', ); - return code.match(structureRegex) ?? []; + return code.match(tokenRegex) ?? []; } function extractHeadingTokens(code: string): string[] { diff --git a/apps/backend/src/utils/story-summary.ts b/apps/backend/src/utils/story-summary.ts index 2976286c0..b479d4f91 100644 --- a/apps/backend/src/utils/story-summary.ts +++ b/apps/backend/src/utils/story-summary.ts @@ -1,4 +1,4 @@ -import { TAG_ATTRS } from '@nao/shared/story-segments'; +import { storyBlockRegex } from '@nao/shared/story-segments'; import { parseStoryTabs } from '@nao/shared/story-tabs'; import type { StorySummary, SummarySegment } from '@nao/shared/types'; @@ -20,10 +20,7 @@ export function extractStorySummary(code: string): StorySummary { function extractSegments(code: string): SummarySegment[] { const segments: SummarySegment[] = []; - const blockRegex = new RegExp( - `]*)>([\\s\\S]*?)<\\/grid>||`, - 'g', - ); + const blockRegex = storyBlockRegex(); let match; let lastIndex = 0; diff --git a/apps/frontend/src/components/side-panel/story-editor.tsx b/apps/frontend/src/components/side-panel/story-editor.tsx index 6f62db011..bc15652eb 100644 --- a/apps/frontend/src/components/side-panel/story-editor.tsx +++ b/apps/frontend/src/components/side-panel/story-editor.tsx @@ -1,10 +1,11 @@ import { + chartTagRegex, getGridClass, parseChartAttributes, parseChartBlock, parseTableBlock, splitCodeIntoSegments, - TAG_ATTRS, + tableTagRegex, } from '@nao/shared/story-segments'; import { Extension, mergeAttributes, Node } from '@tiptap/core'; import { DragHandle } from '@tiptap/extension-drag-handle-react'; @@ -48,11 +49,11 @@ export function preprocessForEditor(code: string): string { return `
\n\n`; }); - result = result.replace(new RegExp(``, 'g'), (match) => { + result = result.replace(chartTagRegex('g'), (match) => { return `
\n\n`; }); - result = result.replace(new RegExp(``, 'g'), (match) => { + result = result.replace(tableTagRegex('g'), (match) => { return `
\n\n`; }); @@ -67,7 +68,7 @@ function ChartBlockView({ node, updateAttributes }: ReactNodeViewProps) { const rawTag = node.attrs.rawTag as string; const chart = useMemo(() => { - const attrMatch = rawTag.match(new RegExp(``)); + const attrMatch = rawTag.match(chartTagRegex()); if (!attrMatch) { return null; } @@ -151,7 +152,7 @@ function TableBlockView({ node }: ReactNodeViewProps) { const rawTag = node.attrs.rawTag as string; const table = useMemo(() => { - const attrMatch = rawTag.match(new RegExp(``)); + const attrMatch = rawTag.match(tableTagRegex()); if (!attrMatch) { return null; } diff --git a/apps/shared/src/story-segments.ts b/apps/shared/src/story-segments.ts index 9bae4d942..0fbeaf2ae 100644 --- a/apps/shared/src/story-segments.ts +++ b/apps/shared/src/story-segments.ts @@ -1,14 +1,6 @@ import { buildStoryTableBlock } from './chart-block'; import { type ColumnConditionalFormats, sanitizeConditionalFormats } from './conditional-formatting'; -/** - * Matches a tag's attribute list while treating single/double-quoted values as - * opaque, so `>` and `/` inside a quoted attribute (e.g. a threshold rule's - * `">="` operator inside `formatting='{...}'`) do not prematurely terminate the - * tag. Kept as a shared constant so every block-tag regex stays consistent. - */ -export const TAG_ATTRS = `(?:[^>"']|"(?:[^"\\\\]|\\\\.)*"|'(?:[^'\\\\]|\\\\.)*')*?`; - export interface ParsedChartBlock { queryId: string; chartType: string; @@ -37,6 +29,23 @@ export type Segment = | { type: 'table'; table: ParsedTableBlock } | { type: 'grid'; cols: number; children: Segment[] }; +export const TAG_ATTRS = String.raw`(?:[^>"']|"(?:[^"\\]|\\.)*"|'(?:[^'\\]|\\.)*')*?`; + +export function chartTagRegex(flags = ''): RegExp { + return new RegExp(String.raw``, flags); +} + +export function tableTagRegex(flags = ''): RegExp { + return new RegExp(String.raw``, flags); +} + +export function storyBlockRegex(): RegExp { + return new RegExp( + String.raw`([\s\S]*?)<\/grid>||`, + 'g', + ); +} + function unescapeAttributeValue(value: string): string { return value.replace(/\\(["'\\])/g, '$1'); } @@ -157,15 +166,27 @@ export function getGridClass(cols: number): string { return GRID_CLASSES[Math.min(cols, 4)] ?? GRID_CLASSES[2]; } -function tryParseSeriesJson(value: string): ParsedChartBlock['series'] | null { +export function parseSeriesJsonArray(value: string): unknown[] | null { + const parsed = tryJsonParse(value) ?? tryJsonParse(escapeStrayBackslashes(value)); + return Array.isArray(parsed) ? parsed : null; +} + +function tryJsonParse(value: string): unknown { try { - const parsed = JSON.parse(value); - return Array.isArray(parsed) ? parsed : null; + return JSON.parse(value); } catch { return null; } } +function escapeStrayBackslashes(value: string): string { + return value.replace(/\\(?!(?:["\\/bfnrt]|u[0-9a-fA-F]{4}))/g, '\\\\'); +} + +function tryParseSeriesJson(value: string): ParsedChartBlock['series'] | null { + return parseSeriesJsonArray(value) as ParsedChartBlock['series'] | null; +} + function parseOptionalNumberAttr(value: string | undefined): number | undefined { return value !== undefined && value !== '' && Number.isFinite(Number(value)) ? Number(value) : undefined; } @@ -197,10 +218,7 @@ function extractSeriesFromRawAttrs(attrString: string): ParsedChartBlock['series export function splitCodeIntoSegments(code: string): Segment[] { const segments: Segment[] = []; - const blockRegex = new RegExp( - `]*)>([\\s\\S]*?)<\\/grid>||`, - 'g', - ); + const blockRegex = storyBlockRegex(); let match; let lastIndex = 0; diff --git a/apps/shared/src/story-validation.ts b/apps/shared/src/story-validation.ts index 8dcad6a31..aadd20d73 100644 --- a/apps/shared/src/story-validation.ts +++ b/apps/shared/src/story-validation.ts @@ -1,4 +1,4 @@ -import { parseChartAttributes, TAG_ATTRS } from './story-segments'; +import { parseChartAttributes, parseSeriesJsonArray, TAG_ATTRS } from './story-segments'; export interface StoryValidationError { message: string; @@ -117,7 +117,7 @@ function validateTabsBlocks(code: string): StoryValidationError[] { function validateChartBlocks(code: string): StoryValidationError[] { const errors: StoryValidationError[] = []; - const chartRegex = new RegExp(``, 'g'); + const chartRegex = new RegExp(String.raw``, 'g'); let match: RegExpExecArray | null; while ((match = chartRegex.exec(code)) !== null) { @@ -162,7 +162,7 @@ function validateChartBlocks(code: string): StoryValidationError[] { }); } - const seriesError = validateChartSeries(attrs, attrString ?? '', position, fullMatch.length); + const seriesError = validateChartSeries(attrs, position, fullMatch.length); if (seriesError) { errors.push(seriesError); } @@ -173,7 +173,6 @@ function validateChartBlocks(code: string): StoryValidationError[] { function validateChartSeries( attrs: Record, - attrString: string, position: { line: number; column: number }, length: number, ): StoryValidationError | null { @@ -190,13 +189,9 @@ function validateChartSeries( return null; } - const rawSeries = extractRawSeriesBracket(attrString); - const jsonSource = rawSeries ?? attrs.series; + const parsed = parseSeriesJsonArray(attrs.series); - let parsed: unknown; - try { - parsed = JSON.parse(jsonSource); - } catch { + if (parsed === null) { return { message: 'Chart `series` attribute must be a valid JSON array.', line: position.line, @@ -205,7 +200,7 @@ function validateChartSeries( }; } - if (!Array.isArray(parsed) || parsed.length === 0) { + if (parsed.length === 0) { return { message: 'Chart `series` attribute must be a non-empty JSON array.', line: position.line, @@ -228,32 +223,9 @@ function validateChartSeries( return null; } -function extractRawSeriesBracket(attrString: string): string | null { - const seriesIdx = attrString.search(/\bseries\s*=/); - if (seriesIdx === -1) { - return null; - } - const bracketStart = attrString.indexOf('[', seriesIdx); - if (bracketStart === -1) { - return null; - } - let depth = 0; - for (let i = bracketStart; i < attrString.length; i++) { - if (attrString[i] === '[') { - depth++; - } else if (attrString[i] === ']') { - depth--; - if (depth === 0) { - return attrString.slice(bracketStart, i + 1); - } - } - } - return null; -} - function validateTableBlocks(code: string): StoryValidationError[] { const errors: StoryValidationError[] = []; - const tableRegex = new RegExp(``, 'g'); + const tableRegex = new RegExp(String.raw``, 'g'); let match: RegExpExecArray | null; while ((match = tableRegex.exec(code)) !== null) { diff --git a/apps/shared/tests/story-segments.test.ts b/apps/shared/tests/story-segments.test.ts new file mode 100644 index 000000000..fb0f5ca5d --- /dev/null +++ b/apps/shared/tests/story-segments.test.ts @@ -0,0 +1,123 @@ +import { describe, expect, it } from 'vitest'; + +import { buildStoryChartBlock } from '../src/chart-block'; +import { splitCodeIntoSegments } from '../src/story-segments'; + +function chartOf(code: string) { + const segment = splitCodeIntoSegments(code).find((s) => s.type === 'chart'); + return segment?.type === 'chart' ? segment.chart : null; +} + +function seriesOf(code: string) { + return chartOf(code)?.series ?? null; +} + +describe('splitCodeIntoSegments chart series', () => { + it('round-trips a label containing a backslash built via buildStoryChartBlock', () => { + const code = buildStoryChartBlock({ + query_id: 'q1', + chart_type: 'bar', + x_axis_key: 'month', + series: [{ data_key: 'rev', color: 'var(--chart-1)', label: 'Disc\\Rebate' }], + title: 'Revenue', + }); + + expect(seriesOf(code)).toEqual([{ data_key: 'rev', color: 'var(--chart-1)', label: 'Disc\\Rebate' }]); + }); + + it('recovers a hand-authored series where a backslash was not JSON-escaped', () => { + const code = + ''; + + expect(seriesOf(code)).toEqual([{ data_key: 'rev', label: 'Disc\\Rebate' }]); + }); + + it('recovers a hand-authored series with a malformed unicode escape', () => { + const code = + ''; + + expect(seriesOf(code)).toEqual([{ data_key: 'rev', label: 'A\\uZZZZ' }]); + }); + + it('recovers a hand-authored series with a truncated unicode escape', () => { + const code = + ''; + + expect(seriesOf(code)).toEqual([{ data_key: 'rev', label: 'A\\u00' }]); + }); + + it('preserves a well-formed unicode escape', () => { + const code = + ''; + + expect(seriesOf(code)).toEqual([{ data_key: 'rev', label: 'AA' }]); + }); + + it('round-trips a label containing a double quote', () => { + const code = buildStoryChartBlock({ + query_id: 'q1', + chart_type: 'bar', + x_axis_key: 'month', + series: [{ data_key: 'rev', color: 'var(--chart-1)', label: 'a "quoted" label' }], + title: 'Revenue', + }); + + expect(seriesOf(code)).toEqual([{ data_key: 'rev', color: 'var(--chart-1)', label: 'a "quoted" label' }]); + }); +}); + +describe('splitCodeIntoSegments slash-in-attribute handling', () => { + it('parses a chart whose title contains a slash', () => { + const code = + ''; + + const chart = chartOf(code); + expect(chart?.title).toBe('13/07 update'); + expect(chart?.series).toEqual([ + { data_key: 'number_of_orders', color: '#2563eb', label: 'Number of orders', is_total: false }, + ]); + }); + + it('parses a series label containing a slash', () => { + const code = buildStoryChartBlock({ + query_id: 'q1', + chart_type: 'bar', + x_axis_key: 'month', + series: [{ data_key: 'rev', color: 'var(--chart-1)', label: 'rev/cost' }], + title: 'Ratio', + }); + + expect(seriesOf(code)).toEqual([{ data_key: 'rev', color: 'var(--chart-1)', label: 'rev/cost' }]); + }); + + it('handles a slash in the title together with a backslash in a label', () => { + const code = buildStoryChartBlock({ + query_id: 'q1', + chart_type: 'bar', + x_axis_key: 'month', + series: [{ data_key: 'rev', color: 'var(--chart-1)', label: 'Disc\\Rebate' }], + title: '13/07 update', + }); + + const chart = chartOf(code); + expect(chart?.title).toBe('13/07 update'); + expect(chart?.series).toEqual([{ data_key: 'rev', color: 'var(--chart-1)', label: 'Disc\\Rebate' }]); + }); + + it('parses a chart whose title contains a greater-than sign', () => { + const code = + ''; + + const chart = chartOf(code); + expect(chart?.title).toBe('rev > 100'); + expect(chart?.series).toEqual([{ data_key: 'rev' }]); + }); + + it('still parses a plain self-closing chart tag', () => { + const code = ''; + + const chart = chartOf(code); + expect(chart?.title).toBe('Revenue'); + expect(chart?.series).toEqual([{ data_key: 'rev', color: 'var(--chart-1)', label: undefined }]); + }); +}); diff --git a/apps/shared/tests/story-validation.test.ts b/apps/shared/tests/story-validation.test.ts index eb28c2105..42b16f287 100644 --- a/apps/shared/tests/story-validation.test.ts +++ b/apps/shared/tests/story-validation.test.ts @@ -73,6 +73,24 @@ describe('validateStoryCode', () => { expect(errors.some((e) => e.message.includes('non-empty JSON array'))).toBe(true); }); + it('accepts a series label containing a backslash', () => { + const code = + ''; + expect(validateStoryCode(code)).toEqual([]); + }); + + it('accepts a series label containing a bracket', () => { + const code = + ''; + expect(validateStoryCode(code)).toEqual([]); + }); + + it('accepts a self-closing chart whose title contains a slash', () => { + const code = + ''; + expect(validateStoryCode(code)).toEqual([]); + }); + it('flags series entries without data_key', () => { const code = '';