fix: do not accidentally throw off serve results when test repairs fail

We accidentally regressed when we introduced the test command feature.
See the rationale we had back then (shortly before we landed the test
command feature): angular#69

Author: devversion

runner/eval-cli.ts

@@ -42,6 +42,7 @@ interface Options {
   skipLighthouse?: boolean;
   maxTestRepairAttempts?: number;
   maxBuildRepairAttempts?: number;
+  preserveBreakingTestRepairAttempts?: boolean;
 }
 
 function builder(argv: Argv): Argv<Options> {
@@ -168,6 +169,13 @@ function builder(argv: Argv): Argv<Options> {
         description:
           'Number of repair attempts for discovered test failures (including a11y violations and ones from testCommand)',
       })
+      .option('preserve-breaking-test-repair-attempts', {
+        type: 'boolean',
+        // See rationale for the default via:
+        // https://github.com/angular/web-codegen-scorer/pull/69
+        default: false,
+        description: `Whether test repair attempts which break a build should be captured.`,
+      })
       .strict()
       .version(false)
       .help()
@@ -221,6 +229,7 @@ async function handler(cliArgs: Arguments<Options>): Promise<void> {
       skipLighthouse: cliArgs.skipLighthouse,
       maxBuildRepairAttempts: cliArgs.maxBuildRepairAttempts,
       maxTestRepairAttempts: cliArgs.maxTestRepairAttempts,
+      preserveBreakingTestRepairAttempts: cliArgs.preserveBreakingTestRepairAttempts,
       abortSignal: abortCtrl.signal,
     });
 

runner/orchestration/build-serve-test-loop.ts

@@ -218,8 +218,14 @@ export async function attemptBuildAndTest(
 
     let hasBuildFailure = attempt.buildResult.status !== BuildResultStatus.SUCCESS;
     attempt.buildFailedDuringTestRepair = hasBuildFailure;
-    attemptDetails.push(attempt);
-    lastAttempt = attempt;
+
+    // By default, we don't preserve breaking test repair attempts as they significantly
+    // impact evaluation results by e.g. lacking serve results.
+    // TODO: In the future we should consider exploring this further, or at least capture tokens.
+    if (!hasBuildFailure || config.preserveBreakingTestRepairAttempts) {
+      attemptDetails.push(attempt);
+      lastAttempt = attempt;
+    }
     // If we somehow introduced build errors via the repair loop, we abort
     // further repairs and capture the failed build. This is useful insight
     // as LLMs seem to regress when asked to repair violations.

runner/shared-interfaces.ts

@@ -30,6 +30,7 @@ export interface AssessmentConfig {
   skipLighthouse?: boolean;
   maxTestRepairAttempts?: number;
   maxBuildRepairAttempts?: number;
+  preserveBreakingTestRepairAttempts?: boolean;
   abortSignal?: AbortSignal;
 }