diff --git a/packages/ai/src/tools/communication-tools/done-tool/helpers/done-tool-file-selection-report-filtering.test.ts b/packages/ai/src/tools/communication-tools/done-tool/helpers/done-tool-file-selection-report-filtering.test.ts index c4727577a..121ac4f6b 100644 --- a/packages/ai/src/tools/communication-tools/done-tool/helpers/done-tool-file-selection-report-filtering.test.ts +++ b/packages/ai/src/tools/communication-tools/done-tool/helpers/done-tool-file-selection-report-filtering.test.ts @@ -507,6 +507,276 @@ describe('done-tool-file-selection - report filtering functionality', () => { }); }); + describe('BUG-1885: Metrics used in reports appearing in response messages', () => { + it('should filter out metric when using new single-file report structure (user reported bug)', () => { + // This test reproduces the exact bug scenario reported by the user + const messages: ModelMessage[] = [ + // Create a metric first + { + role: 'tool', + content: [ + { + type: 'tool-result', + toolName: 'createMetrics', + toolCallId: 'metric-call', + output: { + type: 'json', + value: { + files: [ + { + id: 'e774e254-7ccd-4f03-b28d-6d91b5331b8a', + name: 'Top 10 Customers by Lifetime Value', + version_number: 1, + }, + ], + }, + }, + }, + ], + }, + // Create a report using new single-file structure that references the metric + { + role: 'assistant', + content: [ + { + type: 'tool-call', + toolName: 'createReports', + toolCallId: 'report-call', + input: { + name: 'Top Customers Analysis', + content: `# Top Customers Analysis\n\n\n\nAnalysis of top customers...`, + }, + }, + ], + }, + { + role: 'tool', + content: [ + { + type: 'tool-result', + toolName: 'createReports', + toolCallId: 'report-call', + output: { + type: 'json', + value: { + file: { + id: '1a3bf75a-a4b9-415e-b121-d26d1873a45e', + name: 'Top Customers Analysis', + version_number: 1, + }, + }, + }, + }, + ], + }, + ]; + + const files = extractFilesFromToolCalls(messages); + + // The metric should be filtered out because it's referenced in the report + // The report should also be filtered out (all reports are filtered) + expect(files).toHaveLength(0); + expect(files.find((f) => f.id === 'e774e254-7ccd-4f03-b28d-6d91b5331b8a')).toBeUndefined(); + expect(files.find((f) => f.id === '1a3bf75a-a4b9-415e-b121-d26d1873a45e')).toBeUndefined(); + }); + + it('should filter metrics referenced in ANY report, not just the last one', () => { + const messages: ModelMessage[] = [ + // Create three metrics + { + role: 'tool', + content: [ + { + type: 'tool-result', + toolName: 'createMetrics', + toolCallId: 'metrics-call', + output: { + type: 'json', + value: { + files: [ + { id: 'metric-1', name: 'Metric 1', version_number: 1 }, + { id: 'metric-2', name: 'Metric 2', version_number: 1 }, + { id: 'metric-3', name: 'Metric 3', version_number: 1 }, + ], + }, + }, + }, + ], + }, + // First report referencing metric-1 + { + role: 'assistant', + content: [ + { + type: 'tool-call', + toolName: 'createReports', + toolCallId: 'report-1-call', + input: { + name: 'Report 1', + content: '', + }, + }, + ], + }, + { + role: 'tool', + content: [ + { + type: 'tool-result', + toolName: 'createReports', + toolCallId: 'report-1-call', + output: { + type: 'json', + value: { + file: { id: 'report-1', name: 'Report 1', version_number: 1 }, + }, + }, + }, + ], + }, + // Second report referencing metric-2 + { + role: 'assistant', + content: [ + { + type: 'tool-call', + toolName: 'createReports', + toolCallId: 'report-2-call', + input: { + name: 'Report 2', + content: '', + }, + }, + ], + }, + { + role: 'tool', + content: [ + { + type: 'tool-result', + toolName: 'createReports', + toolCallId: 'report-2-call', + output: { + type: 'json', + value: { + file: { id: 'report-2', name: 'Report 2', version_number: 1 }, + }, + }, + }, + ], + }, + ]; + + const files = extractFilesFromToolCalls(messages); + + // Only metric-3 should remain (not referenced in any report) + expect(files).toHaveLength(1); + expect(files[0]?.id).toBe('metric-3'); + expect(files.find((f) => f.id === 'metric-1')).toBeUndefined(); // In first report + expect(files.find((f) => f.id === 'metric-2')).toBeUndefined(); // In second report + }); + + it('should handle both legacy array and new single-file report structures', () => { + const messages: ModelMessage[] = [ + // Create metrics + { + role: 'tool', + content: [ + { + type: 'tool-result', + toolName: 'createMetrics', + toolCallId: 'metrics-mixed', + output: { + type: 'json', + value: { + files: [ + { id: 'metric-legacy', name: 'Legacy Metric', version_number: 1 }, + { id: 'metric-new', name: 'New Metric', version_number: 1 }, + { id: 'metric-standalone', name: 'Standalone Metric', version_number: 1 }, + ], + }, + }, + }, + ], + }, + // Report using legacy array structure + { + role: 'assistant', + content: [ + { + type: 'tool-call', + toolName: 'createReports', + toolCallId: 'legacy-report', + input: { + files: [ + { + name: 'Legacy Report', + content: '', + }, + ], + }, + }, + ], + }, + { + role: 'tool', + content: [ + { + type: 'tool-result', + toolName: 'createReports', + toolCallId: 'legacy-report', + output: { + type: 'json', + value: { + files: [{ id: 'report-legacy', name: 'Legacy Report', version_number: 1 }], + }, + }, + }, + ], + }, + // Report using new single-file structure + { + role: 'assistant', + content: [ + { + type: 'tool-call', + toolName: 'createReports', + toolCallId: 'new-report', + input: { + name: 'New Report', + content: '', + }, + }, + ], + }, + { + role: 'tool', + content: [ + { + type: 'tool-result', + toolName: 'createReports', + toolCallId: 'new-report', + output: { + type: 'json', + value: { + file: { id: 'report-new', name: 'New Report', version_number: 1 }, + }, + }, + }, + ], + }, + ]; + + const files = extractFilesFromToolCalls(messages); + + // Only the standalone metric should remain + expect(files).toHaveLength(1); + expect(files[0]?.id).toBe('metric-standalone'); + expect(files.find((f) => f.id === 'metric-legacy')).toBeUndefined(); + expect(files.find((f) => f.id === 'metric-new')).toBeUndefined(); + }); + }); + describe('createFileResponseMessages', () => { it('should create response messages for selected files', () => { const files = [ diff --git a/packages/ai/src/tools/communication-tools/done-tool/helpers/done-tool-file-selection.ts b/packages/ai/src/tools/communication-tools/done-tool/helpers/done-tool-file-selection.ts index e30d06d4d..0fb258d4e 100644 --- a/packages/ai/src/tools/communication-tools/done-tool/helpers/done-tool-file-selection.ts +++ b/packages/ai/src/tools/communication-tools/done-tool/helpers/done-tool-file-selection.ts @@ -348,18 +348,16 @@ export function extractFilesFromToolCalls(messages: ModelMessage[]): ExtractedFi let filteredFiles = filterOutDashboardMetrics(deduplicatedFiles); // Filter out metrics and dashboards that are referenced in reports - filteredFiles = filterOutReportContainedFiles(filteredFiles, lastReportInfo); + // Pass all files so the function can check ALL reports, not just the last one + filteredFiles = filterOutReportContainedFiles(filteredFiles); - // Filter out reports that don't have metrics (they won't be in responseMessages anyway) - // Keep reports that have metrics since they'll be in the responseMessages + // Filter out ALL reports (they are already in responseMessages) filteredFiles = filteredFiles.filter((file) => { if (file.fileType !== 'report_file') { return true; // Keep all non-report files that passed previous filters } - // For reports, only keep them if they contain metrics - // Reports with metrics will be in responseMessages already, so we filter them out here - // to avoid duplication + // Filter out ALL reports - they are handled separately and already in responseMessages return false; }); @@ -733,49 +731,51 @@ function filterOutDashboardMetrics(files: ExtractedFile[]): ExtractedFile[] { /** * Filter out metrics and dashboards that are referenced in reports - * Any metric that appears in a report should not be selected + * Any metric that appears in ANY report should not be selected */ -function filterOutReportContainedFiles( - files: ExtractedFile[], - lastReportInfo?: ReportInfo -): ExtractedFile[] { - if (!lastReportInfo || !lastReportInfo.content) { - // No report content to check against +function filterOutReportContainedFiles(files: ExtractedFile[]): ExtractedFile[] { + // Extract all report files from the files array + const reportFiles = files.filter((f) => f.fileType === 'report_file' && f.content); + + if (reportFiles.length === 0) { + // No reports to check against console.info('[done-tool-file-selection] No report content to filter against'); return files; } - const reportContent = lastReportInfo.content; - - // Extract metric IDs from the last report content + // Extract metric IDs and dashboard IDs from ALL reports const metricsInReports = new Set(); const dashboardsInReports = new Set(); - // Extract metric IDs from report content using regex pattern - // Match any alphanumeric ID with hyphens (UUIDs, simple IDs like "metric-1", etc.) - const metricIdPattern = /metricId[="']+([a-zA-Z0-9-]+)["']/gi; - const matches = reportContent.matchAll(metricIdPattern); + // Process each report's content + for (const report of reportFiles) { + if (!report.content) continue; - for (const match of matches) { - if (match[1]) { - metricsInReports.add(match[1]); + // Extract metric IDs from report content using regex pattern + // Match any alphanumeric ID with hyphens (UUIDs, simple IDs like "metric-1", etc.) + const metricIdPattern = /metricId[="']+([a-zA-Z0-9-]+)["']/gi; + const matches = report.content.matchAll(metricIdPattern); + + for (const match of matches) { + if (match[1]) { + metricsInReports.add(match[1]); + } + } + + // Also check for dashboard IDs in reports + const dashboardIdPattern = /dashboardId[="']+([a-zA-Z0-9-]+)["']/gi; + const dashboardMatches = report.content.matchAll(dashboardIdPattern); + + for (const match of dashboardMatches) { + if (match[1]) { + dashboardsInReports.add(match[1]); + } } } - // Also check for dashboard IDs in reports - const dashboardIdPattern = /dashboardId[="']+([a-zA-Z0-9-]+)["']/gi; - const dashboardMatches = reportContent.matchAll(dashboardIdPattern); - - for (const match of dashboardMatches) { - if (match[1]) { - dashboardsInReports.add(match[1]); - } - } - - console.info('[done-tool-file-selection] Checking for files absorbed by last report', { - reportId: lastReportInfo.id, - reportOperation: lastReportInfo.operation, - contentLength: reportContent.length, + console.info('[done-tool-file-selection] Checking for files absorbed by reports', { + reportCount: reportFiles.length, + reportIds: reportFiles.map((r) => r.id), metricsInReports: metricsInReports.size, dashboardsInReports: dashboardsInReports.size, metricIds: Array.from(metricsInReports),