Fix for stalling relative scoring

jmgasper · jmgasper · commit a338dd9e7cae · 2026-07-03T16:26:29.000+10:00
diff --git a/README.md b/README.md
@@ -300,7 +300,7 @@ Config identity semantics:
 - `POST /challenge/:challengeId` validates that the challenge exists in challenge-api and that `reviewScorecardId` resolves in review-api before persisting. If a config already exists for the challenge, the API returns `409 Conflict`.
 - `phaseConfig.phaseId` stores the canonical challenge phase definition id from challenge-api `phases[].phaseId`. Create/update requests also accept the challenge-phase row `id` and normalize it before persistence for backwards compatibility.
 - `reviewScorecardId` can be either the current review-api scorecard id or a legacy id; scoring callback processing resolves it to the canonical scorecard id before posting review summations.
-- When `relativeScoringEnabled = true`, review scores are recomputed from per-test raw scores against the best score currently held by the latest submission from each member, so final review summation scores stay within `0..100` and can change as new submissions arrive. The raw callback result is stored only as a pending placeholder until the recompute worker writes the normalized score; queued recompute passes also finalize visible pending placeholders that are not selected as the latest member submission.
+- When `relativeScoringEnabled = true`, review scores are recomputed from per-test raw scores against the best score currently held by the latest submission from each member, so final review summation scores stay within `0..100` and can change as new submissions arrive. The raw callback result is stored only as a pending placeholder until the recompute worker writes the normalized score; queued recompute passes also finalize visible pending placeholders that are not selected as the latest member submission, and fetch the queued callback submission's review summation directly from Review API.
 
 Important runtime behavior:
 
diff --git a/docs/review-phase-scoring.md b/docs/review-phase-scoring.md
@@ -84,7 +84,7 @@ The queue is keyed by challenge ID, review ID, and submission ID so repeated pha
 
 ## Relative scoring at completion
 
-If `relativeScoringEnabled = true`, `ScoringResultService` persists the raw callback result as a pending, non-passing placeholder and queues a pg-boss `relative-scoring-recompute` job. The worker recalculates normalized aggregate scores under a challenge/phase PostgreSQL advisory lock, updates Review API, and then completes matching SYSTEM reviews with the recomputed scores. Each worker pass also finalizes any persisted `relativeScoringPending` placeholders it can see, even when a placeholder is not the selected latest submission for that member. `ChallengeCompletionService.finalizeChallenge(...)` consumes those persisted review summaries; it does not recompute relative scoring itself.
+If `relativeScoringEnabled = true`, `ScoringResultService` persists the raw callback result as a pending, non-passing placeholder and queues a pg-boss `relative-scoring-recompute` job. The worker recalculates normalized aggregate scores under a challenge/phase PostgreSQL advisory lock, updates Review API, and then completes matching SYSTEM reviews with the recomputed scores. Each worker pass also finalizes any persisted `relativeScoringPending` placeholders it can see, even when a placeholder is not the selected latest submission for that member; the queued callback submission's review summation is fetched directly from Review API so reruns omitted from the challenge submission list still complete. `ChallengeCompletionService.finalizeChallenge(...)` consumes those persisted review summaries; it does not recompute relative scoring itself.
 
 The recompute queue debounces SYSTEM bursts by challenge and phase so simultaneous scorer callbacks do not all run the expensive fan-out. Retry behavior is controlled by `RELATIVE_SCORING_RECOMPUTE_RETRY_DELAY_SECONDS` (default `60`), `RELATIVE_SCORING_RECOMPUTE_RETRY_LIMIT` (default `1000`), `RELATIVE_SCORING_RECOMPUTE_DEBOUNCE_SECONDS` (default `15`), `RELATIVE_SCORING_RECOMPUTE_START_DELAY_SECONDS` (default `10`), and `RELATIVE_SCORING_RECOMPUTE_WORKER_CONCURRENCY` (default `1`).
 
diff --git a/docs/submission-phase-scoring.md b/docs/submission-phase-scoring.md
@@ -112,7 +112,7 @@ Relative scoring applies when:
 - `relativeScoringEnabled = true` on the Marathon Match config
 - `testScores` are present in the scorer metadata
 
-In that case, `ScoringResultService` writes the raw callback result as a pending, non-passing placeholder, then queues a pg-boss relative scoring recomputation. The worker recalculates latest-submission review scores relative to the current best result, updates the persisted aggregate scores, and keeps existing `reviewedDate` values so relative-score updates do not move historical review timestamps. A worker pass also finalizes visible `relativeScoringPending` placeholders that are not selected as the latest member submission, so completed reruns do not remain in progress.
+In that case, `ScoringResultService` writes the raw callback result as a pending, non-passing placeholder, then queues a pg-boss relative scoring recomputation. The worker recalculates latest-submission review scores relative to the current best result, updates the persisted aggregate scores, and keeps existing `reviewedDate` values so relative-score updates do not move historical review timestamps. A worker pass also finalizes visible `relativeScoringPending` placeholders that are not selected as the latest member submission, and fetches the queued callback submission's review summation directly from Review API so completed reruns do not remain in progress.
 
 ## Tester-change rerun
 
diff --git a/src/api/scoring-result/scoring-result.service.spec.ts b/src/api/scoring-result/scoring-result.service.spec.ts
@@ -943,6 +943,122 @@ describe('ScoringResultService', () => {
     );
   });
 
+  it('loads the queued submission pending relative placeholder directly when submissions omit it', async () => {
+    const { service, m2mService, prisma } = createService();
+
+    prisma.marathonMatchConfig.findUnique.mockResolvedValue({
+      challengeId: basePayload.challengeId,
+      name: 'Blocks',
+      submissionApiUrl: 'https://api.topcoder-dev.com/v6',
+      relativeScoringEnabled: true,
+      scoreDirection: ScoreDirection.MAXIMIZE,
+    });
+    m2mService.getM2MToken.mockResolvedValue('m2m-token');
+
+    jest
+      .spyOn(service as any, 'withRelativeScoringLock')
+      .mockImplementation(
+        async (
+          _challengeId: string,
+          _testPhase: string,
+          work: () => Promise<unknown>,
+        ) => work(),
+      );
+    jest
+      .spyOn(service as any, 'resolveScorecardId')
+      .mockResolvedValue('scorecard-1');
+    jest.spyOn(service as any, 'fetchChallengeSubmissions').mockResolvedValue([
+      {
+        id: 'submission-visible-latest',
+        memberId: 'member-visible',
+        isLatest: true,
+        submittedDate: '2026-05-01T00:00:02.000Z',
+        reviewSummation: [
+          {
+            id: 'summation-visible-latest',
+            aggregateScore: 100,
+            isFinal: true,
+            metadata: {
+              testScores: [{ testcase: '753388858', score: 100 }],
+              testStatus: ScoringTestStatus.Success,
+              testType: 'system',
+            },
+            scorecardId: 'scorecard-1',
+          },
+        ],
+      },
+    ]);
+    jest
+      .spyOn(service as any, 'findExistingReviewSummations')
+      .mockResolvedValue([
+        {
+          id: 'summation-hidden-pending',
+          aggregateScore: 0,
+          isFinal: true,
+          metadata: {
+            relativeScoringPending: true,
+            testScores: [{ testcase: '753388858', score: 50 }],
+            testStatus: ScoringTestStatus.InProgress,
+            testType: 'system',
+          },
+          scorecardId: 'scorecard-1',
+        },
+      ]);
+    const upsertReviewSummationSpy = jest
+      .spyOn(service as any, 'upsertReviewSummation')
+      .mockResolvedValue(undefined);
+    const completeSystemReviewIfNeededSpy = jest
+      .spyOn(service as any, 'completeSystemReviewIfNeeded')
+      .mockResolvedValue(undefined);
+    jest
+      .spyOn(service as any, 'notifyScoringCompletionEmailIfReady')
+      .mockResolvedValue(undefined);
+
+    await expect(
+      service.recomputeQueuedRelativeScoring({
+        challengeId: basePayload.challengeId,
+        submissionId: 'submission-hidden-pending',
+        reviewId: 'review-hidden-pending',
+        testPhase: 'system',
+        reviewTypeId: basePayload.reviewTypeId,
+        scorecardId: 'scorecard-1',
+        queuedAt: '2026-05-01T00:00:10.000Z',
+      }),
+    ).resolves.toBe(undefined);
+
+    expect(upsertReviewSummationSpy).toHaveBeenCalledWith(
+      'm2m-token',
+      'system',
+      expect.objectContaining({
+        aggregateScore: 50,
+        isPassing: true,
+        submissionId: 'submission-hidden-pending',
+        metadata: expect.objectContaining({
+          testProgress: 1,
+          testStatus: ScoringTestStatus.Success,
+        }),
+      }),
+      'summation-hidden-pending',
+    );
+    const hiddenPayload = upsertReviewSummationSpy.mock.calls.find(
+      ([, , payload]) =>
+        (payload as { submissionId: string }).submissionId ===
+        'submission-hidden-pending',
+    )?.[2] as { metadata?: Record<string, unknown> } | undefined;
+    expect(hiddenPayload?.metadata).not.toHaveProperty(
+      'relativeScoringPending',
+    );
+    expect(completeSystemReviewIfNeededSpy).toHaveBeenCalledWith(
+      'm2m-token',
+      'review-hidden-pending',
+      50,
+      'system',
+      expect.objectContaining({
+        submissionId: 'submission-hidden-pending',
+      }),
+    );
+  });
+
   it('serializes concurrent relative scoring recomputations for the same challenge phase', async () => {
     const { service, m2mService, prisma } = createService();
     const firstPayload: ScoringResultCallbackPayload = {
diff --git a/src/api/scoring-result/scoring-result.service.ts b/src/api/scoring-result/scoring-result.service.ts
@@ -1225,6 +1225,7 @@ export class ScoringResultService {
           reviewTypeId,
           fallbackScorecardId,
           lockedSettings,
+          submissionId,
         ),
     );
 
@@ -1352,6 +1353,8 @@ export class ScoringResultService {
    * @param reviewTypeId Review type identifier to preserve in normalized metadata.
    * @param fallbackScorecardId Scorecard ID resolved from callback/config data.
    * @param settings Relative scoring configuration for the challenge.
+   * @param callbackSubmissionId Submission from the queued callback; its pending
+   * summation is fetched directly so reruns for non-listed submissions finalize.
    * @returns Recomputed relative review payloads that were written.
    * @throws Error when submission-api or review-api calls fail.
    */
@@ -1361,6 +1364,7 @@ export class ScoringResultService {
     reviewTypeId: string,
     fallbackScorecardId: string | undefined,
     settings: Required<RelativeScoringSettings>,
+    callbackSubmissionId?: string,
   ): Promise<RelativeReviewPayload[]> {
     const submissions = await this.fetchChallengeSubmissions(
       token,
@@ -1379,10 +1383,27 @@ export class ScoringResultService {
       testPhase,
       reviewTypeId,
     );
-    const reviewRecords = this.mergeRelativeReviewRecords(
+    const visibleReviewRecords = this.mergeRelativeReviewRecords(
       latestReviewRecords,
       pendingReviewRecords,
     );
+    const callbackPendingReviewRecords =
+      this.hasRelativeReviewRecordForSubmission(
+        visibleReviewRecords,
+        callbackSubmissionId,
+      )
+        ? []
+        : await this.fetchPendingRelativeReviewRecordsForSubmission(
+            token,
+            submissions,
+            testPhase,
+            reviewTypeId,
+            callbackSubmissionId,
+          );
+    const reviewRecords = this.mergeRelativeReviewRecords(
+      visibleReviewRecords,
+      callbackPendingReviewRecords,
+    );
 
     if (reviewRecords.length === 0) {
       this.logger.warn({
@@ -2663,6 +2684,87 @@ export class ScoringResultService {
     return pendingRecords;
   }
 
+  /**
+   * Fetches pending relative-scoring placeholders for the queued callback
+   * submission directly from Review API. This covers reruns whose submission is
+   * not present, or not expanded with metadata, in the challenge submission list.
+   * @param token M2M token for review-api.
+   * @param challengeSubmissions Submission records already fetched for context.
+   * @param testPhase Requested scoring phase.
+   * @param reviewTypeId Review type identifier to preserve in normalized metadata.
+   * @param callbackSubmissionId Submission ID carried by the recompute job.
+   * @returns Recomputable pending review records for the callback submission.
+   */
+  private async fetchPendingRelativeReviewRecordsForSubmission(
+    token: string,
+    challengeSubmissions: Record<string, unknown>[],
+    testPhase: string,
+    reviewTypeId: string,
+    callbackSubmissionId?: string,
+  ): Promise<RelativeReviewRecord[]> {
+    const submissionId = this.asString(callbackSubmissionId);
+    if (!submissionId) {
+      return [];
+    }
+
+    const submission = challengeSubmissions.find(
+      (entry) => this.extractSubmissionId(entry) === submissionId,
+    );
+    const reviewSummations = await this.findExistingReviewSummations(
+      token,
+      submissionId,
+      testPhase,
+    );
+    const pendingRecords: RelativeReviewRecord[] = [];
+
+    for (const reviewObject of reviewSummations) {
+      const metadata = this.asRecord(reviewObject.metadata);
+      if (this.parseBooleanFlag(metadata.relativeScoringPending) !== true) {
+        continue;
+      }
+
+      const pendingRecord = this.buildRelativeReviewRecord({
+        createdAt: submission
+          ? this.resolveSubmissionDate(submission)
+          : undefined,
+        memberKey: submission
+          ? (this.extractSubmissionMemberKey(submission) ??
+            `submission:${submissionId}`)
+          : `submission:${submissionId}`,
+        reviewObject,
+        reviewTypeId,
+        submissionId,
+        testPhase,
+      });
+
+      if (pendingRecord) {
+        pendingRecords.push(pendingRecord);
+      }
+    }
+
+    return pendingRecords;
+  }
+
+  /**
+   * Checks whether a recompute record set already includes one submission.
+   * @param reviewRecords Candidate relative review records.
+   * @param submissionId Submission ID to find.
+   * @returns True when the submission is already represented.
+   */
+  private hasRelativeReviewRecordForSubmission(
+    reviewRecords: RelativeReviewRecord[],
+    submissionId?: string,
+  ): boolean {
+    const normalizedSubmissionId = this.asString(submissionId);
+    if (!normalizedSubmissionId) {
+      return false;
+    }
+
+    return reviewRecords.some(
+      (reviewRecord) => reviewRecord.submissionId === normalizedSubmissionId,
+    );
+  }
+
   /**
    * Merges relative review records by persisted review summation id, falling back
    * to submission id for records that do not expose an id.
