mirror of https://github.com/buster-so/buster.git
Add in more tests and catch multiple report metric files
This commit is contained in:
dal
parent
22d3b8573c
commit
eeeec56bae
|
|
@ -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<metric metricId="e774e254-7ccd-4f03-b28d-6d91b5331b8a"/>\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: '<metric metricId="metric-1"/>',
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
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: '<metric metricId="metric-2"/>',
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
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: '<metric metricId="metric-legacy"/>',
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
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: '<metric metricId="metric-new"/>',
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
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 = [
|
||||
|
|
|
|||
|
|
@ -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<string>();
|
||||
const dashboardsInReports = new Set<string>();
|
||||
|
||||
// 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),
|
||||
|
|
|
|||
Loading…
Reference in New Issue