From a54b3b0ba5394e0087d250aeb5db653be3275ac4 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Wed, 11 Nov 2026 00:11:28 +0000 Subject: [PATCH] fix(service): Count only filtered skill findings in outcome totals The dashboard summary and outcome summary summed runs.finding_count for any run that included the filtered skill, so a skill filter reported findings from every other skill in those runs. Sum the matching skill executions' finding counts instead, so totals agree with the findings list. Co-Authored-By: Jeffrey Hung --- .../warden-service/src/history/store.test.ts | 18 ++++++++++++ packages/warden-service/src/history/store.ts | 29 ++++++++++++++----- .../integration/postgres.integration.test.ts | 18 ++++++++++++ 3 files changed, 57 insertions(+), 8 deletions(-) diff --git a/packages/warden-service/src/history/store.test.ts b/packages/warden-service/src/history/store.test.ts index e30f3416..40ae12b9 100644 --- a/packages/warden-service/src/history/store.test.ts +++ b/packages/warden-service/src/history/store.test.ts @@ -123,6 +123,24 @@ describe('history store', () => { } }); + it('counts only matching skill findings when outcomes filter by skill', async () => { + const statements: { sql: string; values: readonly unknown[] }[] = []; + const database = databaseFor((sql, values) => { + statements.push({ sql, values }); + return { rows: [], rowCount: 0 }; + }); + + await summarizeOutcomes(database, context, {}); + await summarizeOutcomes(database, context, { skill: 'integrity-review' }); + + const [unfiltered, filtered] = statements.filter((statement) => statement.sql.includes('COUNT(*)')); + expect(unfiltered?.sql).toContain('SUM("runs"."finding_count")'); + expect(filtered?.sql).not.toContain('SUM("runs"."finding_count")'); + expect(filtered?.sql).toContain('SUM((select SUM("finding_count") from "skill_executions" "filtered_se"'); + expect(filtered?.sql).toContain('"filtered_se"."run_id" = "runs"."id"'); + expect(filtered?.values.filter((value) => value === 'integrity-review')).toHaveLength(2); + }); + it('lists findings with literal search and investigation filters', async () => { let captured: { sql: string; values: readonly unknown[] } | undefined; const database = databaseFor((sql, values) => { diff --git a/packages/warden-service/src/history/store.ts b/packages/warden-service/src/history/store.ts index 55926eaa..f0be7933 100644 --- a/packages/warden-service/src/history/store.ts +++ b/packages/warden-service/src/history/store.ts @@ -216,21 +216,25 @@ function historyWhere( } conditions.push(exists(usageQuery.where(and(...executionConditions)))); } else if (filters.skill || filters.errorCode) { - const executionConditions: SQL[] = [ - eq(filteredSkillExecutions.tenantId, runs.tenantId), - eq(filteredSkillExecutions.runId, runs.id), - ]; - if (filters.skill) executionConditions.push(eq(filteredSkillExecutions.skill, filters.skill)); - if (filters.errorCode) executionConditions.push(eq(filteredSkillExecutions.errorCode, filters.errorCode)); conditions.push(exists( database.select({ value: sql`1` }) .from(filteredSkillExecutions) - .where(and(...executionConditions)), + .where(and(...runSkillExecutionConditions(filters))), )); } return conditions; } +function runSkillExecutionConditions(filters: HistoryFilters): SQL[] { + const conditions: SQL[] = [ + eq(filteredSkillExecutions.tenantId, runs.tenantId), + eq(filteredSkillExecutions.runId, runs.id), + ]; + if (filters.skill) conditions.push(eq(filteredSkillExecutions.skill, filters.skill)); + if (filters.errorCode) conditions.push(eq(filteredSkillExecutions.errorCode, filters.errorCode)); + return conditions; +} + function encodeCursor(completedAt: Date | string, id: string): string { return Buffer.from(JSON.stringify([iso(completedAt), id]), 'utf8').toString('base64url'); } @@ -1037,13 +1041,22 @@ async function aggregateOutcomeRow( failed: sql`COUNT(*) FILTER (WHERE ${runs.outcome} = 'failure')::integer`.as('failed'), cancelled: sql`COUNT(*) FILTER (WHERE ${runs.outcome} = 'cancelled')::integer`.as('cancelled'), skipped: sql`COUNT(*) FILTER (WHERE ${runs.outcome} = 'skipped')::integer`.as('skipped'), - findings: sql`COALESCE(SUM(${runs.findingCount}), 0)::integer`.as('findings'), + findings: sql`COALESCE(SUM(${runFindingCount(read, filters)}), 0)::integer`.as('findings'), }) .from(runs) .where(and(...conditions)); return rows[0]; } +// runs.finding_count covers every skill in the run. With a skill or error +// filter, count only the matching executions so totals match the findings list. +function runFindingCount(database: WardenReadDatabase, filters: HistoryFilters): SQL { + if (!filters.skill && !filters.errorCode) return sql`${runs.findingCount}`; + return sql`${database.select({ value: sql`SUM(${filteredSkillExecutions.findingCount})` }) + .from(filteredSkillExecutions) + .where(and(...runSkillExecutionConditions(filters)))}`; +} + function mapOutcomeRow(row: Awaited>) { return { runs: Number(row?.runs ?? 0), diff --git a/packages/warden-service/src/integration/postgres.integration.test.ts b/packages/warden-service/src/integration/postgres.integration.test.ts index 7ceaf1a3..adaf1e2d 100644 --- a/packages/warden-service/src/integration/postgres.integration.test.ts +++ b/packages/warden-service/src/integration/postgres.integration.test.ts @@ -373,6 +373,24 @@ function defineDriverIntegration(driver: DatabaseDriver, environmentName: string { dimension: 'skill', groups: expect.any(Array) }, ], }); + const multiSkill = findingsEnvelope(`history-multi-${randomUUID()}`); + await ingestRun(database, context, { + ...multiSkill, + findingCounts: counts(3), + skills: [ + { ...multiSkill.skills[0]!, findingCounts: counts(1) }, + { ...metricsEnvelope(multiSkill.clientRunId).skills[1]!, findingCounts: counts(2) }, + ], + }); + await expect(summarizeOutcomes(database, context, {})).resolves.toMatchObject({ + totals: { runs: 2, findings: 4 }, + }); + await expect(summarizeOutcomes(database, context, { skill: 'performance' })).resolves.toMatchObject({ + totals: { runs: 1, findings: 2 }, + }); + await expect(summarizeOutcomes(database, context, { skill: 'security' })).resolves.toMatchObject({ + totals: { runs: 2, findings: 2 }, + }); }, 30_000); it('persists multiple passive evidence rows in one transaction', async () => {