Recovered: feat(test): add failure triage for batch root-cause grouping (#44 by @SahilRakhaiya05)#245
Open
jangjos-128 wants to merge 8 commits into
Open
Conversation
Add testsprite test failure triage --project <id> to group failed tests into root-cause clusters using existing M2.1 analysis fields. Returns a representative test per cluster, confidence score, and fix priority without downloading failure bundles. Includes grouping library, command wiring, unit/integration tests, docs, CHANGELOG entry, agent skill update, and help snapshot.
…iage - Fix test failure triage help snapshot default value quoting - Run prettier on changed files - Add filter and max-concurrency validation tests - Remove draft issue/PR markdown files from repo
- Move --dry-run payload to sampleFailureTriageResult in samples.ts - Use .option for --project so requireProjectId emits exit 5 - Build cluster labels from the chosen representative test - Append short hash suffix to clusterId to avoid slug collisions
WalkthroughAdds a client-side ChangesFailure triage
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant runFailureTriage
participant TestsAPI
participant FailureSummaryAPI
participant buildFailureClusters
CLI->>runFailureTriage: triage options
runFailureTriage->>TestsAPI: fetch failed tests
runFailureTriage->>FailureSummaryAPI: fetch failure summaries
runFailureTriage->>buildFailureClusters: clustering inputs
buildFailureClusters-->>runFailureTriage: scored clusters
runFailureTriage-->>CLI: JSON or text output
Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@skills/testsprite-verify.skill.md`:
- Line 434: Update the single-failure workflow instruction to use the
test-id-based failure bundle command, replacing the run-id-dependent artifact
retrieval with testsprite test failure get <test-id>. Keep the instruction
scoped to the single failed test path and preserve the existing triage-skipping
behavior.
In `@src/commands/test.ts`:
- Around line 4520-4557: Update the failure-summary fan-out around startNext and
its request catch handler so only NOT_FOUND errors whose details explicitly
identify no_failing_run are added to skipped; propagate all other errors
unchanged for the existing documented exit-code mapping. Track terminal
rejection state, prevent startNext from scheduling additional requests after the
first terminal error, and ensure in-flight completions cannot resolve
successfully afterward. Add coverage for a distinct NOT_FOUND reason that
verifies propagation and confirms no further requests are started.
In `@src/lib/dry-run/samples.ts`:
- Around line 350-422: Update sampleFailureTriageResult to construct canned
FailureTriageInput[] matching these scenarios and return the result of
buildFailureClusters instead of hard-coded clusters and derived fields. Preserve
the intended network-timeout and CheckoutForm fix-target cases, allowing
production-generated priorities, hashed cluster IDs, labels, and summaries to
flow through. Add the required matching dry-run parity test using this sample
and the production clustering output.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1c011c06-262d-4835-9950-72c7ddcdc1f7
⛔ Files ignored due to path filters (1)
test/__snapshots__/help.snapshot.test.ts.snapis excluded by!**/*.snap,!**/*.snap
📒 Files selected for processing (11)
CHANGELOG.mdDOCUMENTATION.mdREADME.mdskills/testsprite-verify.skill.mdsrc/commands/test.test.tssrc/commands/test.tssrc/lib/dry-run/samples.test.tssrc/lib/dry-run/samples.tssrc/lib/failure-triage.test.tssrc/lib/failure-triage.tstest/help.snapshot.test.ts
| representative test from the highest-priority cluster — not an arbitrary failed | ||
| test. After a fix, rerun that representative before rerunning the full suite. | ||
|
|
||
| For a **single** failed test, skip triage and go straight to the artifact: |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== skills/testsprite-verify.skill.md around line 434 ==\n'
sed -n '410,455p' skills/testsprite-verify.skill.md
printf '\n== DOCUMENTATION.md search for failure-related commands ==\n'
rg -n "test failure get|test list --status failed|run ID|run id|latest-failure|failure" DOCUMENTATION.md skills/testsprite-verify.skill.mdRepository: TestSprite/testsprite-cli
Length of output: 7971
Use the test-id-based failure bundle here. test artifact get needs a run-id; for the single-failure path, call testsprite test failure get <test-id> instead.
🤖 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 `@skills/testsprite-verify.skill.md` at line 434, Update the single-failure
workflow instruction to use the test-id-based failure bundle command, replacing
the run-id-dependent artifact retrieval with testsprite test failure get
<test-id>. Keep the instruction scoped to the single failed test path and
preserve the existing triage-skipping behavior.
Comment on lines
+4520
to
+4557
| await new Promise<void>((resolve, reject) => { | ||
| function startNext(): void { | ||
| while (inFlight < concurrencyLimit && nextIdx < failedTests.length) { | ||
| const test = failedTests[nextIdx++]!; | ||
| inFlight++; | ||
| client | ||
| .get<CliFailureSummary>(`/tests/${encodeURIComponent(test.id)}/failure/summary`) | ||
| .then(summary => { | ||
| triageInputs.push({ | ||
| testId: test.id, | ||
| testName: test.name, | ||
| testType: test.type, | ||
| updatedAt: test.updatedAt, | ||
| summary: { | ||
| status: summary.status, | ||
| failureKind: summary.failureKind, | ||
| snapshotId: summary.snapshotId, | ||
| rootCauseHypothesis: summary.rootCauseHypothesis, | ||
| recommendedFixTarget: summary.recommendedFixTarget, | ||
| }, | ||
| }); | ||
| inFlight--; | ||
| startNext(); | ||
| if (inFlight === 0 && nextIdx >= failedTests.length) resolve(); | ||
| }) | ||
| .catch(err => { | ||
| if (err instanceof ApiError && err.code === 'NOT_FOUND') { | ||
| skipped.push({ testId: test.id, reason: 'no_failing_run' }); | ||
| if (opts.verbose) { | ||
| stderrFn(`[triage] skipped ${test.id} — no failing run (race or stale list row)`); | ||
| } | ||
| } else { | ||
| reject(err); | ||
| } | ||
| inFlight--; | ||
| startNext(); | ||
| if (inFlight === 0 && nextIdx >= failedTests.length) resolve(); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not swallow unrelated NOT_FOUND errors or continue fan-out after rejection.
Every NOT_FOUND currently becomes no_failing_run, so missing-resource or unsupported-route errors return success instead of their mapped exit code. Moreover, startNext() continues scheduling requests after reject(err). Only skip errors whose details explicitly identify no_failing_run, and stop scheduling after the first terminal error.
Add coverage for another NOT_FOUND reason and assert that no further requests start. As per path instructions, “error paths must map to the documented exit code; don't introduce ad-hoc codes.”
Proposed approach
let nextIdx = 0;
let inFlight = 0;
+ let aborted = false;
await new Promise<void>((resolve, reject) => {
function startNext(): void {
- while (inFlight < concurrencyLimit && nextIdx < failedTests.length) {
+ while (!aborted && inFlight < concurrencyLimit && nextIdx < failedTests.length) {
// ...
}
}
// ...
.catch(err => {
- if (err instanceof ApiError && err.code === 'NOT_FOUND') {
+ const reason =
+ err instanceof ApiError
+ ? err.getDetail('reason', (value): value is string => typeof value === 'string')
+ : undefined;
+
+ if (err instanceof ApiError && err.code === 'NOT_FOUND' && reason === 'no_failing_run') {
skipped.push({ testId: test.id, reason: 'no_failing_run' });
} else {
+ aborted = true;
reject(err);
}
inFlight--;
startNext();
- if (inFlight === 0 && nextIdx >= failedTests.length) resolve();
+ if (!aborted && inFlight === 0 && nextIdx >= failedTests.length) resolve();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| await new Promise<void>((resolve, reject) => { | |
| function startNext(): void { | |
| while (inFlight < concurrencyLimit && nextIdx < failedTests.length) { | |
| const test = failedTests[nextIdx++]!; | |
| inFlight++; | |
| client | |
| .get<CliFailureSummary>(`/tests/${encodeURIComponent(test.id)}/failure/summary`) | |
| .then(summary => { | |
| triageInputs.push({ | |
| testId: test.id, | |
| testName: test.name, | |
| testType: test.type, | |
| updatedAt: test.updatedAt, | |
| summary: { | |
| status: summary.status, | |
| failureKind: summary.failureKind, | |
| snapshotId: summary.snapshotId, | |
| rootCauseHypothesis: summary.rootCauseHypothesis, | |
| recommendedFixTarget: summary.recommendedFixTarget, | |
| }, | |
| }); | |
| inFlight--; | |
| startNext(); | |
| if (inFlight === 0 && nextIdx >= failedTests.length) resolve(); | |
| }) | |
| .catch(err => { | |
| if (err instanceof ApiError && err.code === 'NOT_FOUND') { | |
| skipped.push({ testId: test.id, reason: 'no_failing_run' }); | |
| if (opts.verbose) { | |
| stderrFn(`[triage] skipped ${test.id} — no failing run (race or stale list row)`); | |
| } | |
| } else { | |
| reject(err); | |
| } | |
| inFlight--; | |
| startNext(); | |
| if (inFlight === 0 && nextIdx >= failedTests.length) resolve(); | |
| }); | |
| let nextIdx = 0; | |
| let inFlight = 0; | |
| let aborted = false; | |
| await new Promise<void>((resolve, reject) => { | |
| function startNext(): void { | |
| while (!aborted && inFlight < concurrencyLimit && nextIdx < failedTests.length) { | |
| const test = failedTests[nextIdx++]!; | |
| inFlight++; | |
| client | |
| .get<CliFailureSummary>(`/tests/${encodeURIComponent(test.id)}/failure/summary`) | |
| .then(summary => { | |
| triageInputs.push({ | |
| testId: test.id, | |
| testName: test.name, | |
| testType: test.type, | |
| updatedAt: test.updatedAt, | |
| summary: { | |
| status: summary.status, | |
| failureKind: summary.failureKind, | |
| snapshotId: summary.snapshotId, | |
| rootCauseHypothesis: summary.rootCauseHypothesis, | |
| recommendedFixTarget: summary.recommendedFixTarget, | |
| }, | |
| }); | |
| inFlight--; | |
| startNext(); | |
| if (inFlight === 0 && nextIdx >= failedTests.length) resolve(); | |
| }) | |
| .catch(err => { | |
| const reason = | |
| err instanceof ApiError | |
| ? err.getDetail('reason', (value): value is string => typeof value === 'string') | |
| : undefined; | |
| if (err instanceof ApiError && err.code === 'NOT_FOUND' && reason === 'no_failing_run') { | |
| skipped.push({ testId: test.id, reason: 'no_failing_run' }); | |
| if (opts.verbose) { | |
| stderrFn(`[triage] skipped ${test.id} — no failing run (race or stale list row)`); | |
| } | |
| } else { | |
| aborted = true; | |
| reject(err); | |
| } | |
| inFlight--; | |
| startNext(); | |
| if (!aborted && inFlight === 0 && nextIdx >= failedTests.length) resolve(); | |
| }); |
🤖 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 `@src/commands/test.ts` around lines 4520 - 4557, Update the failure-summary
fan-out around startNext and its request catch handler so only NOT_FOUND errors
whose details explicitly identify no_failing_run are added to skipped; propagate
all other errors unchanged for the existing documented exit-code mapping. Track
terminal rejection state, prevent startNext from scheduling additional requests
after the first terminal error, and ensure in-flight completions cannot resolve
successfully afterward. Add coverage for a distinct NOT_FOUND reason that
verifies propagation and confirms no further requests are started.
Source: Path instructions
Comment on lines
+350
to
+422
| export function sampleFailureTriageResult(projectId: string): FailureTriageResult { | ||
| return { | ||
| projectId, | ||
| clusters: [ | ||
| { | ||
| clusterId: 'cluster_kind_network_timeout', | ||
| label: 'Environment issue (network_timeout)', | ||
| groupKey: 'kind:network_timeout', | ||
| groupReason: 'failure_kind', | ||
| failureKind: 'network_timeout', | ||
| representativeTestId: 'test_dryrun_a', | ||
| memberTestIds: ['test_dryrun_a', 'test_dryrun_b'], | ||
| members: [ | ||
| { | ||
| testId: 'test_dryrun_a', | ||
| testName: 'Dry-run checkout flow', | ||
| testType: 'frontend', | ||
| updatedAt: '2026-06-26T12:00:00.000Z', | ||
| status: 'failed', | ||
| failureKind: 'network_timeout', | ||
| snapshotId: 'snap_dryrun_a', | ||
| rootCauseHypothesis: null, | ||
| recommendedFixTarget: null, | ||
| }, | ||
| { | ||
| testId: 'test_dryrun_b', | ||
| testName: 'Dry-run profile update', | ||
| testType: 'frontend', | ||
| updatedAt: '2026-06-26T12:01:00.000Z', | ||
| status: 'failed', | ||
| failureKind: 'network_timeout', | ||
| snapshotId: 'snap_dryrun_b', | ||
| rootCauseHypothesis: null, | ||
| recommendedFixTarget: null, | ||
| }, | ||
| ], | ||
| canonicalRootCause: null, | ||
| confidence: 0.88, | ||
| fixPriority: 1, | ||
| }, | ||
| { | ||
| clusterId: 'cluster_ref_src_components_checkoutform_tsx_412', | ||
| label: 'Shared fix target: src/components/CheckoutForm.tsx:412', | ||
| groupKey: 'ref:src/components/CheckoutForm.tsx:412', | ||
| groupReason: 'fix_target', | ||
| failureKind: 'assertion', | ||
| representativeTestId: 'test_dryrun_c', | ||
| memberTestIds: ['test_dryrun_c'], | ||
| members: [ | ||
| { | ||
| testId: 'test_dryrun_c', | ||
| testName: 'Dry-run submit checkout', | ||
| testType: 'frontend', | ||
| updatedAt: '2026-06-26T12:02:00.000Z', | ||
| status: 'failed', | ||
| failureKind: 'assertion', | ||
| snapshotId: 'snap_dryrun_c', | ||
| rootCauseHypothesis: | ||
| 'Submit button is disabled because the credit-card field is empty.', | ||
| recommendedFixTarget: { | ||
| kind: 'code', | ||
| reference: 'src/components/CheckoutForm.tsx:412', | ||
| rationale: 'Disabled state originates from `isFormValid()`.', | ||
| }, | ||
| }, | ||
| ], | ||
| canonicalRootCause: 'Submit button is disabled because the credit-card field is empty.', | ||
| confidence: 0.7, | ||
| fixPriority: 3, | ||
| }, | ||
| ], | ||
| summary: { totalFailed: 3, clusterCount: 2, skipped: 0 }, | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Generate the dry-run result through the production clustering engine.
This fixture has already drifted from live output: a one-member fix_target cluster receives priority 6 in buildFailureClusters, not 3, and production cluster IDs include hash suffixes. Build canned FailureTriageInput[] and pass them to buildFailureClusters so derived fields cannot diverge.
As per path instructions, dry-run parity requires a matching canned sample and test.
🤖 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 `@src/lib/dry-run/samples.ts` around lines 350 - 422, Update
sampleFailureTriageResult to construct canned FailureTriageInput[] matching
these scenarios and return the result of buildFailureClusters instead of
hard-coded clusters and derived fields. Preserve the intended network-timeout
and CheckoutForm fix-target cases, allowing production-generated priorities,
hashed cluster IDs, labels, and summaries to flow through. Add the required
matching dry-run parity test using this sample and the production clustering
output.
Source: Path instructions
Contributor
Author
|
Hi @SahilRakhaiya05 👋 — this is the recovery of your PR #44 , which was auto-closed by an error in our 2026-07-09 release process. Your commits are intact — thank you for the contribution! It currently shows merge conflicts because Option A — merge (no force-push): git checkout BRANCH
git fetch upstream # your remote for TestSprite/testsprite-cli (may be "origin")
git merge upstream/main # resolve conflicts, commit
git pushOption B — rebase (cleaner history, needs force): git checkout BRANCH
git fetch upstream
git rebase upstream/main # resolve conflicts
git push --force-with-leaseOnce you push, the conflicts here resolve automatically and we'll take it into review. Sorry again for the extra step! |
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
2 participants
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.
This PR adds a new CLI command:
The command groups failed tests into root-cause clusters instead of returning a flat list of unrelated failures. This helps agents and developers quickly identify the highest-priority issue, investigate one representative test first, and avoid fixing the same underlying problem multiple times.
Each cluster includes:
The command uses existing TestSprite APIs only, so no backend changes are required. It fetches lightweight failure summary data per test and does not download screenshots, videos, or full failure bundles.
Problem
Today, when a batch run fails many tests, the CLI and agents only see individual failures:
This makes agents and developers:
The CLI already has strong per-test analysis through
test failure get,test failure summary,rootCauseHypothesis,recommendedFixTarget, andfailureKind.What was missing is cross-test grouping after a batch failure.
Solution
test failure triageworks in three steps:The grouping algorithm uses the following signals:
recommendedFixTarget.referencefailureKind, such asnetwork_timeoutorinfrarootCauseHypothesisClusters are ordered by fix priority first, then by member count.
Command surface
Supported options:
--project <id>— required project ID--type frontend|backend— filter failed tests by type--filter <substr>— filter tests by name substring--max-concurrency <n>— parallel summary fetches, default 5--output json|text— machine or human output--endpoint-url <url>— override API hostAlso supports global flags such as
--dry-run,--profile,--verbose, and--debug.Recommended agent workflow
The agent skill was also updated to recommend triage before downloading bundles when multiple tests fail.
Implementation details
Added new grouping logic in:
This includes:
normalizeHypothesis()computeGroupKey()pickRepresentativeTestId()computeClusterConfidence()computeFixPriority()buildFailureClusters()renderFailureTriageText()Added command implementation in:
The command validates inputs, paginates failed tests, applies filters, fetches summaries with bounded concurrency, handles stale failed rows, and emits JSON or text output through the existing output system.
Test coverage
This PR adds 18 automated tests:
src/lib/failure-triage.test.tssrc/commands/test.test.tsCoverage includes:
Future work
Out of scope for this PR:
GET /projects/{id}/failures/clustersAPIrootCauseHypothesis--rerun-representatives --waitorchestration flagChecklist
The Pr solving issue #116
Summary by CodeRabbit
New Features
test failure triage --project <id>to group failed tests into root-cause clusters.Documentation