feat(test): add failure triage for batch root-cause grouping#44
Closed
SahilRakhaiya05 wants to merge 77 commits into
Closed
feat(test): add failure triage for batch root-cause grouping#44SahilRakhaiya05 wants to merge 77 commits into
SahilRakhaiya05 wants to merge 77 commits into
Conversation
The Lint & Format job runs prettier --check over workflow YAML; the aligned trailing comment failed it on every push.
Updated the README with a new link and added a video description.
- prettier-clean the README video block (fixes CI format:check on main) - bump version to 0.1.1 - CHANGELOG: add [0.1.1] docs-only entry Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
NPM_TOKEN secret was never configured, so tag-triggered releases failed with ENEEDAUTH. Auth now uses npm trusted publishing (configured on npmjs.com for this repo + release.yaml): - drop NODE_AUTH_TOKEN env (empty token would shadow OIDC auth) - upgrade npm to >= 11.5.1 (trusted publishing requirement; Node 22 bundles 10.x) - bump checkout/setup-node to v5 ahead of the June 16 Node 20 runner cutoff
ci(release): switch npm publish to OIDC trusted publishing
- Onboarding consolidated into `testsprite setup` (formerly `init`); the granular auth commands remain as hidden, deprecated aliases. - CLI reports its version in the User-Agent header. - README: the launch video no longer renders as a bare URL on npm.
create-batch --run launched its first concurrencyLimit triggers in parallel, but the steady-state loop awaited each subsequent job to fully finish (trigger + full --wait poll) before launching the next one. Effective concurrency dropped to 1 after the initial wave regardless of --max-concurrency. Switch to the launch-then-race pattern already used by the other three fan-outs in this file: launch up to the limit, relaunch on each completion via startNext(), never await a whole job inline. Add a regression test using equal-delay trigger responses so the first wave settles in the same microtask batch, which is the exact condition that exposed the bug.
…urrency fix(test): prevent batch-run scheduler from serializing after first wave
…estSprite#7) test code get --out <path> opened (truncated) the destination file before the network request. If the GET then failed, or hit the "no code generated yet" branch which writes nothing, the user's pre-existing --out file was left emptied with no way to recover it. Write to a sibling temp file instead and rename it onto the real path only after a successful, complete write. Mirrors the atomic rename contract bundle.ts already uses for multi-file bundles. Add regression tests for both failure modes: a failing fetch and the no-code-yet branch. Both reproduce the truncation on the old code and pass on the fix.
…ite#9) Batch-rerun chunks (>50 testIds) were dispatched concurrently via Promise.all. The backend's producer/teardown closure dedup happens per-request, not across requests, so two concurrent chunks sharing a project's producer could each independently decide to trigger it, double-running the producer or teardown. Dispatch chunks sequentially in both the initial and deferred-retry paths, closing the race at the source. Also dedupe the aggregated accepted[] by testId and merge closure.byProject across chunks as a defensive second layer, warning on stderr if a duplicate trigger is detected. Fixed a pre-existing test whose fixture relied on the old double-counting behavior (same accepted entry returned from every retry call).
…RROR (TestSprite#19) A malformed API endpoint produced an opaque or misleading failure instead of a clear config error: --endpoint-url "not a url" -> `Error: Invalid URL` (exit 1, a raw `new URL()` throw with no guidance) --endpoint-url "localhost:3000" (missing scheme, parses as scheme "localhost:") and "ftp://x" (wrong scheme) -> `fetch failed` / Service unavailable, emitted only after a full retry+backoff cycle — looks like a network outage, not a config typo. Add `assertValidEndpointUrl` in client-factory.ts and run it in both the real and dry-run paths of `makeHttpClient`, on the resolved endpoint (so it covers --endpoint-url, TESTSPRITE_API_URL, and the credentials file). A malformed value now throws a typed VALIDATION_ERROR (exit 5) with an actionable message. Crucially, and unlike the `--target-url` SSRF guard, this does NOT reject localhost or private hosts — the API endpoint legitimately points at a self-hosted, local-dev, or mock backend. Only syntactically invalid values (unparseable, or a non-http(s) scheme) are rejected, so existing self-hosted/CI configs and the test suite's localhost mock backend are unaffected. Adds unit coverage for assertValidEndpointUrl and the two makeHttpClient paths, plus subprocess regressions.
runFailureGet now resolves and validates --out via resolveBundleDir and assertOutDirParentExists before calling GET /tests/{id}/failure, matching runArtifactGet and runCodeGet fast-fail behavior.
Adds regression tests asserting zero fetch calls on empty --out and missing parent dir paths.
…TestSprite#23) Co-authored-by: zeshi-du <zeshi@testsprite.com>
…on (TestSprite#21) A profile name (`--profile` / `TESTSPRITE_PROFILE`) is written verbatim as an INI section header (`[name]`) in `~/.testsprite/credentials`, but was never validated. A name containing the characters that break that grammar silently corrupted the file: --profile "prod]" -> serialises to `[prod]]`, which the section regex cannot match, so the api_key/api_url lines that follow are DROPPED on read. `setup` reports success while the credential never persists. --profile $'a\nb' -> the newline splits the header across two lines. --profile " x " -> does not round-trip (the parser trims section names, so it reads back as `x`). Add `assertValidProfileName` in credentials.ts (a conservative allowlist: letters, digits, dot, underscore, hyphen — covering `default`, `prod`, `ci-staging`, `team.qa`) and call it from every profile-keyed entry point (`readProfile`, `writeProfile`, `deleteProfile`). A malformed name now throws a typed VALIDATION_ERROR (exit 5) before any file write, instead of silently corrupting or failing to persist credentials. Adds unit coverage for the guard and the three entry points, plus a subprocess regression. Co-authored-by: Zeshi Du <duke.zeshi@gmail.com>
…t json (TestSprite#22) When a subcommand fired a parse error (unknown command, missing required argument, invalid option), Commander's outputError callback wrote plain text to stderr immediately and the catch block exited 5 with no further output. A machine consumer that always parses stderr as JSON received an unexpected plain-text string and crashed its JSON.parse. Root cause: configureOutput was only applied to the root program, not to subcommands. Each subcommand retained the default outputError that calls write(str) directly. applyExitOverrideDeep now also propagates configureOutput to every leaf so the message is buffered instead of written. In the CommanderError catch block, a resolved output mode is used to either write a VALIDATION_ERROR JSON envelope or the buffered plain-text message. An argv scan fallback handles the edge case where --output json appears after the bad argument and was not yet parsed when the error fired. The renderCommanderError helper is extracted to src/lib/render-error.ts (alongside the existing rephraseUnknownOption helper) so it is unit-testable without a subprocess. Eight unit tests cover JSON/text output, null fallback, message trimming, and rephrased global-flag embedding. Four subprocess regression tests in the [fix-5] block cover missing-arg, unknown subcommand, argv-fallback, and text-mode no-regression paths. Co-authored-by: zeshi-du <zeshi@testsprite.com>
New issue form for hackathon submissions — auto-applies the `hackathon` label and captures the submitter's Discord identity for reward payout coordination.
Contributor
|
@coderabbitai review |
✅ Action performedReview finished.
|
WalkthroughThis PR adds a client-side "test failure triage" CLI command that clusters a project's failed tests by root cause using existing analysis fields (fix target reference, failure kind, hypothesis) without downloading failure bundles. Includes a new clustering library, CLI wiring, dry-run sample data, tests, and documentation updates. ChangesFailure Triage Feature
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant runFailureTriage
participant TestSpriteAPI
participant buildFailureClusters
User->>CLI: testsprite test failure triage --project <id>
CLI->>runFailureTriage: parsed options (type, filter, maxConcurrency)
alt dry-run mode
runFailureTriage-->>CLI: sampleFailureTriageResult (canned)
else live mode
runFailureTriage->>TestSpriteAPI: fetch failed tests (paginated, filtered)
loop per failed test (bounded concurrency)
runFailureTriage->>TestSpriteAPI: GET /tests/{testId}/failure/summary
alt NOT_FOUND
TestSpriteAPI-->>runFailureTriage: 404 NOT_FOUND
runFailureTriage->>runFailureTriage: record skipped test
else success
TestSpriteAPI-->>runFailureTriage: failure summary fields
end
end
runFailureTriage->>buildFailureClusters: FailureTriageInput[]
buildFailureClusters-->>runFailureTriage: FailureTriageResult (clusters, summary)
end
runFailureTriage-->>CLI: render text or JSON
CLI-->>User: cluster report
Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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
SahilRakhaiya05
force-pushed
the
feat/failure-triage
branch
from
62af870 to
9f7c9e4
Compare
SahilRakhaiya05
requested review from
ruili-testsprite and
zeshi-du
as code owners
![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)
src/lib/failure-triage.test.ts (1)
1-212: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider adding coverage for
buildClusterLabel,renderFailureTriageText, and representative tie-breaks.Current tests exercise the grouping/priority/confidence heuristics well but skip
buildClusterLabel/renderFailureTriageTextentirely and only test the "prefers hypothesis" branch ofpickRepresentativeTestId(not the recency/testId tie-breaks). A test assertingcluster.labelis derived consistently withcluster.canonicalRootCause/representativeTestIdfor a multi-member hypothesis cluster would have caught the label/representative mismatch noted infailure-triage.ts.🤖 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/failure-triage.test.ts` around lines 1 - 212, Add tests covering the missing failure-triage paths: verify buildClusterLabel and renderFailureTriageText produce labels/text that stay aligned with cluster.canonicalRootCause and representativeTestId, especially for multi-member hypothesis clusters. Also extend pickRepresentativeTestId coverage to include the recency and testId tie-break cases, since the current suite only checks the hypothesis-preference branch. Use the existing failure-triage.test.ts helpers and symbols buildFailureClusters, buildClusterLabel, renderFailureTriageText, and pickRepresentativeTestId to locate the relevant assertions.src/commands/test.test.ts (1)
3454-3482: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a command-level missing-
--projecttest fortest failure triage.These cases only call
runFailureTriage()directly, so they won't catch the new.requiredOption('--project')parse path returning Commander's generic failure instead of the documented validation exit code. Please exercise the CLI command itself here and assert the missing-project exit behavior.As per path instructions, "Tests must be deterministic and offline" and "Cover new behavior, including error and exit-code paths."
🤖 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.test.ts` around lines 3454 - 3482, Add a command-level test for missing --project in test failure triage by exercising the CLI parser path instead of only calling runFailureTriage(), so the new .requiredOption('--project') behavior is covered. Update the existing validation tests in test.test.ts to invoke the command entrypoint for the missing-project case and assert the exact exit behavior/exit code returned by Commander, while keeping the test deterministic and offline. Use the existing runFailureTriage and test failure triage command symbols to locate the right place to extend coverage.Source: Path instructions
🤖 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 `@src/commands/test.ts`:
- Around line 4067-4141: The `opts.dryRun` branch in `test.ts` is hard-coding a
triage response that should live in the shared dry-run sample registry instead.
Move this canned `FailureTriageResult` payload into
`src/lib/dry-run/samples.ts`, update the `dry-run` sample lookup/use path so the
command reads the fixture from the registry, and add or adjust the corresponding
test to cover the shared sample shape and keep `renderFailureTriageText` output
aligned.
- Around line 8920-8926: The triage command currently uses Commander’s
requiredOption for --project, which short-circuits before runFailureTriage() and
bypasses the VALIDATION_ERROR envelope and exit-5 mapping. Update the command
definition in failure.command('triage') to make --project a plain option, then
keep validation inside requireProjectId() so the existing createTestCommand()
error-path behavior and documented exit code handling remain intact.
In `@src/lib/failure-triage.ts`:
- Around line 191-212: buildClusterLabel is re-selecting its own representative
member, which can diverge from the representative already chosen in
buildFailureClusters via representativeTestId. Update buildClusterLabel to
accept and use that existing representative (or look it up by ID) instead of
falling back to members[0], so hypothesis/fix-target labels always come from the
same test as cluster.canonicalRootCause. Keep the existing label formatting
logic, but ensure the rep/testName/rootCauseHypothesis source is consistent with
the cluster’s chosen representative.
- Around line 214-221: `slugifyClusterId` is truncating cluster IDs in a way
that can cause collisions between distinct clusters. Update `slugifyClusterId`
in `failure-triage` to keep the readable slug but append a short stable
disambiguator, such as a hash of the full `groupKey`, so different `hyp:`
entries do not end up with the same `clusterId` after truncation. Make sure the
`groups`/cluster creation flow continues to use this new unique `clusterId`
shape consistently wherever `normalizeHypothesis` and downstream consumers rely
on it.
---
Nitpick comments:
In `@src/commands/test.test.ts`:
- Around line 3454-3482: Add a command-level test for missing --project in test
failure triage by exercising the CLI parser path instead of only calling
runFailureTriage(), so the new .requiredOption('--project') behavior is covered.
Update the existing validation tests in test.test.ts to invoke the command
entrypoint for the missing-project case and assert the exact exit behavior/exit
code returned by Commander, while keeping the test deterministic and offline.
Use the existing runFailureTriage and test failure triage command symbols to
locate the right place to extend coverage.
In `@src/lib/failure-triage.test.ts`:
- Around line 1-212: Add tests covering the missing failure-triage paths: verify
buildClusterLabel and renderFailureTriageText produce labels/text that stay
aligned with cluster.canonicalRootCause and representativeTestId, especially for
multi-member hypothesis clusters. Also extend pickRepresentativeTestId coverage
to include the recency and testId tie-break cases, since the current suite only
checks the hypothesis-preference branch. Use the existing failure-triage.test.ts
helpers and symbols buildFailureClusters, buildClusterLabel,
renderFailureTriageText, and pickRepresentativeTestId to locate the relevant
assertions.
🪄 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: c17d6b2c-df3b-47d1-88af-90fadf5de85a
⛔ Files ignored due to path filters (1)
test/__snapshots__/help.snapshot.test.ts.snapis excluded by!**/*.snap,!**/*.snap
📒 Files selected for processing (11)
.github/workflows/ci.ymlCHANGELOG.mdDOCUMENTATION.mdREADME.mdskills/testsprite-verify.skill.mdsrc/commands/test.test.tssrc/commands/test.tssrc/lib/failure-triage.test.tssrc/lib/failure-triage.tstest/help.snapshot.test.tsvitest.config.ts
…Sprite#166) A successful (200) response whose body is not JSON — a misconfigured endpoint, a proxy / captive-portal / login page returning HTML with a success status, or an empty body — caused the OK path to call raw `response.json()`, whose SyntaxError escaped to the top-level handler. That produced an opaque exit 1 and, under --output json, a bare `{"error":"<parse message>"}` that breaks the typed-envelope contract every other error honors (the non-OK path already reads defensively via safeReadJson). Wrap the OK-path parse failure in a typed INTERNAL ApiError (exit 1 unchanged) that carries the requestId, names the likely cause, and points the operator at their endpoint config; details include the HTTP status, content-type, and underlying parse message. Abort/timeout errors mid-read keep their existing classification. Fixes TestSprite#94
TestSprite#33) pollAccepted in runTestRerun hardcoded exitCode:1 on ApiError, causing the auth-escalation find(r => r.error?.exitCode === 3) to always return undefined -- auth failures silently exited 1 instead of 3. - preserve err.exitCode in pollAccepted (mirrors runTestRunAll fix) - add auth escalation block before generic exit-1 throw - bound initial chunk idempotency key to <=256 chars (mirrors retry path)
* feat(agent): add kiro as an install target
Adds kiro as an own-file agent target on the current multi-skill model: AgentTarget union, a pathFor('kiro') case landing at .kiro/skills/<skill>/SKILL.md, and a TARGETS entry (experimental, own-file, wrapSkill frontmatter like claude/antigravity). Kiro installs both default skills (testsprite-verify + testsprite-onboard). Updates the --target help string, README/DOCUMENTATION target lists and counts, unit + command tests (six keys, list 12 rows, five own-file targets, 10 dry-run would-write lines, renderForTarget + content-integrity coverage), and regenerates the agent/setup help snapshots. Rebuilt on current main.
* test(e2e): include kiro in target matrix guards and multi-target install
The Local E2E Tests CI job failed because two matrix-coverage guards hardcoded the target set (claude, antigravity, cursor, cline, codex) and did not include the new kiro target. Add kiro (own-file, between cline and codex to match TARGETS order) to both guards, and add kiro to the multi-target install e2e so the target is actually exercised end-to-end.
…nd test wait (TestSprite#153) Apply the fix in src/commands/ (the compiled CLI path). When the overall --timeout polling deadline is exceeded, emit {runId, status:"running"} to stdout before exit 7 so JSON agents can chain into test wait. Co-authored-by: Contributor <contributor@testsprite.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…ressions (TestSprite#168) * feat(test): add "test diff <run-a> <run-b>" to isolate run-to-run regressions * test(diff): cover runDiff --dry-run branch (offline canned sample)
… files in --out dir (TestSprite#162) commitBundle's stale-file sweep removed EVERY directory entry not part of the fresh bundle, so 'test failure get --out <dir>' / 'test artifact get --out <dir>' pointed at a pre-existing, populated directory silently deleted the user's unrelated files (exit 0, no warning) — on the very first write, not just re-commits. Scope the sweep to entries the bundle format owns (result.json, failure.json, video.mp4, meta.json, steps, .tmp, .partial, code.<ext>). Stale bundle files are still cleaned — an old video.mp4 when the new bundle has no video, a code.py when the new bundle writes code.ts — but foreign files and directories are never touched. Fixes TestSprite#159 Co-authored-by: Kshitij Bhardwaj <tothemoon202154@outlook.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…al without --all (TestSprite#163) Two silent-footgun gaps in test rerun's flag validation: 1. Explicit test IDs combined with --all silently discarded the listed IDs — the --all branch resolves the full project test set and overwrites them — so 'rerun test_abc --all' dispatched a batch rerun of EVERY test in the project, burning rerun/auto-heal credits with no error. Both siblings already guard this exact ambiguity (test run's positional+--all guard, delete-batch's ids+--all guard). 2. --status <list> and --skip-terminal without --all were silently ignored — including INVALID --status values, which were never validated — while the same misuse of rerun's own --filter (and delete-batch's --status) exits 5. All three narrowing filters now share the same guard. Both reject with VALIDATION_ERROR (exit 5) before any network dispatch. Fixes TestSprite#160 Co-authored-by: Kshitij Bhardwaj <tothemoon202154@outlook.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…e#171) The 'testsprite usage' command help text listed three examples but omitted the --debug flag (a global option useful for diagnosing auth and network issues). It also didn't document the exit codes, which matters for CI/CD scripts that gate on the return value. This PR adds: - A --debug example showing what it traces (HTTP method/path, request id, latency) — useful when debugging 'auth error' or 'transport failure' messages. - An explicit exit-codes section (0 success, 3 auth error, 10 network failure) so users scripting against the CLI know what to expect. Pure documentation improvement — no behavioral change, no new deps. Tested: existing unit tests pass (npm test). Co-authored-by: MOAAMN SAYED <moaamnsayed560@gmail.com>
…t pure) (TestSprite#31) Interactive prompts (`prompt.ts` — the API-key prompt during `setup`/`auth configure`, the target prompt during `agent install`) wrote the question and masking to STDOUT, and the "Configuring profile …" prelude defaulted to stdout too. On the interactive path that mixes UI text into stdout — and under `--output json` it breaks the contract that stdout is a single JSON document, so a consumer doing `JSON.parse(stdout)` fails. Default both to stderr: prompts and informational preludes are interactive UI, not result data. stdout now carries only the command's result (the §8.1 stdout-purity principle the repo already enforces elsewhere). stderr is still the user's TTY, so prompts remain visible; the secret is still never echoed. Callers that inject explicit streams are unaffected. Adds regression tests: promptText writes the question to stderr by default, and the configure prelude lands on stderr (not the result stdout).
* block artifact download redirects * redact artifact download urls --------- Co-authored-by: merlinsantiago982-cmd <merlinsantiago982-cmd@users.noreply.github.com>
* fix(test): validate artifact run id default path * fix(test): reject windows dot artifact run ids * fix(test): reject windows dot-suffix artifact run ids * style(test): format artifact run id guard --------- Co-authored-by: Lexiie <28455136+Lexiie@users.noreply.github.com>
…TestSprite#11) * feat(cli): add runtime Node.js version check with clear error message * refactor(version-guard): extract to a documented module tested against the real implementation
Windsurf (Cascade) reads workspace rules from `.windsurf/rules/*.md`. Add it as an own-file agent target so `testsprite agent install --target windsurf` (and `setup --agent windsurf`) installs the TestSprite skills into a Windsurf project. Reworked onto the v0.2.0 multi-skill agent-targets API (pathFor / SKILLS / DEFAULT_SKILLS). Rule files use Cascade frontmatter with `trigger: model_decision` — the equivalent of the Cursor `.mdc` `alwaysApply: false` mode (description shown up front; full body pulled in on relevance). Budget handling: a `.windsurf/rules/*.md` file caps at ~12 K characters and Cascade silently truncates beyond it, which would cut the full ~22 KB verify skill in half. The windsurf target therefore renders the COMPACT body per skill (new `compactBody` flag + `compactBodyFor`): a skill that ships a trimmed codex asset (`testsprite-verify` → ~5 KB) uses it, while a skill whose codex contribution is only a one-liner (`testsprite-onboard`, ~6.5 KB full) keeps its full body — both land well under the cap. `agent.ts` and `renderForTarget` select the same body so installed bytes match the render. Everything else derives from the TARGETS map automatically (agent list, the setup --agent choices, skill-nudge install detection). Updated the hardcoded help strings, the --help snapshot, the agent-targets/agent unit tests (incl. Cascade-frontmatter and per-skill budget tests), the e2e matrix guards / content-integrity (gated on compactBody), and the README/DOCUMENTATION target lists (incl. the --force own-file list).
… CI proxies (TestSprite#169) * feat(cli): honor HTTPS_PROXY/HTTP_PROXY/NO_PROXY behind corporate and CI proxies * fix(proxy): degrade to default dispatcher when proxy agent init fails instead of crashing startup * fix(proxy): pin undici to ^7.16.0 for Node 20 compatibility (8.x requires Node >=22.19)
…m check, opt-out, CI-safe) (TestSprite#181)
…e#132) * feat(cli): add 'test flaky' repeat-run flaky-test detector * fix(flaky): cap --runs at 10 per maintainer scope (TestSprite#115) Rescope the flaky detector's --runs bound from 1-100 to 1-10 as requested in the TestSprite#115 triage: uncapped FE replays amplify free executions. Updates the MAX_FLAKY_RUNS constant (which drives the validation, error message, and --runs help text), docs, changelog, and the runs-bound tests. Regenerates the help snapshot, which also adds the previously-missing 'test flaky' entry. * test(snapshot): refresh flaky help snapshot after rebase onto main Rebasing onto current main (which added the global --request-timeout option, TestSprite#17) changes the 'Global options' line rendered in the test flaky --help output. Regenerate the snapshot so the help snapshot test stays green on CI. * docs(changelog): resolve leftover merge-conflict markers (keep JUnit + flaky 1-10)
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/commands/test.ts (1)
4520-4562: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winStop fan-out after the first fatal error.
reject(err)still falls through toinFlight--andstartNext(), so a non-NOT_FOUNDfailure can keep launching more/failure/summaryrequests after the operation has already failed. Add an abort guard or return immediately on fatal errors.🤖 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 - 4562, Stop launching more triage requests after a fatal `/failure/summary` error. In the `startNext` flow inside `test.ts`, a non-`NOT_FOUND` failure currently calls `reject(err)` and then still decrements `inFlight` and recurses, so add a fatal-error/aborted guard or return immediately from the `.catch` path after rejecting. Make sure `startNext`, the `inFlight` bookkeeping, and the `triageInputs` fan-out all stop once a real error has been hit.
🧹 Nitpick comments (1)
src/lib/failure-triage.test.ts (1)
213-244: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest doesn't actually distinguish representative-based label from insertion-order label.
Both
t_firstandt_recentuse the identicalsharedHypstring, socluster.label/canonicalRootCausewould equalsharedHypregardless of which member is picked as representative — even under a buggy insertion-order implementation. The test name promises to catch that regression but the assertions can't actually detect it.♻️ Suggested fix: use distinct hypothesis text per member
it('labels hypothesis clusters from the representative test, not map-insertion order', () => { const sharedHyp = 'Shared login validation failure across checkout flows.'; + const olderHyp = 'shared login validation failure across checkout flows.'; // normalized-equal groupKey, distinct display text const result = buildFailureClusters('proj_hyp', [ makeInput({ testId: 't_first', updatedAt: '2026-06-20T00:00:00.000Z', summary: { status: 'failed', failureKind: 'assertion', snapshotId: 'snap_first', - rootCauseHypothesis: sharedHyp, + rootCauseHypothesis: 'Shared login validation failure across checkout flows (older wording).', recommendedFixTarget: null, }, }),Note the raw hypothesis text must still normalize to the same
groupKey(vianormalizeHypothesis) for both members to land in the same cluster, so any replacement text needs to collapse to the same key while differing enough in display casing/whitespace to prove selection-order matters, or simply differ in a way that keeps grouping intact while makinglabel/canonicalRootCausedistinguishable per representative.🤖 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/failure-triage.test.ts` around lines 213 - 244, The cluster-label test in failure-triage.test.ts does not prove representative-based labeling because both inputs share the same hypothesis text, so a buggy insertion-order implementation would still pass. Update the test data in buildFailureClusters to give each member a distinct rootCauseHypothesis that still normalizes to the same groupKey via normalizeHypothesis, then keep the assertions on representativeTestId, label, and canonicalRootCause so the chosen representative actually affects the expected value.
🤖 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 `@src/lib/dry-run/samples.ts`:
- Around line 355-419: The canned dry-run fixtures are using human-readable
cluster IDs instead of the hashed wire-format produced by
buildFailureClusters(). Update the sample data in samples.ts so the clusterId
values follow the cluster_${slug}_${8hex} pattern, and make the corresponding
dry-run test expect the same hashed shape. Use the existing groupKey/fix-target
entries to keep the fixture semantics unchanged while aligning the IDs with the
real response format.
---
Outside diff comments:
In `@src/commands/test.ts`:
- Around line 4520-4562: Stop launching more triage requests after a fatal
`/failure/summary` error. In the `startNext` flow inside `test.ts`, a
non-`NOT_FOUND` failure currently calls `reject(err)` and then still decrements
`inFlight` and recurses, so add a fatal-error/aborted guard or return
immediately from the `.catch` path after rejecting. Make sure `startNext`, the
`inFlight` bookkeeping, and the `triageInputs` fan-out all stop once a real
error has been hit.
---
Nitpick comments:
In `@src/lib/failure-triage.test.ts`:
- Around line 213-244: The cluster-label test in failure-triage.test.ts does not
prove representative-based labeling because both inputs share the same
hypothesis text, so a buggy insertion-order implementation would still pass.
Update the test data in buildFailureClusters to give each member a distinct
rootCauseHypothesis that still normalizes to the same groupKey via
normalizeHypothesis, then keep the assertions on representativeTestId, label,
and canonicalRootCause so the chosen representative actually affects the
expected 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ca395e38-f7b5-4a21-a4f2-3e16c35cdeb5
⛔ Files ignored due to path filters (1)
test/__snapshots__/help.snapshot.test.ts.snapis excluded by!**/*.snap,!**/*.snap
📒 Files selected for processing (10)
CHANGELOG.mdDOCUMENTATION.mdREADME.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
✅ Files skipped from review due to trivial changes (3)
- README.md
- CHANGELOG.md
- DOCUMENTATION.md
🚧 Files skipped from review as they are similar to previous changes (3)
- test/help.snapshot.test.ts
- src/lib/failure-triage.ts
- src/commands/test.test.ts
Comment on lines
+355
to
+419
| 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, | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the real clusterId construction to see if the hash suffix is unconditional.
ast-grep run --pattern 'clusterId: $_' --lang typescript src/lib/failure-triage.ts
rg -n -A5 -B5 'slugifyClusterId' src/lib/failure-triage.tsRepository: TestSprite/testsprite-cli
Length of output: 958
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the relevant implementation and sample lines.
sed -n '215,280p' src/lib/failure-triage.ts
printf '\n--- sample ---\n'
sed -n '355,419p' src/lib/dry-run/samples.ts
printf '\n--- expected cluster ids from group keys ---\n'
python3 - <<'PY'
import hashlib
keys = [
'kind:network_timeout',
'ref:src/components/CheckoutForm.tsx:412',
]
for key in keys:
slug = ''.join(ch if ch.isalnum() else '_' for ch in key.lower())
while '__' in slug:
slug = slug.replace('__', '_')
slug = slug.strip('_')
h = hashlib.sha256(key.encode()).hexdigest()[:8]
print(key, '=>', f'cluster_{slug}_{h}')
PYRepository: TestSprite/testsprite-cli
Length of output: 5111
Align dry-run cluster IDs with the hashed wire format
buildFailureClusters() always emits cluster_${slug}_${8hex}, but these canned clusters still use unhashed IDs. Update the sample (and the matching dry-run test) so --dry-run matches the real response shape.
🤖 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 355 - 419, The canned dry-run
fixtures are using human-readable cluster IDs instead of the hashed wire-format
produced by buildFailureClusters(). Update the sample data in samples.ts so the
clusterId values follow the cluster_${slug}_${8hex} pattern, and make the
corresponding dry-run test expect the same hashed shape. Use the existing
groupKey/fix-target entries to keep the fixture semantics unchanged while
aligning the IDs with the real response format.
Source: Path instructions
Contributor
|
Hi @SahilRakhaiya05 — so sorry for the disruption here. 🙏 While we were publishing a new CLI release, a hiccup in our process unintentionally closed this PR. To be clear: your contribution wasn't rejected, and nothing on your end caused this. Your work is completely safe — your branch and every commit are intact and untouched, and Because of how the closure happened, the cleanest path is a fresh PR from your existing branch (rather than reopening this one) — and opening it yourself keeps it under your name, with full credit for your work: 👉 Open a new PR from your branch It should merge cleanly. So your work doesn't get lost, if we don't hear back in the next ~3 days we'll go ahead and open it for you — but opening it yourself keeps it under your name, and either way we're always happy to hand it back to you. Thank you for contributing to TestSprite, and again, we're sorry for the hassle. We really appreciate this work and want to see it merged. 🙌 |
9 tasks
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
19 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 triagecommand to group failed tests into root-cause clusters and show a representative test, affected test IDs, confidence, and fix priority.Documentation