Cross-run failure clustering on persisted error signatures#59
Open
joefairburn wants to merge 6 commits into
Open
Cross-run failure clustering on persisted error signatures#59joefairburn wants to merge 6 commits into
joefairburn wants to merge 6 commits into
Conversation
…atures Persist the normalized failure fingerprint at ingest (testResults.errorSignature + partial project/signature/createdAt index), add a first-class Failures page (per-signature clusters, first/last seen, New pills, KPI strip), classify run-page failure rows as new-vs-known via a UI-only enrichment beside attachHasTrace, and switch the MCP diagnose / test-history tools to read the persisted column so no surface re-derives the grouping key.
… clustering Two review findings from the failure-clustering branch: - Synthetic runs get no new-vs-known flags. Run-detail pages serve monitor runs too, and classifying their rows against CI-only history would badge a recurring monitor failure "New" on every execution forever. - The window aggregate now carries each signature's project-wide first-seen as a correlated min on the grouped signature (one partial-index seek per group), so the KPI strip and the row pills share one firstSeenAt >= window definition and the loader no longer round-trips every window signature through an unbounded IN-list. loadSignatureFirstSeen (and the never-rendered firstRunId it existed for) is deleted; the example fetch stays slice-bounded.
Remaining minor review findings: - KpiInlineSkeleton in components/skeletons.tsx replaces the verbatim FlakyKpiSkeleton / FailureKpiSkeleton copies (widths per page). - NewFailurePill defines the "New" first-CI-appearance badge once for the run page's Tests tab and the Failures page's cluster rows. - The run-results route sets isNewFailure unconditionally from the flags map; undefined serializes away, so the conditional-spread branch bought nothing.
📝 WalkthroughWalkthroughThis change persists normalized failure signatures during ingest, adds cross-run failure analytics and novelty classification, introduces a streamed Failures dashboard page, displays new-failure indicators on run results, and updates MCP diagnostics to read persisted signatures. ChangesFailure clustering
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant TestResultIngest
participant FailureAnalytics
participant FailuresLoader
participant FailuresPage
participant ResultsRoute
participant TestRow
TestResultIngest->>TestResultIngest: persist normalized errorSignature
FailuresLoader->>FailureAnalytics: request aggregates and examples
FailureAnalytics->>FailuresLoader: return clustered failures and novelty data
FailuresLoader->>FailuresPage: stream KPI and table payload
ResultsRoute->>FailureAnalytics: classify paginated result signatures
FailureAnalytics->>ResultsRoute: return isNewFailure flags
ResultsRoute->>TestRow: render enriched result rows
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
wrightful | 9b28448 | Jul 22 2026, 10:09 AM |
…ntract Review follow-ups on the failure-clustering branch: - filters.ts gains ciRunsJoinFragmentAs(resultsAlias, runsAlias) — the one textual home of the raw-SQL origin <> 'synthetic' join. ciRunsJoinFragment and testResultsScopeJoin now delegate to it, and the failures loader's correlated first-seen subquery composes it under its prior/prior_run aliases instead of re-typing the policy clause. Aliases are guarded by assertSqlIdentifier, matching the per-test.ts raw-fragment idiom. - loadNewFailureFlags takes readonly string[] ids — it only ever used row ids, and the narrower contract drops the realtime RunProgressTest type from the lib module. - analytics-filters.workers.test.ts pins the delegated construction (via renderSql, which already handles nested/raw fragments) and adds alias-rendering + injection-guard tests for the new fragment.
joefairburn
marked this pull request as ready for review
July 20, 2026 11:29
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
apps/dashboard/src/__tests__/pg-integration/mcp-diagnose.test.ts (1)
78-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the fixture prove the stored value is used.
This seeds the same value that the old read-time normalization would produce, so it cannot catch a regression back to recomputing signatures. Seed one failure with a deliberately distinct persisted signature and assert both MCP history and diagnosis return/group that value.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/dashboard/src/__tests__/pg-integration/mcp-diagnose.test.ts` around lines 78 - 80, Update the fixture using failureSignature in the MCP integration test so at least one seeded failure has a deliberately different persisted errorSignature from the value recomputed from status, errorMessage, and errorStack. Extend assertions for both MCP history and diagnosis to verify they return and group by this persisted signature, proving reads use the stored value.apps/dashboard/src/__tests__/pg-integration/failure-clustering.test.ts (1)
20-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the schema import alias.
Replace the relative schema import with
@schemato keep database access on the dashboard’s required import boundary.Proposed fix
-const { runs, testResults } = await import("../../../db/schema"); +const { runs, testResults } = await import("`@schema`");As per coding guidelines, “Import the database instance from
void/dband schema tables from@schemawhen using Drizzle.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/dashboard/src/__tests__/pg-integration/failure-clustering.test.ts` at line 20, Update the schema import in the failure-clustering integration test to use the `@schema` alias instead of the relative path, preserving the existing runs and testResults usage.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/dashboard/db/migrations/20260717220925_chubby_blockbuster.sql`:
- Line 2: Update the migration statement for
testResults_project_signature_createdAt_idx to use CREATE INDEX CONCURRENTLY,
and configure the migration step as non-transactional so the concurrent index
creation is executed outside a transaction.
In `@apps/dashboard/src/lib/mcp/diagnose.ts`:
- Line 20: Replace the legacy error-signature import in
apps/dashboard/src/lib/mcp/diagnose.ts at lines 20-20 with the configured
`@void/`* namespace, and update the matching dynamic import in
apps/dashboard/src/__tests__/pg-integration/mcp-diagnose.test.ts at lines 15-15
to use the same Void package path.
In `@docs/worklog/2026-07-17-failure-signature-clustering.md`:
- Line 1: Update the worklog heading in
“2026-07-17-failure-signature-clustering.md” to include the documented commit’s
UTC author date, matching the filename date and not the review date.
- Line 83: Update the verification entry in the worklog to replace the pnpm test
and check commands with the applicable Vite+ commands, using vp test, vp check,
or vp run <script> as appropriate for the dashboard package.
---
Nitpick comments:
In `@apps/dashboard/src/__tests__/pg-integration/failure-clustering.test.ts`:
- Line 20: Update the schema import in the failure-clustering integration test
to use the `@schema` alias instead of the relative path, preserving the existing
runs and testResults usage.
In `@apps/dashboard/src/__tests__/pg-integration/mcp-diagnose.test.ts`:
- Around line 78-80: Update the fixture using failureSignature in the MCP
integration test so at least one seeded failure has a deliberately different
persisted errorSignature from the value recomputed from status, errorMessage,
and errorStack. Extend assertions for both MCP history and diagnosis to verify
they return and group by this persisted signature, proving reads use the stored
value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4d4cd7f5-3f50-46ea-bc98-98795ac89e5d
📒 Files selected for processing (27)
apps/dashboard/db/migrations/20260717220925_chubby_blockbuster.sqlapps/dashboard/db/migrations/meta/20260717220925_snapshot.jsonapps/dashboard/db/migrations/meta/_journal.jsonapps/dashboard/db/schema.tsapps/dashboard/pages/t/[teamSlug]/p/[projectSlug]/failures.server.tsapps/dashboard/pages/t/[teamSlug]/p/[projectSlug]/failures.tsxapps/dashboard/pages/t/[teamSlug]/p/[projectSlug]/flaky.tsxapps/dashboard/routes/api/t/[teamSlug]/p/[projectSlug]/runs/[runId]/results.tsapps/dashboard/src/__tests__/analytics-filters.workers.test.tsapps/dashboard/src/__tests__/analytics-kpi-summaries.test.tsapps/dashboard/src/__tests__/error-signature.workers.test.tsapps/dashboard/src/__tests__/pg-integration/failure-clustering.test.tsapps/dashboard/src/__tests__/pg-integration/mcp-diagnose.test.tsapps/dashboard/src/components/app-layout.tsxapps/dashboard/src/components/new-failure-pill.tsxapps/dashboard/src/components/run-progress-row.tsxapps/dashboard/src/components/skeletons.tsxapps/dashboard/src/lib/analytics/failures.tsapps/dashboard/src/lib/analytics/filters.tsapps/dashboard/src/lib/error-signature.tsapps/dashboard/src/lib/failure-novelty.tsapps/dashboard/src/lib/ingest.tsapps/dashboard/src/lib/mcp/diagnose.tsapps/dashboard/src/realtime/events.tsdocs/ARCHITECTURE.mddocs/worklog/2026-07-17-failure-signature-clustering.mdpackages/e2e/tests-dashboard/global-setup.ts
…ure-clustering Conflict: db/migrations/meta/_journal.json — both branches appended a migration. Resolved by taking main's journal entry and regenerating the errorSignature migration on top of main's snapshot (20260717220925 → 20260722100729_dear_smiling_tiger; identical SQL), so the latest Drizzle snapshot is cumulative over both changes and the next db:generate stays clean.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b284486b3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -0,0 +1,2 @@ | |||
| ALTER TABLE "testResults" ADD COLUMN "errorSignature" text;--> statement-breakpoint | |||
There was a problem hiding this comment.
On any existing deployment, this adds the column as NULL for every retained result, while the Failures queries filter to non-null signatures and the MCP diagnosis/history readers now return this persisted value rather than deriving it from errorMessage/errorStack. Consequently the new Failures page is empty for all pre-migration history and MCP signature groups disappear until enough new runs arrive, despite the underlying failure text still being present. Populate the column for retained failure rows in the migration (or retain a read fallback until backfill completes).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/dashboard/db/migrations/20260722100729_dear_smiling_tiger.sql`:
- Line 1: Extend the migration that adds errorSignature with a resumable
backfill for retained failure rows, using the same normalization logic as the
existing ingest flow. Populate signatures for eligible historical rows before
analytics failure reads exclude NULL values, and make the backfill restartable
without duplicating or corrupting existing signatures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 09dfb279-5766-4014-b574-24001210e3b4
📒 Files selected for processing (7)
apps/dashboard/db/migrations/20260722100729_dear_smiling_tiger.sqlapps/dashboard/db/migrations/meta/20260722100729_snapshot.jsonapps/dashboard/db/migrations/meta/_journal.jsonapps/dashboard/db/schema.tsapps/dashboard/src/lib/ingest.tsdocs/ARCHITECTURE.mddocs/worklog/2026-07-17-failure-signature-clustering.md
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/dashboard/db/schema.ts
- docs/ARCHITECTURE.md
- apps/dashboard/src/lib/ingest.ts
- docs/worklog/2026-07-17-failure-signature-clustering.md
| @@ -0,0 +1,2 @@ | |||
| ALTER TABLE "testResults" ADD COLUMN "errorSignature" text;--> statement-breakpoint | |||
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Backfill retained failure rows before relying on novelty.
Existing rows keep errorSignature = NULL; apps/dashboard/src/lib/analytics/failures.ts:40-96 excludes them, while apps/dashboard/src/lib/ingest.ts:415-447 only populates new/re-streamed results. This makes previously known retained failures absent from clusters and eligible to be shown as “New.” Add a resumable retained-history backfill using the same normalizer before enabling these reads.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/dashboard/db/migrations/20260722100729_dear_smiling_tiger.sql` at line
1, Extend the migration that adds errorSignature with a resumable backfill for
retained failure rows, using the same normalization logic as the existing ingest
flow. Populate signatures for eligible historical rows before analytics failure
reads exclude NULL values, and make the backfill restartable without duplicating
or corrupting existing signatures.
This was referenced Jul 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Adds cross-run failure clustering built on a persisted error fingerprint, answering "is this failure new, or the same thing that's been failing all week?" — which per-run, computed-on-read grouping can't do without scanning all retained history per page load. Ingest now stamps
testResults.errorSignature(normalized message-first/stack-fallback fingerprint, partial-indexed per project), a new Failures page clusters signatures over a 7/14/30d window with KPIs and "New" pills, run-detail failure rows get a "New" badge when the signature first appears in CI on that run, and the MCP diagnose/history tools read the persisted column so dashboard and MCP can never disagree on grouping. Novelty is project-wide, CI-only, and retention-bounded; synthetic monitor runs are excluded from clustering and never badged.How verified
pnpm checkpasses (format + lint + type-check)error-signature.workers.test.ts,analytics-kpi-summaries.test.ts), newpg-integration/failure-clustering.test.tsagainst real Postgres, reseededpg-integration/mcp-diagnose.test.ts, fullpnpm --filter @wrightful/dashboard test; dashboard e2e preflight now covers/failuresSSRdocs/worklog/—2026-07-17-failure-signature-clustering.md20260717220925_chubby_blockbuster.sql)Notes for reviewers
The stored value freezes the normalizer: changing
normalizeErrorSignature's masking rules later means a backfill over retained rows (cheap pre-launch, a real migration after). A signature quiet longer than run retention resurfaces as "New" — intended semantics. Two overlapping runs racing the same brand-new failure can both badge "New"; accepted as the honest reading.Summary by CodeRabbit
New Features
Bug Fixes
Documentation