Merge pull request #419 from topcoder-platform/develop

jmgasper · web-flow · commit bf5b10ba9420 · 2026-07-03T17:08:02.000+10:00
Updates for relative scoring score hydration
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, and fetch the queued callback submission's review summation directly from Review API.
+- 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. Recompute uses submission-api to choose the latest submission per member, hydrates phase review summation metadata directly from Review API, finalizes pending placeholders it can see, and retries instead of writing a normalized score if no latest-submission baseline with usable raw `testScores` is available.
 
 Important runtime behavior:
 
diff --git a/docs/Marathon Match Setup and Management.md b/docs/Marathon Match Setup and Management.md
@@ -187,7 +187,7 @@ Relative scoring applies when:
 - `Relative Scoring` is enabled on the scorer config.
 - The ECS runner callback metadata includes `testScores`.
 
-For each testcase, the API finds the best raw score among the latest scored submission for each member. `scoreDirection` controls whether the best raw score is the maximum or minimum value. The stored relative testcase score is normalized to `0..100`, with zero raw or zero best scores counted as `0` for `MAXIMIZE` challenges. For `MINIMIZE` challenges, a raw score of `0` tied with the best score of `0` receives `100`. The aggregate review score is the average of those relative testcase scores.
+For each testcase, the API finds the best raw score among the latest submissions for members whose latest submission has usable raw score metadata for the scoring phase. `scoreDirection` controls whether the best raw score is the maximum or minimum value. The stored relative testcase score is normalized to `0..100`, with zero raw or zero best scores counted as `0` for `MAXIMIZE` challenges. For `MINIMIZE` challenges, a raw score of `0` tied with the best score of `0` receives `100`. The aggregate review score is the average of those relative testcase scores.
 
 Implications:
 
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; 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.
+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. It uses submission-api to select the latest submission for each member, then hydrates phase review summation metadata directly from Review API so incomplete embedded `reviewSummation` expansions do not shrink the baseline set. 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. If no latest-submission baseline has usable raw `testScores`, the job retries instead of writing a self-normalized score. `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, and fetches the queued callback submission's review summation directly from Review API 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 selects the latest submission for each member from submission-api, hydrates the phase review summation metadata directly from Review API, recalculates review scores relative to the current best raw testcase results, 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. If no latest-submission baseline has usable raw `testScores`, the job retries instead of writing a self-normalized score.
 
 ## 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
@@ -198,6 +198,7 @@ describe('ScoringResultService', () => {
         challengeId: true,
         name: true,
         submissionApiUrl: true,
+        reviewScorecardId: true,
         relativeScoringEnabled: true,
         scoreDirection: true,
       },
@@ -500,6 +501,22 @@ describe('ScoringResultService', () => {
         reviewSummation: [],
       },
     ]);
+    jest
+      .spyOn(service as any, 'fetchChallengeReviewSummations')
+      .mockResolvedValue([
+        {
+          id: 'summation-2',
+          isProvisional: true,
+          metadata: {
+            testType: 'provisional',
+            testScores: [
+              { testcase: '1', score: 0 },
+              { testcase: '2', score: 0 },
+            ],
+          },
+          submissionId: 'submission-2',
+        },
+      ]);
     const upsertReviewSummationSpy = jest
       .spyOn(service as any, 'upsertReviewSummation')
       .mockResolvedValue(undefined);
@@ -756,6 +773,12 @@ describe('ScoringResultService', () => {
         reviewSummation: [reviewFor('submission-b', 45)],
       },
     ]);
+    jest
+      .spyOn(service as any, 'fetchChallengeReviewSummations')
+      .mockResolvedValue([
+        { ...reviewFor('submission-a', 90), submissionId: 'submission-a' },
+        { ...reviewFor('submission-b', 45), submissionId: 'submission-b' },
+      ]);
     const upsertReviewSummationSpy = jest
       .spyOn(service as any, 'upsertReviewSummation')
       .mockResolvedValue(undefined);
@@ -893,6 +916,22 @@ describe('ScoringResultService', () => {
         reviewSummation: [reviewFor('submission-other', 80)],
       },
     ]);
+    jest
+      .spyOn(service as any, 'fetchChallengeReviewSummations')
+      .mockResolvedValue([
+        {
+          ...reviewFor('submission-newer', 100),
+          submissionId: 'submission-newer',
+        },
+        {
+          ...reviewFor('submission-old-pending', 50, true),
+          submissionId: 'submission-old-pending',
+        },
+        {
+          ...reviewFor('submission-other', 80),
+          submissionId: 'submission-other',
+        },
+      ]);
     const upsertReviewSummationSpy = jest
       .spyOn(service as any, 'upsertReviewSummation')
       .mockResolvedValue(undefined);
@@ -988,6 +1027,22 @@ describe('ScoringResultService', () => {
         ],
       },
     ]);
+    jest
+      .spyOn(service as any, 'fetchChallengeReviewSummations')
+      .mockResolvedValue([
+        {
+          id: 'summation-visible-latest',
+          aggregateScore: 100,
+          isFinal: true,
+          metadata: {
+            testScores: [{ testcase: '753388858', score: 100 }],
+            testStatus: ScoringTestStatus.Success,
+            testType: 'system',
+          },
+          scorecardId: 'scorecard-1',
+          submissionId: 'submission-visible-latest',
+        },
+      ]);
     jest
       .spyOn(service as any, 'findExistingReviewSummations')
       .mockResolvedValue([
@@ -1059,6 +1114,129 @@ describe('ScoringResultService', () => {
     );
   });
 
+  it('normalizes queued system scores against Review API hydrated latest submissions', 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-current',
+        memberId: 'member-current',
+        isLatest: true,
+        submittedDate: '2026-05-01T00:00:02.000Z',
+        reviewSummation: [],
+      },
+      {
+        id: 'submission-other',
+        memberId: 'member-other',
+        isLatest: true,
+        submittedDate: '2026-05-01T00:00:03.000Z',
+        reviewSummation: [],
+      },
+    ]);
+    jest
+      .spyOn(service as any, 'fetchChallengeReviewSummations')
+      .mockResolvedValue([
+        {
+          id: 'summation-current',
+          aggregateScore: 0,
+          isFinal: true,
+          metadata: {
+            relativeScoringPending: true,
+            testScores: [{ testcase: '753388858', score: 50 }],
+            testStatus: ScoringTestStatus.InProgress,
+            testType: 'system',
+          },
+          scorecardId: 'scorecard-1',
+          submissionId: 'submission-current',
+        },
+        {
+          id: 'summation-other',
+          aggregateScore: 100,
+          isFinal: true,
+          metadata: {
+            testScores: [{ testcase: '753388858', score: 100 }],
+            testStatus: ScoringTestStatus.Success,
+            testType: 'system',
+          },
+          scorecardId: 'scorecard-1',
+          submissionId: 'submission-other',
+        },
+      ]);
+    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-current',
+        reviewId: 'review-current',
+        testPhase: 'system',
+        reviewTypeId: basePayload.reviewTypeId,
+        scorecardId: 'scorecard-1',
+        queuedAt: '2026-05-01T00:00:10.000Z',
+      }),
+    ).resolves.toBe(undefined);
+
+    const currentPayload = upsertReviewSummationSpy.mock.calls.find(
+      ([, , payload]) =>
+        (payload as { submissionId: string }).submissionId ===
+        'submission-current',
+    )?.[2] as
+      | {
+          aggregateScore: number;
+          metadata?: { relativeScores?: Array<{ score: number }> };
+        }
+      | undefined;
+
+    expect(currentPayload).toEqual(
+      expect.objectContaining({
+        aggregateScore: 50,
+        isPassing: true,
+        submissionId: 'submission-current',
+      }),
+    );
+    expect(currentPayload?.metadata?.relativeScores).toEqual([
+      { testcase: '1', score: 50 },
+    ]);
+    expect(completeSystemReviewIfNeededSpy).toHaveBeenCalledWith(
+      'm2m-token',
+      'review-current',
+      50,
+      'system',
+      expect.objectContaining({
+        submissionId: 'submission-current',
+      }),
+    );
+  });
+
   it('serializes concurrent relative scoring recomputations for the same challenge phase', async () => {
     const { service, m2mService, prisma } = createService();
     const firstPayload: ScoringResultCallbackPayload = {
@@ -1149,6 +1327,20 @@ describe('ScoringResultService', () => {
           reviewSummation: [],
         },
       ]);
+    jest
+      .spyOn(service as any, 'fetchChallengeReviewSummations')
+      .mockResolvedValueOnce([])
+      .mockResolvedValueOnce([
+        {
+          id: 'review-a',
+          isProvisional: true,
+          metadata: {
+            testType: 'provisional',
+            testScores: [{ testcase: '1', score: 90 }],
+          },
+          submissionId: 'submission-a',
+        },
+      ]);
     const upsertReviewSummationSpy = jest
       .spyOn(service as any, 'upsertReviewSummation')
       .mockImplementationOnce(async () => {
@@ -2147,6 +2339,9 @@ describe('ScoringResultService', () => {
     const upsertReviewSummationSpy = jest
       .spyOn(service as any, 'upsertReviewSummation')
       .mockResolvedValue(undefined);
+    jest
+      .spyOn(service as any, 'fetchChallengeReviewSummations')
+      .mockResolvedValue([]);
 
     httpService.get
       .mockReturnValueOnce(
@@ -2913,6 +3108,38 @@ describe('ScoringResultService', () => {
         memberId: 'member-tsegaye',
       },
     ]);
+    jest
+      .spyOn(service as any, 'fetchChallengeReviewSummations')
+      .mockResolvedValue([
+        {
+          ...reviewFor('submission-ghost', 100),
+          submissionId: 'submission-ghost',
+        },
+        {
+          ...reviewFor('submission-vdave', 88.33507138754184),
+          submissionId: 'submission-vdave',
+        },
+        {
+          ...reviewFor('submission-bitrelica', 79.5106923255694),
+          submissionId: 'submission-bitrelica',
+        },
+        {
+          ...reviewFor('submission-kazaward', 0.004946668727778636),
+          submissionId: 'submission-kazaward',
+        },
+        {
+          ...reviewFor('submission-eulerschez', 0.004946668727778636),
+          submissionId: 'submission-eulerschez',
+        },
+        {
+          ...reviewFor('submission-shxzhaosr', 0),
+          submissionId: 'submission-shxzhaosr',
+        },
+        {
+          ...reviewFor('submission-failed', -1),
+          submissionId: 'submission-failed',
+        },
+      ]);
     const upsertReviewSummationSpy = jest
       .spyOn(service as any, 'upsertReviewSummation')
       .mockResolvedValue(undefined);
@@ -3236,6 +3463,55 @@ describe('ScoringResultService', () => {
     );
   });
 
+  it('does not use an older scored summation when the latest submission has no phase score', () => {
+    const { service } = createService();
+    const selectLatestRelativeReviewRecords = (
+      service as any
+    ).selectLatestRelativeReviewRecords.bind(service) as (
+      submissions: Record<string, unknown>[],
+      testPhase: string,
+      reviewTypeId: string,
+      excludedSubmissionId: string,
+      excludedMemberId?: string,
+    ) => Array<{
+      submissionId: string;
+      rawTestScores: Array<{ score: number }>;
+    }>;
+
+    const records = selectLatestRelativeReviewRecords(
+      [
+        {
+          id: 'latest-unscored-submission',
+          memberId: 'member-1',
+          isLatest: true,
+          submittedDate: '2026-05-28T15:23:32.877Z',
+          reviewSummation: [],
+        },
+        {
+          id: 'older-scored-submission',
+          memberId: 'member-1',
+          isLatest: false,
+          submittedDate: '2026-05-28T15:21:12.605Z',
+          reviewSummation: [
+            {
+              id: 'older-scored-review',
+              isFinal: true,
+              metadata: {
+                testType: 'system',
+                testScores: [{ testcase: '753388858', score: 100 }],
+              },
+            },
+          ],
+        },
+      ],
+      'system',
+      basePayload.reviewTypeId,
+      'current-submission',
+    );
+
+    expect(records).toEqual([]);
+  });
+
   it('groups relative reviews by extracted submission ID and member identity', () => {
     const { service } = createService();
     const selectLatestRelativeReviewRecords = (
diff --git a/src/api/scoring-result/scoring-result.service.ts b/src/api/scoring-result/scoring-result.service.ts
