Skip to content

Commit cd755ad

Browse files
fix(test): address CodeRabbit review on failure triage
- 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
1 parent 9f7c9e4 commit cd755ad

5 files changed

Lines changed: 190 additions & 86 deletions

File tree

src/commands/test.ts

Lines changed: 5 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import {
1717
writeBundle,
1818
type WriteBundleResult,
1919
} from '../lib/bundle.js';
20-
import { findSample } from '../lib/dry-run/samples.js';
20+
import { findSample, sampleFailureTriageResult } from '../lib/dry-run/samples.js';
2121
import {
2222
ApiError,
2323
CLIError,
@@ -4065,78 +4065,7 @@ export async function runFailureTriage(
40654065
const stderrFn = deps.stderr ?? ((line: string) => process.stderr.write(`${line}\n`));
40664066

40674067
if (opts.dryRun) {
4068-
const dryRunResult: FailureTriageResult = {
4069-
projectId: opts.projectId,
4070-
clusters: [
4071-
{
4072-
clusterId: 'cluster_kind_network_timeout',
4073-
label: 'Environment issue (network_timeout)',
4074-
groupKey: 'kind:network_timeout',
4075-
groupReason: 'failure_kind',
4076-
failureKind: 'network_timeout',
4077-
representativeTestId: 'test_dryrun_a',
4078-
memberTestIds: ['test_dryrun_a', 'test_dryrun_b'],
4079-
members: [
4080-
{
4081-
testId: 'test_dryrun_a',
4082-
testName: 'Dry-run checkout flow',
4083-
testType: 'frontend',
4084-
updatedAt: '2026-06-26T12:00:00.000Z',
4085-
status: 'failed',
4086-
failureKind: 'network_timeout',
4087-
snapshotId: 'snap_dryrun_a',
4088-
rootCauseHypothesis: null,
4089-
recommendedFixTarget: null,
4090-
},
4091-
{
4092-
testId: 'test_dryrun_b',
4093-
testName: 'Dry-run profile update',
4094-
testType: 'frontend',
4095-
updatedAt: '2026-06-26T12:01:00.000Z',
4096-
status: 'failed',
4097-
failureKind: 'network_timeout',
4098-
snapshotId: 'snap_dryrun_b',
4099-
rootCauseHypothesis: null,
4100-
recommendedFixTarget: null,
4101-
},
4102-
],
4103-
canonicalRootCause: null,
4104-
confidence: 0.88,
4105-
fixPriority: 1,
4106-
},
4107-
{
4108-
clusterId: 'cluster_ref_src_components_checkoutform_tsx_412',
4109-
label: 'Shared fix target: src/components/CheckoutForm.tsx:412',
4110-
groupKey: 'ref:src/components/CheckoutForm.tsx:412',
4111-
groupReason: 'fix_target',
4112-
failureKind: 'assertion',
4113-
representativeTestId: 'test_dryrun_c',
4114-
memberTestIds: ['test_dryrun_c'],
4115-
members: [
4116-
{
4117-
testId: 'test_dryrun_c',
4118-
testName: 'Dry-run submit checkout',
4119-
testType: 'frontend',
4120-
updatedAt: '2026-06-26T12:02:00.000Z',
4121-
status: 'failed',
4122-
failureKind: 'assertion',
4123-
snapshotId: 'snap_dryrun_c',
4124-
rootCauseHypothesis:
4125-
'Submit button is disabled because the credit-card field is empty.',
4126-
recommendedFixTarget: {
4127-
kind: 'code',
4128-
reference: 'src/components/CheckoutForm.tsx:412',
4129-
rationale: 'Disabled state originates from `isFormValid()`.',
4130-
},
4131-
},
4132-
],
4133-
canonicalRootCause: 'Submit button is disabled because the credit-card field is empty.',
4134-
confidence: 0.7,
4135-
fixPriority: 3,
4136-
},
4137-
],
4138-
summary: { totalFailed: 3, clusterCount: 2, skipped: 0 },
4139-
};
4068+
const dryRunResult = sampleFailureTriageResult(opts.projectId);
41404069
out.print(dryRunResult, data => renderFailureTriageText(data as FailureTriageResult));
41414070
return dryRunResult;
41424071
}
@@ -8922,7 +8851,7 @@ function createTestFailureCommand(deps: TestDeps): Command {
89228851
.description(
89238852
'Group all failed tests in a project into root-cause clusters (lightweight summary fan-out — no bundle downloads)',
89248853
)
8925-
.requiredOption('--project <id>', 'project id (returned by `testsprite project list`)')
8854+
.option('--project <id>', 'project id (returned by `testsprite project list`)')
89268855
.option('--type <type>', 'filter by test type (frontend|backend)')
89278856
.option(
89288857
'--filter <substr>',
@@ -8950,13 +8879,13 @@ function createTestFailureCommand(deps: TestDeps): Command {
89508879
)
89518880
.action(
89528881
async (
8953-
cmdOpts: { project: string; type?: string; filter?: string; maxConcurrency?: string },
8882+
cmdOpts: { project?: string; type?: string; filter?: string; maxConcurrency?: string },
89548883
command: Command,
89558884
) => {
89568885
await runFailureTriage(
89578886
{
89588887
...resolveCommonOptions(command),
8959-
projectId: cmdOpts.project,
8888+
projectId: cmdOpts.project ?? '',
89608889
type: parseEnumFlag(cmdOpts.type, 'type', TEST_TYPES),
89618890
nameFilter: cmdOpts.filter,
89628891
maxConcurrency:

src/lib/dry-run/samples.test.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,25 @@
11
import { describe, expect, it } from 'vitest';
2-
import { DRY_RUN_SAMPLE_ENTRIES, findSample } from './samples.js';
2+
import { DRY_RUN_SAMPLE_ENTRIES, findSample, sampleFailureTriageResult } from './samples.js';
3+
4+
describe('sampleFailureTriageResult', () => {
5+
it('returns a FailureTriageResult envelope wired for --dry-run', () => {
6+
const result = sampleFailureTriageResult('proj_dry');
7+
expect(result.projectId).toBe('proj_dry');
8+
expect(result.summary).toEqual({ totalFailed: 3, clusterCount: 2, skipped: 0 });
9+
expect(result.clusters).toHaveLength(2);
10+
expect(result.clusters[0]).toMatchObject({
11+
groupReason: 'failure_kind',
12+
representativeTestId: expect.any(String),
13+
memberTestIds: expect.any(Array),
14+
confidence: expect.any(Number),
15+
fixPriority: expect.any(Number),
16+
});
17+
expect(result.clusters[1]).toMatchObject({
18+
groupReason: 'fix_target',
19+
label: expect.stringContaining('Shared fix target:'),
20+
});
21+
});
22+
});
323

424
describe('findSample', () => {
525
it('resolves /me', () => {

src/lib/dry-run/samples.ts

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import type {
2727
CliTestStep,
2828
} from '../../commands/test.js';
2929
import type { MeResponse } from '../../commands/auth.js';
30+
import type { FailureTriageResult } from '../failure-triage.js';
3031
import type { Page } from '../pagination.js';
3132
import type {
3233
TriggerRunResponse,
@@ -309,6 +310,86 @@ const failureSummary: CliFailureSummary = {
309310
recommendedFixTarget: failureContext.failure.recommendedFixTarget,
310311
};
311312

313+
/**
314+
* Canned response for `test failure triage --dry-run`. Client-side
315+
* orchestration command — not a single backend endpoint — so the full
316+
* clustered envelope lives here alongside the per-route samples above.
317+
*/
318+
export function sampleFailureTriageResult(projectId: string): FailureTriageResult {
319+
return {
320+
projectId,
321+
clusters: [
322+
{
323+
clusterId: 'cluster_kind_network_timeout',
324+
label: 'Environment issue (network_timeout)',
325+
groupKey: 'kind:network_timeout',
326+
groupReason: 'failure_kind',
327+
failureKind: 'network_timeout',
328+
representativeTestId: 'test_dryrun_a',
329+
memberTestIds: ['test_dryrun_a', 'test_dryrun_b'],
330+
members: [
331+
{
332+
testId: 'test_dryrun_a',
333+
testName: 'Dry-run checkout flow',
334+
testType: 'frontend',
335+
updatedAt: '2026-06-26T12:00:00.000Z',
336+
status: 'failed',
337+
failureKind: 'network_timeout',
338+
snapshotId: 'snap_dryrun_a',
339+
rootCauseHypothesis: null,
340+
recommendedFixTarget: null,
341+
},
342+
{
343+
testId: 'test_dryrun_b',
344+
testName: 'Dry-run profile update',
345+
testType: 'frontend',
346+
updatedAt: '2026-06-26T12:01:00.000Z',
347+
status: 'failed',
348+
failureKind: 'network_timeout',
349+
snapshotId: 'snap_dryrun_b',
350+
rootCauseHypothesis: null,
351+
recommendedFixTarget: null,
352+
},
353+
],
354+
canonicalRootCause: null,
355+
confidence: 0.88,
356+
fixPriority: 1,
357+
},
358+
{
359+
clusterId: 'cluster_ref_src_components_checkoutform_tsx_412',
360+
label: 'Shared fix target: src/components/CheckoutForm.tsx:412',
361+
groupKey: 'ref:src/components/CheckoutForm.tsx:412',
362+
groupReason: 'fix_target',
363+
failureKind: 'assertion',
364+
representativeTestId: 'test_dryrun_c',
365+
memberTestIds: ['test_dryrun_c'],
366+
members: [
367+
{
368+
testId: 'test_dryrun_c',
369+
testName: 'Dry-run submit checkout',
370+
testType: 'frontend',
371+
updatedAt: '2026-06-26T12:02:00.000Z',
372+
status: 'failed',
373+
failureKind: 'assertion',
374+
snapshotId: 'snap_dryrun_c',
375+
rootCauseHypothesis:
376+
'Submit button is disabled because the credit-card field is empty.',
377+
recommendedFixTarget: {
378+
kind: 'code',
379+
reference: 'src/components/CheckoutForm.tsx:412',
380+
rationale: 'Disabled state originates from `isFormValid()`.',
381+
},
382+
},
383+
],
384+
canonicalRootCause: 'Submit button is disabled because the credit-card field is empty.',
385+
confidence: 0.7,
386+
fixPriority: 3,
387+
},
388+
],
389+
summary: { totalFailed: 3, clusterCount: 2, skipped: 0 },
390+
};
391+
}
392+
312393
/**
313394
* Dry-run sample lookup keyed by OpenAPI operationId. Order matters in
314395
* {@link findSample}: more specific patterns must precede their generic

src/lib/failure-triage.test.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,4 +209,76 @@ describe('buildFailureClusters', () => {
209209
expect(result.clusters).toEqual([]);
210210
expect(result.summary.totalFailed).toBe(0);
211211
});
212+
213+
it('labels hypothesis clusters from the representative test, not map-insertion order', () => {
214+
const sharedHyp = 'Shared login validation failure across checkout flows.';
215+
const result = buildFailureClusters('proj_hyp', [
216+
makeInput({
217+
testId: 't_first',
218+
updatedAt: '2026-06-20T00:00:00.000Z',
219+
summary: {
220+
status: 'failed',
221+
failureKind: 'assertion',
222+
snapshotId: 'snap_first',
223+
rootCauseHypothesis: sharedHyp,
224+
recommendedFixTarget: null,
225+
},
226+
}),
227+
makeInput({
228+
testId: 't_recent',
229+
updatedAt: '2026-06-26T00:00:00.000Z',
230+
summary: {
231+
status: 'failed',
232+
failureKind: 'assertion',
233+
snapshotId: 'snap_recent',
234+
rootCauseHypothesis: sharedHyp,
235+
recommendedFixTarget: null,
236+
},
237+
}),
238+
]);
239+
240+
const cluster = result.clusters[0]!;
241+
expect(cluster.representativeTestId).toBe('t_recent');
242+
expect(cluster.label).toBe(sharedHyp);
243+
expect(cluster.canonicalRootCause).toBe(sharedHyp);
244+
});
245+
246+
it('assigns distinct clusterIds when slug prefixes would otherwise collide', () => {
247+
const longRef = `${'a'.repeat(50)}`;
248+
const result = buildFailureClusters('proj_collide', [
249+
makeInput({
250+
testId: 't_one',
251+
summary: {
252+
status: 'failed',
253+
failureKind: 'assertion',
254+
snapshotId: 'snap_one',
255+
rootCauseHypothesis: null,
256+
recommendedFixTarget: {
257+
kind: 'code',
258+
reference: `${longRef}_one`,
259+
rationale: 'First target',
260+
},
261+
},
262+
}),
263+
makeInput({
264+
testId: 't_two',
265+
summary: {
266+
status: 'failed',
267+
failureKind: 'assertion',
268+
snapshotId: 'snap_two',
269+
rootCauseHypothesis: null,
270+
recommendedFixTarget: {
271+
kind: 'code',
272+
reference: `${longRef}_two`,
273+
rationale: 'Second target',
274+
},
275+
},
276+
}),
277+
]);
278+
279+
expect(result.clusters).toHaveLength(2);
280+
expect(result.clusters[0]!.clusterId).not.toBe(result.clusters[1]!.clusterId);
281+
expect(result.clusters[0]!.clusterId).toMatch(/_[0-9a-f]{8}$/);
282+
expect(result.clusters[1]!.clusterId).toMatch(/_[0-9a-f]{8}$/);
283+
});
212284
});

src/lib/failure-triage.ts

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { createHash } from 'node:crypto';
2+
13
/**
24
* Client-side failure triage — groups per-test failure summaries into
35
* root-cause clusters using deterministic heuristics over existing
@@ -190,11 +192,10 @@ export function computeFixPriority(
190192

191193
function buildClusterLabel(
192194
groupReason: FailureTriageGroupReason,
193-
members: FailureTriageMember[],
195+
representative: FailureTriageMember,
194196
failureKind: string | null,
195197
): string {
196-
const rep = members.find(m => m.recommendedFixTarget?.reference) ?? members[0]!;
197-
const ref = rep.recommendedFixTarget?.reference;
198+
const ref = representative.recommendedFixTarget?.reference;
198199

199200
if (groupReason === 'fix_target' && ref) {
200201
return `Shared fix target: ${ref}`;
@@ -203,21 +204,22 @@ function buildClusterLabel(
203204
return `Environment issue (${failureKind})`;
204205
}
205206
if (groupReason === 'hypothesis') {
206-
const hyp = rep.rootCauseHypothesis;
207+
const hyp = representative.rootCauseHypothesis;
207208
if (hyp) {
208209
return hyp.length > 80 ? `${hyp.slice(0, 77)}…` : hyp;
209210
}
210211
}
211-
return `Independent failure: ${rep.testName}`;
212+
return `Independent failure: ${representative.testName}`;
212213
}
213214

214215
function slugifyClusterId(groupKey: string): string {
215216
const slug = groupKey
216217
.toLowerCase()
217218
.replace(/[^a-z0-9]+/g, '_')
218-
.replace(/^_|_$/g, '')
219-
.slice(0, 48);
220-
return slug.length > 0 ? slug : 'unknown';
219+
.replace(/^_|_$/g, '');
220+
const hash = createHash('sha256').update(groupKey).digest('hex').slice(0, 8);
221+
const prefix = slug.length > 0 ? slug.slice(0, 48) : 'unknown';
222+
return `${prefix}_${hash}`;
221223
}
222224

223225
function toMember(input: FailureTriageInput): FailureTriageMember {
@@ -267,7 +269,7 @@ export function buildFailureClusters(
267269

268270
clusters.push({
269271
clusterId: `cluster_${slugifyClusterId(groupKey)}`,
270-
label: buildClusterLabel(reason, members, failureKind),
272+
label: buildClusterLabel(reason, rep, failureKind),
271273
groupKey,
272274
groupReason: reason,
273275
failureKind,

0 commit comments

Comments
 (0)