From c367f65531dc7ee9f00ace230dfc17394989a287 Mon Sep 17 00:00:00 2001 From: jmgasper Date: Fri, 19 Jun 2026 10:26:34 +1000 Subject: [PATCH 1/4] PM-4151: suppress incomplete MM final export data What was broken Marathon Match submitter exports could show negative failed-run scores, finalScore values while a challenge was still in submission, and a finalRank column before the challenge was completed. Root cause The challenge export SQL treated any final score field as exportable regardless of challenge status, used negative review summation scores as normal scores, and always emitted Marathon Match final columns from the formatter. What was changed Changed the submitter, valid submitter, and winner SQL to ignore negative Marathon Match scores, only expose finalScore/finalRank data for completed challenges, and avoid falling back to submission.finalScore when a final review row explicitly failed. Updated the challenge report formatter so Marathon Match finalScore and finalRank columns are only emitted when at least one row has those values. Updated report descriptions and DTO documentation to match the completed-only final data behavior. Any added/updated tests Updated the challenge SQL regression spec for completed-only final scores, negative score filtering, and completed-only final ranks. Added a service regression test that verifies active Marathon Match exports omit finalScore and finalRank columns when final scoring is unavailable. --- sql/reports/challenges/submitters.sql | 32 ++++++--- sql/reports/challenges/valid-submitters.sql | 32 ++++++--- sql/reports/challenges/winners.sql | 23 +++++-- .../challenges/challenge-export-sql.spec.ts | 51 +++++++++++++-- .../challenges-reports.service.spec.ts | 65 +++++++++++++++++++ .../challenges/challenges-reports.service.ts | 16 ++++- .../challenges/dtos/challenge-users.dto.ts | 6 +- src/reports/report-directory.data.ts | 6 +- 8 files changed, 193 insertions(+), 38 deletions(-) diff --git a/sql/reports/challenges/submitters.sql b/sql/reports/challenges/submitters.sql index 3a19f08..1949c22 100644 --- a/sql/reports/challenges/submitters.sql +++ b/sql/reports/challenges/submitters.sql @@ -1,7 +1,8 @@ WITH challenge_context AS ( SELECT c.id, - (ct.name = 'Marathon Match') AS is_marathon_match + (ct.name = 'Marathon Match') AS is_marathon_match, + (c.status = 'COMPLETED') AS is_completed FROM challenges."Challenge" AS c JOIN challenges."ChallengeType" AS ct ON ct.id = c."typeId" @@ -18,10 +19,18 @@ submission_metrics AS ( s."initialScore"::double precision ) AS standard_score, provisional_review.provisional_score, - COALESCE( - final_review."aggregateScore", - s."finalScore"::double precision - ) AS final_score_raw + CASE + WHEN cc.is_completed THEN CASE + WHEN final_review."aggregateScore" IS NOT NULL THEN CASE + WHEN final_review."aggregateScore" >= 0 THEN final_review."aggregateScore" + ELSE NULL + END + WHEN s."finalScore"::double precision >= 0 THEN s."finalScore"::double precision + ELSE NULL + END + ELSE NULL + END AS final_score_raw, + cc.is_completed FROM challenge_context AS cc JOIN reviews."submission" AS s ON s."challengeId" = cc.id @@ -40,6 +49,7 @@ submission_metrics AS ( FROM reviews."reviewSummation" AS rs WHERE rs."submissionId" = s.id AND rs."isProvisional" IS TRUE + AND rs."aggregateScore" >= 0 ORDER BY COALESCE(rs."reviewedDate", rs."createdAt") DESC NULLS LAST, rs.id DESC LIMIT 1 ) AS provisional_review ON TRUE @@ -73,7 +83,7 @@ mm_latest_submission_scores AS ( sm."memberId", sm.provisional_score AS provisional_score_raw, sm.final_score_raw, - COALESCE(sm.final_score_raw, sm.provisional_score) AS effective_score_raw, + sm.is_completed, sm.submission_timestamp FROM submission_metrics AS sm ORDER BY @@ -93,13 +103,13 @@ mm_ranked_scores AS ( ELSE ROUND(mlss.final_score_raw::numeric, 2) END AS "finalScore", CASE - WHEN mlss.effective_score_raw IS NULL THEN NULL - ELSE ROW_NUMBER() OVER ( + WHEN mlss.is_completed AND mlss.final_score_raw IS NOT NULL THEN ROW_NUMBER() OVER ( ORDER BY - mlss.effective_score_raw DESC NULLS LAST, + mlss.final_score_raw DESC NULLS LAST, mlss.submission_timestamp ASC NULLS LAST, mlss."memberId" ASC ) + ELSE NULL END AS "finalRank" FROM mm_latest_submission_scores AS mlss ) @@ -177,6 +187,10 @@ ORDER BY WHEN sm.is_marathon_match THEN mrs."finalRank" ELSE NULL END ASC NULLS LAST, + CASE + WHEN sm.is_marathon_match AND mrs."finalRank" IS NULL THEN mrs."provisionalScore" + ELSE NULL + END DESC NULLS LAST, CASE WHEN sm.is_marathon_match THEN NULL ELSE sms."submissionScore" diff --git a/sql/reports/challenges/valid-submitters.sql b/sql/reports/challenges/valid-submitters.sql index 1f4ee22..77c4659 100644 --- a/sql/reports/challenges/valid-submitters.sql +++ b/sql/reports/challenges/valid-submitters.sql @@ -1,7 +1,8 @@ WITH challenge_context AS ( SELECT c.id, - (ct.name = 'Marathon Match') AS is_marathon_match + (ct.name = 'Marathon Match') AS is_marathon_match, + (c.status = 'COMPLETED') AS is_completed FROM challenges."Challenge" AS c JOIN challenges."ChallengeType" AS ct ON ct.id = c."typeId" @@ -18,10 +19,18 @@ submission_metrics AS ( s."initialScore"::double precision ) AS standard_score, provisional_review.provisional_score, - COALESCE( - final_review."aggregateScore", - s."finalScore"::double precision - ) AS final_score_raw, + CASE + WHEN cc.is_completed THEN CASE + WHEN final_review."aggregateScore" IS NOT NULL THEN CASE + WHEN final_review."aggregateScore" >= 0 THEN final_review."aggregateScore" + ELSE NULL + END + WHEN s."finalScore"::double precision >= 0 THEN s."finalScore"::double precision + ELSE NULL + END + ELSE NULL + END AS final_score_raw, + cc.is_completed, ( passing_review.is_passing IS TRUE OR COALESCE(s."finalScore"::double precision, 0) > 98 @@ -44,6 +53,7 @@ submission_metrics AS ( FROM reviews."reviewSummation" AS rs WHERE rs."submissionId" = s.id AND rs."isProvisional" IS TRUE + AND rs."aggregateScore" >= 0 ORDER BY COALESCE(rs."reviewedDate", rs."createdAt") DESC NULLS LAST, rs.id DESC LIMIT 1 ) AS provisional_review ON TRUE @@ -90,7 +100,7 @@ mm_latest_submission_scores AS ( vsm."memberId", vsm.provisional_score AS provisional_score_raw, vsm.final_score_raw, - COALESCE(vsm.final_score_raw, vsm.provisional_score) AS effective_score_raw, + vsm.is_completed, vsm.submission_timestamp FROM valid_submission_metrics AS vsm ORDER BY @@ -110,13 +120,13 @@ mm_ranked_scores AS ( ELSE ROUND(mlss.final_score_raw::numeric, 2) END AS "finalScore", CASE - WHEN mlss.effective_score_raw IS NULL THEN NULL - ELSE ROW_NUMBER() OVER ( + WHEN mlss.is_completed AND mlss.final_score_raw IS NOT NULL THEN ROW_NUMBER() OVER ( ORDER BY - mlss.effective_score_raw DESC NULLS LAST, + mlss.final_score_raw DESC NULLS LAST, mlss.submission_timestamp ASC NULLS LAST, mlss."memberId" ASC ) + ELSE NULL END AS "finalRank" FROM mm_latest_submission_scores AS mlss ) @@ -194,6 +204,10 @@ ORDER BY WHEN vsm.is_marathon_match THEN mrs."finalRank" ELSE NULL END ASC NULLS LAST, + CASE + WHEN vsm.is_marathon_match AND mrs."finalRank" IS NULL THEN mrs."provisionalScore" + ELSE NULL + END DESC NULLS LAST, CASE WHEN vsm.is_marathon_match THEN NULL ELSE sms."submissionScore" diff --git a/sql/reports/challenges/winners.sql b/sql/reports/challenges/winners.sql index 8dc628a..7d338d2 100644 --- a/sql/reports/challenges/winners.sql +++ b/sql/reports/challenges/winners.sql @@ -1,7 +1,8 @@ WITH challenge_context AS ( SELECT c.id, - (ct.name = 'Marathon Match') AS is_marathon_match + (ct.name = 'Marathon Match') AS is_marathon_match, + (c.status = 'COMPLETED') AS is_completed FROM challenges."Challenge" AS c JOIN challenges."ChallengeType" AS ct ON ct.id = c."typeId" @@ -16,10 +17,17 @@ submission_metrics AS ( s."initialScore"::double precision ) AS standard_score, provisional_review.provisional_score, - COALESCE( - final_review."aggregateScore", - s."finalScore"::double precision - ) AS final_score_raw + CASE + WHEN cc.is_completed THEN CASE + WHEN final_review."aggregateScore" IS NOT NULL THEN CASE + WHEN final_review."aggregateScore" >= 0 THEN final_review."aggregateScore" + ELSE NULL + END + WHEN s."finalScore"::double precision >= 0 THEN s."finalScore"::double precision + ELSE NULL + END + ELSE NULL + END AS final_score_raw FROM challenge_context AS cc JOIN reviews."submission" AS s ON s."challengeId" = cc.id @@ -38,11 +46,13 @@ submission_metrics AS ( FROM reviews."reviewSummation" AS rs WHERE rs."submissionId" = s.id AND rs."isProvisional" IS TRUE + AND rs."aggregateScore" >= 0 ) AS provisional_review ON TRUE ), winner_members AS MATERIALIZED ( SELECT cc.is_marathon_match, + cc.is_completed, cw."userId"::text AS "memberId", MAX(cw.handle) AS "winnerHandle", MIN(cw.placement) AS placement @@ -52,6 +62,7 @@ winner_members AS MATERIALIZED ( AND cw.type = 'PLACEMENT' GROUP BY cc.is_marathon_match, + cc.is_completed, cw."userId" ), standard_member_scores AS ( @@ -126,7 +137,7 @@ SELECT ELSE NULL END AS "finalScore", CASE - WHEN wm.is_marathon_match THEN wm.placement + WHEN wm.is_marathon_match AND wm.is_completed THEN wm.placement ELSE NULL END AS "finalRank" FROM winner_members AS wm diff --git a/src/reports/challenges/challenge-export-sql.spec.ts b/src/reports/challenges/challenge-export-sql.spec.ts index 15b92b1..d752f55 100644 --- a/src/reports/challenges/challenge-export-sql.spec.ts +++ b/src/reports/challenges/challenge-export-sql.spec.ts @@ -3,18 +3,59 @@ import { SqlLoaderService } from "src/common/sql-loader.service"; describe("Challenge export SQL", () => { const sqlLoader = new SqlLoaderService(); - it.each([ + const challengeUserSqlPaths = [ "reports/challenges/submitters.sql", "reports/challenges/valid-submitters.sql", "reports/challenges/winners.sql", - ])( - "falls back to submission.finalScore in %s when no final review summary exists", + ]; + + it.each(challengeUserSqlPaths)( + "uses completed, non-failed Marathon Match final scores in %s", (sqlPath) => { const sql = sqlLoader.load(sqlPath); - expect(sql).toMatch( - /COALESCE\(\s*final_review\."aggregateScore",\s*s\."finalScore"::double precision\s*\)\s+AS final_score_raw/, + expect(sql).toContain(`(c.status = 'COMPLETED') AS is_completed`); + expect(sql).toContain("WHEN cc.is_completed THEN CASE"); + expect(sql).toContain( + `WHEN final_review."aggregateScore" IS NOT NULL THEN CASE`, + ); + expect(sql).toContain( + `WHEN final_review."aggregateScore" >= 0 THEN final_review."aggregateScore"`, + ); + expect(sql).toContain( + `WHEN s."finalScore"::double precision >= 0 THEN s."finalScore"::double precision`, ); }, ); + + it.each(challengeUserSqlPaths)( + "filters failed negative Marathon Match provisional scores in %s", + (sqlPath) => { + const sql = sqlLoader.load(sqlPath); + + expect(sql).toContain(`AND rs."aggregateScore" >= 0`); + }, + ); + + it.each([ + "reports/challenges/submitters.sql", + "reports/challenges/valid-submitters.sql", + ])("only ranks completed Marathon Match submissions in %s", (sqlPath) => { + const sql = sqlLoader.load(sqlPath); + + expect(sql).toContain( + "WHEN mlss.is_completed AND mlss.final_score_raw IS NOT NULL THEN ROW_NUMBER() OVER", + ); + expect(sql).toMatch( + /WHEN \w+\.is_marathon_match AND mrs\."finalRank" IS NULL THEN mrs\."provisionalScore"/, + ); + }); + + it("only returns Marathon Match winner finalRank for completed challenges", () => { + const sql = sqlLoader.load("reports/challenges/winners.sql"); + + expect(sql).toContain( + "WHEN wm.is_marathon_match AND wm.is_completed THEN wm.placement", + ); + }); }); diff --git a/src/reports/challenges/challenges-reports.service.spec.ts b/src/reports/challenges/challenges-reports.service.spec.ts index 999ac5d..91e5c17 100644 --- a/src/reports/challenges/challenges-reports.service.spec.ts +++ b/src/reports/challenges/challenges-reports.service.spec.ts @@ -87,6 +87,71 @@ describe("ChallengesReportsService", () => { ]); }); + it("omits Marathon Match final columns when final scoring is unavailable", async () => { + db.query.mockResolvedValue([ + { + userId: 88779578, + handle: "adipowfamo", + email: "topcodergh+adipowfamo@gmail.com", + firstName: "Adipo", + lastName: "Wfamo", + country: "Australia", + isMarathonMatch: true, + provisionalScore: 87.29, + finalScore: null, + finalRank: null, + }, + { + userId: 10000039, + handle: "testaws1", + email: "topcodergh+testaws1@gmail.com", + firstName: "Testaws", + lastName: "One", + country: "Japan", + isMarathonMatch: true, + provisionalScore: 90.83, + finalScore: null, + finalRank: null, + }, + ]); + + const result = await service.getSubmitters({ + challengeId: "1bb94965-32e3-40a6-9933-2c6bd9dcdca8", + }); + + expect(result).toEqual([ + { + userId: 88779578, + handle: "adipowfamo", + email: "topcodergh+adipowfamo@gmail.com", + firstName: "Adipo", + lastName: "Wfamo", + country: "Australia", + provisionalScore: 87.29, + }, + { + userId: 10000039, + handle: "testaws1", + email: "topcodergh+testaws1@gmail.com", + firstName: "Testaws", + lastName: "One", + country: "Japan", + provisionalScore: 90.83, + }, + ]); + expect(Object.keys(result[0])).toEqual([ + "userId", + "handle", + "email", + "firstName", + "lastName", + "country", + "provisionalScore", + ]); + expect(result[0]).not.toHaveProperty("finalScore"); + expect(result[0]).not.toHaveProperty("finalRank"); + }); + it("returns Marathon Match winners with final scores when available", async () => { db.query.mockResolvedValue([ { diff --git a/src/reports/challenges/challenges-reports.service.ts b/src/reports/challenges/challenges-reports.service.ts index a94d39a..df29539 100644 --- a/src/reports/challenges/challenges-reports.service.ts +++ b/src/reports/challenges/challenges-reports.service.ts @@ -184,7 +184,7 @@ export class ChallengesReportsService { /** * Normalizes raw challenge user report rows into the exported column shape. * @param records SQL rows for one challenge report, including the internal Marathon Match flag. - * @returns Export-ready records with either submissionScore or the Marathon Match-specific score and ranking columns. + * @returns Export-ready records with either submissionScore or the available Marathon Match score and ranking columns. * @throws Does not throw. It is used as a pure formatter inside the challenge report service methods. */ private formatChallengeUserReport( @@ -197,6 +197,12 @@ export class ChallengesReportsService { const isMarathonMatch = records.some( (record) => record.isMarathonMatch === true, ); + const hasFinalScore = records.some( + (record) => record.finalScore !== null && record.finalScore !== undefined, + ); + const hasFinalRank = records.some( + (record) => record.finalRank !== null && record.finalRank !== undefined, + ); return records.map((record) => { const normalized: ChallengeUserRecordDto = { @@ -210,8 +216,12 @@ export class ChallengesReportsService { if (isMarathonMatch) { normalized.provisionalScore = record.provisionalScore ?? null; - normalized.finalScore = record.finalScore ?? null; - normalized.finalRank = record.finalRank ?? null; + if (hasFinalScore) { + normalized.finalScore = record.finalScore ?? null; + } + if (hasFinalRank) { + normalized.finalRank = record.finalRank ?? null; + } return normalized; } diff --git a/src/reports/challenges/dtos/challenge-users.dto.ts b/src/reports/challenges/dtos/challenge-users.dto.ts index 8e5dcfb..9204f70 100644 --- a/src/reports/challenges/dtos/challenge-users.dto.ts +++ b/src/reports/challenges/dtos/challenge-users.dto.ts @@ -17,9 +17,9 @@ export class ChallengeUsersPathParamDto { /** * User record returned by challenge user reports including resolved country. * Standard challenge submission-based reports expose submissionScore. - * Marathon Match submission-based reports expose provisionalScore and - * finalScore from the latest submission, plus finalRank by current effective - * score, breaking ties by earlier submission time. + * Marathon Match submission-based reports expose provisionalScore from the + * latest submission. Completed Marathon Match reports also expose finalScore + * and finalRank when final scoring data is available. */ export interface ChallengeUserRecordDto { userId: number; diff --git a/src/reports/report-directory.data.ts b/src/reports/report-directory.data.ts index eab8d1d..cb470c4 100644 --- a/src/reports/report-directory.data.ts +++ b/src/reports/report-directory.data.ts @@ -414,21 +414,21 @@ const REGISTERED_REPORTS_DIRECTORY: RegisteredReportsDirectory = { challengeReport( "Challenge Submitters", "/challenges/:challengeId/submitters", - "Return the challenge submitters report. Marathon Match exports use the latest submission provisionalScore and finalScore when available, plus the current effective rank, with earlier submission times winning score ties.", + "Return the challenge submitters report. Marathon Match exports use the latest non-failed provisionalScore and include finalScore/finalRank only after final scoring is available for completed challenges.", AppScopes.Challenge.Submitters, [challengeIdParam], ), challengeReport( "Challenge Valid Submitters", "/challenges/:challengeId/valid-submitters", - "Return the challenge valid submitters report. Marathon Match exports use the latest submission provisionalScore and finalScore when available, plus the current effective rank, with earlier submission times winning score ties.", + "Return the challenge valid submitters report. Marathon Match exports use the latest non-failed provisionalScore and include finalScore/finalRank only after final scoring is available for completed challenges.", AppScopes.Challenge.ValidSubmitters, [challengeIdParam], ), challengeReport( "Challenge Winners", "/challenges/:challengeId/winners", - "Return the challenge winners report with placement winners only. Marathon Match exports include provisionalScore, finalScore, and the challenge-result finalRank.", + "Return the challenge winners report with placement winners only. Marathon Match exports include non-failed provisionalScore and completed challenge finalScore/finalRank values.", AppScopes.Challenge.Winners, [challengeIdParam], ), From f9caad42b5f7c61ca1d221e31d1c5c545223dde9 Mon Sep 17 00:00:00 2001 From: jmgasper Date: Fri, 19 Jun 2026 11:32:06 +1000 Subject: [PATCH 2/4] PM-4151: guard MM final score fallback What was broken The previous PM-4151 follow-up suppressed active Marathon Match final columns and negative scores, but the final score fallback still depended on final_review.aggregateScore being non-null. If a final review row exists without a valid non-negative score, the query could still fall back to stale submission.finalScore. Root cause The final review lateral subquery only projected aggregateScore, so the scoring CASE could not distinguish no final review row from a final review row that exists but is not usable for export. What was changed Projected a has_final_review flag from the final review lateral subqueries for submitter, valid submitter, and winner exports. The Marathon Match final score CASE now falls back to submission.finalScore only when no final review row exists, while existing final review rows with negative or otherwise unusable scores export null. Any added/updated tests Updated the challenge export SQL regression spec to require the final review presence guard across submitter, valid submitter, and winner SQL. --- sql/reports/challenges/submitters.sql | 6 ++++-- sql/reports/challenges/valid-submitters.sql | 6 ++++-- sql/reports/challenges/winners.sql | 6 ++++-- src/reports/challenges/challenge-export-sql.spec.ts | 5 ++--- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/sql/reports/challenges/submitters.sql b/sql/reports/challenges/submitters.sql index 1949c22..5e1c02a 100644 --- a/sql/reports/challenges/submitters.sql +++ b/sql/reports/challenges/submitters.sql @@ -21,7 +21,7 @@ submission_metrics AS ( provisional_review.provisional_score, CASE WHEN cc.is_completed THEN CASE - WHEN final_review."aggregateScore" IS NOT NULL THEN CASE + WHEN final_review.has_final_review THEN CASE WHEN final_review."aggregateScore" >= 0 THEN final_review."aggregateScore" ELSE NULL END @@ -36,7 +36,9 @@ submission_metrics AS ( ON s."challengeId" = cc.id AND s."memberId" IS NOT NULL LEFT JOIN LATERAL ( - SELECT rs."aggregateScore" + SELECT + TRUE AS has_final_review, + rs."aggregateScore" FROM reviews."reviewSummation" AS rs WHERE rs."submissionId" = s.id AND COALESCE(rs."isFinal", TRUE) = TRUE diff --git a/sql/reports/challenges/valid-submitters.sql b/sql/reports/challenges/valid-submitters.sql index 77c4659..1212929 100644 --- a/sql/reports/challenges/valid-submitters.sql +++ b/sql/reports/challenges/valid-submitters.sql @@ -21,7 +21,7 @@ submission_metrics AS ( provisional_review.provisional_score, CASE WHEN cc.is_completed THEN CASE - WHEN final_review."aggregateScore" IS NOT NULL THEN CASE + WHEN final_review.has_final_review THEN CASE WHEN final_review."aggregateScore" >= 0 THEN final_review."aggregateScore" ELSE NULL END @@ -40,7 +40,9 @@ submission_metrics AS ( ON s."challengeId" = cc.id AND s."memberId" IS NOT NULL LEFT JOIN LATERAL ( - SELECT rs."aggregateScore" + SELECT + TRUE AS has_final_review, + rs."aggregateScore" FROM reviews."reviewSummation" AS rs WHERE rs."submissionId" = s.id AND COALESCE(rs."isFinal", TRUE) = TRUE diff --git a/sql/reports/challenges/winners.sql b/sql/reports/challenges/winners.sql index 7d338d2..55fe21d 100644 --- a/sql/reports/challenges/winners.sql +++ b/sql/reports/challenges/winners.sql @@ -19,7 +19,7 @@ submission_metrics AS ( provisional_review.provisional_score, CASE WHEN cc.is_completed THEN CASE - WHEN final_review."aggregateScore" IS NOT NULL THEN CASE + WHEN final_review.has_final_review THEN CASE WHEN final_review."aggregateScore" >= 0 THEN final_review."aggregateScore" ELSE NULL END @@ -33,7 +33,9 @@ submission_metrics AS ( ON s."challengeId" = cc.id AND s."memberId" IS NOT NULL LEFT JOIN LATERAL ( - SELECT rs."aggregateScore" + SELECT + TRUE AS has_final_review, + rs."aggregateScore" FROM reviews."reviewSummation" AS rs WHERE rs."submissionId" = s.id AND COALESCE(rs."isFinal", TRUE) = TRUE diff --git a/src/reports/challenges/challenge-export-sql.spec.ts b/src/reports/challenges/challenge-export-sql.spec.ts index d752f55..2c40035 100644 --- a/src/reports/challenges/challenge-export-sql.spec.ts +++ b/src/reports/challenges/challenge-export-sql.spec.ts @@ -16,9 +16,8 @@ describe("Challenge export SQL", () => { expect(sql).toContain(`(c.status = 'COMPLETED') AS is_completed`); expect(sql).toContain("WHEN cc.is_completed THEN CASE"); - expect(sql).toContain( - `WHEN final_review."aggregateScore" IS NOT NULL THEN CASE`, - ); + expect(sql).toContain("TRUE AS has_final_review"); + expect(sql).toContain("WHEN final_review.has_final_review THEN CASE"); expect(sql).toContain( `WHEN final_review."aggregateScore" >= 0 THEN final_review."aggregateScore"`, ); From e3be399b22081586b8fdeecbf03e9f2c913728ac Mon Sep 17 00:00:00 2001 From: jmgasper Date: Mon, 22 Jun 2026 16:47:05 +1000 Subject: [PATCH 3/4] PM-4151: ignore failed MM score runs What was broken The previous PM-4151 follow-up suppressed active Marathon Match final columns and guarded final review fallbacks, but failed or deleted Marathon Match submissions could still influence score output. Positive initialScore/finalScore fallbacks from failed submissions could leak stale scores, and a newest failed submission with no usable score could mask an earlier valid scored run. Root cause The challenge export SQL filtered negative review summation scores, but the submission.initialScore and submission.finalScore fallback paths did not check reviews.submission.status. The submitter and valid submitter ranking CTEs also selected the newest submission before checking whether it had any usable Marathon Match score. What was changed Added failed/deleted submission status guards before using Marathon Match provisional and final score fallbacks in submitter, valid submitter, and winner exports. Changed Marathon Match submitter and valid submitter score selection to skip rows where both provisionalScore and finalScore are unavailable, so the latest usable scored run is used instead of a failed no-score run. Updated the challenge user DTO documentation to describe the latest non-failed scored submission behavior. Any added/updated tests Updated src/reports/challenges/challenge-export-sql.spec.ts to cover failed/deleted status guards, provisional initialScore fallback, and latest usable score selection for Marathon Match submitter exports. --- sql/reports/challenges/submitters.sql | 33 +++++++++++++++---- sql/reports/challenges/valid-submitters.sql | 33 +++++++++++++++---- sql/reports/challenges/winners.sql | 30 +++++++++++++---- .../challenges/challenge-export-sql.spec.ts | 22 ++++++++++++- .../challenges/dtos/challenge-users.dto.ts | 4 +-- 5 files changed, 98 insertions(+), 24 deletions(-) diff --git a/sql/reports/challenges/submitters.sql b/sql/reports/challenges/submitters.sql index 5e1c02a..6c6bfe6 100644 --- a/sql/reports/challenges/submitters.sql +++ b/sql/reports/challenges/submitters.sql @@ -18,16 +18,32 @@ submission_metrics AS ( s."finalScore"::double precision, s."initialScore"::double precision ) AS standard_score, - provisional_review.provisional_score, CASE - WHEN cc.is_completed THEN CASE - WHEN final_review.has_final_review THEN CASE - WHEN final_review."aggregateScore" >= 0 THEN final_review."aggregateScore" - ELSE NULL - END - WHEN s."finalScore"::double precision >= 0 THEN s."finalScore"::double precision + WHEN s.status IN ( + 'FAILED_SCREENING', + 'FAILED_REVIEW', + 'FAILED_CHECKPOINT_SCREENING', + 'FAILED_CHECKPOINT_REVIEW', + 'DELETED' + ) THEN NULL + WHEN provisional_review.provisional_score IS NOT NULL THEN provisional_review.provisional_score + WHEN s."initialScore"::double precision >= 0 THEN s."initialScore"::double precision + ELSE NULL + END AS provisional_score, + CASE + WHEN NOT cc.is_completed THEN NULL + WHEN s.status IN ( + 'FAILED_SCREENING', + 'FAILED_REVIEW', + 'FAILED_CHECKPOINT_SCREENING', + 'FAILED_CHECKPOINT_REVIEW', + 'DELETED' + ) THEN NULL + WHEN final_review.has_final_review THEN CASE + WHEN final_review."aggregateScore" >= 0 THEN final_review."aggregateScore" ELSE NULL END + WHEN s."finalScore"::double precision >= 0 THEN s."finalScore"::double precision ELSE NULL END AS final_score_raw, cc.is_completed @@ -88,6 +104,9 @@ mm_latest_submission_scores AS ( sm.is_completed, sm.submission_timestamp FROM submission_metrics AS sm + WHERE + sm.provisional_score IS NOT NULL + OR sm.final_score_raw IS NOT NULL ORDER BY sm."memberId", sm.submission_timestamp DESC NULLS LAST, diff --git a/sql/reports/challenges/valid-submitters.sql b/sql/reports/challenges/valid-submitters.sql index 1212929..434c0f2 100644 --- a/sql/reports/challenges/valid-submitters.sql +++ b/sql/reports/challenges/valid-submitters.sql @@ -18,16 +18,32 @@ submission_metrics AS ( s."finalScore"::double precision, s."initialScore"::double precision ) AS standard_score, - provisional_review.provisional_score, CASE - WHEN cc.is_completed THEN CASE - WHEN final_review.has_final_review THEN CASE - WHEN final_review."aggregateScore" >= 0 THEN final_review."aggregateScore" - ELSE NULL - END - WHEN s."finalScore"::double precision >= 0 THEN s."finalScore"::double precision + WHEN s.status IN ( + 'FAILED_SCREENING', + 'FAILED_REVIEW', + 'FAILED_CHECKPOINT_SCREENING', + 'FAILED_CHECKPOINT_REVIEW', + 'DELETED' + ) THEN NULL + WHEN provisional_review.provisional_score IS NOT NULL THEN provisional_review.provisional_score + WHEN s."initialScore"::double precision >= 0 THEN s."initialScore"::double precision + ELSE NULL + END AS provisional_score, + CASE + WHEN NOT cc.is_completed THEN NULL + WHEN s.status IN ( + 'FAILED_SCREENING', + 'FAILED_REVIEW', + 'FAILED_CHECKPOINT_SCREENING', + 'FAILED_CHECKPOINT_REVIEW', + 'DELETED' + ) THEN NULL + WHEN final_review.has_final_review THEN CASE + WHEN final_review."aggregateScore" >= 0 THEN final_review."aggregateScore" ELSE NULL END + WHEN s."finalScore"::double precision >= 0 THEN s."finalScore"::double precision ELSE NULL END AS final_score_raw, cc.is_completed, @@ -105,6 +121,9 @@ mm_latest_submission_scores AS ( vsm.is_completed, vsm.submission_timestamp FROM valid_submission_metrics AS vsm + WHERE + vsm.provisional_score IS NOT NULL + OR vsm.final_score_raw IS NOT NULL ORDER BY vsm."memberId", vsm.submission_timestamp DESC NULLS LAST, diff --git a/sql/reports/challenges/winners.sql b/sql/reports/challenges/winners.sql index 55fe21d..0301be2 100644 --- a/sql/reports/challenges/winners.sql +++ b/sql/reports/challenges/winners.sql @@ -16,16 +16,32 @@ submission_metrics AS ( s."finalScore"::double precision, s."initialScore"::double precision ) AS standard_score, - provisional_review.provisional_score, CASE - WHEN cc.is_completed THEN CASE - WHEN final_review.has_final_review THEN CASE - WHEN final_review."aggregateScore" >= 0 THEN final_review."aggregateScore" - ELSE NULL - END - WHEN s."finalScore"::double precision >= 0 THEN s."finalScore"::double precision + WHEN s.status IN ( + 'FAILED_SCREENING', + 'FAILED_REVIEW', + 'FAILED_CHECKPOINT_SCREENING', + 'FAILED_CHECKPOINT_REVIEW', + 'DELETED' + ) THEN NULL + WHEN provisional_review.provisional_score IS NOT NULL THEN provisional_review.provisional_score + WHEN s."initialScore"::double precision >= 0 THEN s."initialScore"::double precision + ELSE NULL + END AS provisional_score, + CASE + WHEN NOT cc.is_completed THEN NULL + WHEN s.status IN ( + 'FAILED_SCREENING', + 'FAILED_REVIEW', + 'FAILED_CHECKPOINT_SCREENING', + 'FAILED_CHECKPOINT_REVIEW', + 'DELETED' + ) THEN NULL + WHEN final_review.has_final_review THEN CASE + WHEN final_review."aggregateScore" >= 0 THEN final_review."aggregateScore" ELSE NULL END + WHEN s."finalScore"::double precision >= 0 THEN s."finalScore"::double precision ELSE NULL END AS final_score_raw FROM challenge_context AS cc diff --git a/src/reports/challenges/challenge-export-sql.spec.ts b/src/reports/challenges/challenge-export-sql.spec.ts index 2c40035..66dc31f 100644 --- a/src/reports/challenges/challenge-export-sql.spec.ts +++ b/src/reports/challenges/challenge-export-sql.spec.ts @@ -15,8 +15,9 @@ describe("Challenge export SQL", () => { const sql = sqlLoader.load(sqlPath); expect(sql).toContain(`(c.status = 'COMPLETED') AS is_completed`); - expect(sql).toContain("WHEN cc.is_completed THEN CASE"); + expect(sql).toContain("WHEN NOT cc.is_completed THEN NULL"); expect(sql).toContain("TRUE AS has_final_review"); + expect(sql).toContain(`WHEN s.status IN (`); expect(sql).toContain("WHEN final_review.has_final_review THEN CASE"); expect(sql).toContain( `WHEN final_review."aggregateScore" >= 0 THEN final_review."aggregateScore"`, @@ -27,6 +28,22 @@ describe("Challenge export SQL", () => { }, ); + it.each(challengeUserSqlPaths)( + "uses only non-failed Marathon Match provisional score fallbacks in %s", + (sqlPath) => { + const sql = sqlLoader.load(sqlPath); + + expect(sql).toContain( + "WHEN provisional_review.provisional_score IS NOT NULL THEN provisional_review.provisional_score", + ); + expect(sql).toContain( + `WHEN s."initialScore"::double precision >= 0 THEN s."initialScore"::double precision`, + ); + expect(sql).toContain(`'FAILED_REVIEW'`); + expect(sql).toContain(`'DELETED'`); + }, + ); + it.each(challengeUserSqlPaths)( "filters failed negative Marathon Match provisional scores in %s", (sqlPath) => { @@ -45,6 +62,9 @@ describe("Challenge export SQL", () => { expect(sql).toContain( "WHEN mlss.is_completed AND mlss.final_score_raw IS NOT NULL THEN ROW_NUMBER() OVER", ); + expect(sql).toMatch( + /WHERE\s+\w+\.provisional_score IS NOT NULL\s+OR \w+\.final_score_raw IS NOT NULL/, + ); expect(sql).toMatch( /WHEN \w+\.is_marathon_match AND mrs\."finalRank" IS NULL THEN mrs\."provisionalScore"/, ); diff --git a/src/reports/challenges/dtos/challenge-users.dto.ts b/src/reports/challenges/dtos/challenge-users.dto.ts index 9204f70..be29742 100644 --- a/src/reports/challenges/dtos/challenge-users.dto.ts +++ b/src/reports/challenges/dtos/challenge-users.dto.ts @@ -18,8 +18,8 @@ export class ChallengeUsersPathParamDto { * User record returned by challenge user reports including resolved country. * Standard challenge submission-based reports expose submissionScore. * Marathon Match submission-based reports expose provisionalScore from the - * latest submission. Completed Marathon Match reports also expose finalScore - * and finalRank when final scoring data is available. + * latest non-failed scored submission. Completed Marathon Match reports also + * expose finalScore and finalRank when final scoring data is available. */ export interface ChallengeUserRecordDto { userId: number; From f7b045905c07a830b0b3a6399df5018a3cf9843e Mon Sep 17 00:00:00 2001 From: jmgasper Date: Tue, 23 Jun 2026 06:41:17 +1000 Subject: [PATCH 4/4] PM-4151: guard MM provisional review fallback What was broken The previous PM-4151 follow-up blocked failed/deleted Marathon Match submissions and guarded final score fallback, but a failed provisional review row could still leak stale score data. If the latest provisional review summation had a negative score, the SQL filtered that row out before fallback logic and then used submission.initialScore or an older positive provisional row. Root cause The provisional review lateral queries only selected non-negative reviewSummation rows, so the CASE expression could not distinguish no provisional review from a latest failed provisional review. The final score path already had a has_final_review guard, but the provisional score path did not have the same guard. What was changed Added a has_provisional_review guard to submitter, valid submitter, and winner exports. The reports now use the latest provisional review row, return the score only when that row is non-negative, and only fall back to submission.initialScore when no provisional review row exists. Any added/updated tests Updated src/reports/challenges/challenge-export-sql.spec.ts to assert the provisional review guard and prevent pre-filtering negative provisional review rows before fallback logic runs. --- sql/reports/challenges/submitters.sql | 10 +++++++--- sql/reports/challenges/valid-submitters.sql | 10 +++++++--- sql/reports/challenges/winners.sql | 12 +++++++++--- src/reports/challenges/challenge-export-sql.spec.ts | 10 +++++++--- 4 files changed, 30 insertions(+), 12 deletions(-) diff --git a/sql/reports/challenges/submitters.sql b/sql/reports/challenges/submitters.sql index 6c6bfe6..2007671 100644 --- a/sql/reports/challenges/submitters.sql +++ b/sql/reports/challenges/submitters.sql @@ -26,7 +26,10 @@ submission_metrics AS ( 'FAILED_CHECKPOINT_REVIEW', 'DELETED' ) THEN NULL - WHEN provisional_review.provisional_score IS NOT NULL THEN provisional_review.provisional_score + WHEN provisional_review.has_provisional_review THEN CASE + WHEN provisional_review.provisional_score >= 0 THEN provisional_review.provisional_score + ELSE NULL + END WHEN s."initialScore"::double precision >= 0 THEN s."initialScore"::double precision ELSE NULL END AS provisional_score, @@ -63,11 +66,12 @@ submission_metrics AS ( LIMIT 1 ) AS final_review ON TRUE LEFT JOIN LATERAL ( - SELECT rs."aggregateScore" AS provisional_score + SELECT + TRUE AS has_provisional_review, + rs."aggregateScore" AS provisional_score FROM reviews."reviewSummation" AS rs WHERE rs."submissionId" = s.id AND rs."isProvisional" IS TRUE - AND rs."aggregateScore" >= 0 ORDER BY COALESCE(rs."reviewedDate", rs."createdAt") DESC NULLS LAST, rs.id DESC LIMIT 1 ) AS provisional_review ON TRUE diff --git a/sql/reports/challenges/valid-submitters.sql b/sql/reports/challenges/valid-submitters.sql index 434c0f2..3c6a252 100644 --- a/sql/reports/challenges/valid-submitters.sql +++ b/sql/reports/challenges/valid-submitters.sql @@ -26,7 +26,10 @@ submission_metrics AS ( 'FAILED_CHECKPOINT_REVIEW', 'DELETED' ) THEN NULL - WHEN provisional_review.provisional_score IS NOT NULL THEN provisional_review.provisional_score + WHEN provisional_review.has_provisional_review THEN CASE + WHEN provisional_review.provisional_score >= 0 THEN provisional_review.provisional_score + ELSE NULL + END WHEN s."initialScore"::double precision >= 0 THEN s."initialScore"::double precision ELSE NULL END AS provisional_score, @@ -67,11 +70,12 @@ submission_metrics AS ( LIMIT 1 ) AS final_review ON TRUE LEFT JOIN LATERAL ( - SELECT rs."aggregateScore" AS provisional_score + SELECT + TRUE AS has_provisional_review, + rs."aggregateScore" AS provisional_score FROM reviews."reviewSummation" AS rs WHERE rs."submissionId" = s.id AND rs."isProvisional" IS TRUE - AND rs."aggregateScore" >= 0 ORDER BY COALESCE(rs."reviewedDate", rs."createdAt") DESC NULLS LAST, rs.id DESC LIMIT 1 ) AS provisional_review ON TRUE diff --git a/sql/reports/challenges/winners.sql b/sql/reports/challenges/winners.sql index 0301be2..b45e236 100644 --- a/sql/reports/challenges/winners.sql +++ b/sql/reports/challenges/winners.sql @@ -24,7 +24,10 @@ submission_metrics AS ( 'FAILED_CHECKPOINT_REVIEW', 'DELETED' ) THEN NULL - WHEN provisional_review.provisional_score IS NOT NULL THEN provisional_review.provisional_score + WHEN provisional_review.has_provisional_review THEN CASE + WHEN provisional_review.provisional_score >= 0 THEN provisional_review.provisional_score + ELSE NULL + END WHEN s."initialScore"::double precision >= 0 THEN s."initialScore"::double precision ELSE NULL END AS provisional_score, @@ -60,11 +63,14 @@ submission_metrics AS ( LIMIT 1 ) AS final_review ON TRUE LEFT JOIN LATERAL ( - SELECT MAX(rs."aggregateScore") AS provisional_score + SELECT + TRUE AS has_provisional_review, + rs."aggregateScore" AS provisional_score FROM reviews."reviewSummation" AS rs WHERE rs."submissionId" = s.id AND rs."isProvisional" IS TRUE - AND rs."aggregateScore" >= 0 + ORDER BY COALESCE(rs."reviewedDate", rs."createdAt") DESC NULLS LAST, rs.id DESC + LIMIT 1 ) AS provisional_review ON TRUE ), winner_members AS MATERIALIZED ( diff --git a/src/reports/challenges/challenge-export-sql.spec.ts b/src/reports/challenges/challenge-export-sql.spec.ts index 66dc31f..ebc7157 100644 --- a/src/reports/challenges/challenge-export-sql.spec.ts +++ b/src/reports/challenges/challenge-export-sql.spec.ts @@ -34,22 +34,26 @@ describe("Challenge export SQL", () => { const sql = sqlLoader.load(sqlPath); expect(sql).toContain( - "WHEN provisional_review.provisional_score IS NOT NULL THEN provisional_review.provisional_score", + "WHEN provisional_review.has_provisional_review THEN CASE", + ); + expect(sql).toContain( + "WHEN provisional_review.provisional_score >= 0 THEN provisional_review.provisional_score", ); expect(sql).toContain( `WHEN s."initialScore"::double precision >= 0 THEN s."initialScore"::double precision`, ); + expect(sql).toContain("TRUE AS has_provisional_review"); expect(sql).toContain(`'FAILED_REVIEW'`); expect(sql).toContain(`'DELETED'`); }, ); it.each(challengeUserSqlPaths)( - "filters failed negative Marathon Match provisional scores in %s", + "guards failed Marathon Match provisional reviews before falling back in %s", (sqlPath) => { const sql = sqlLoader.load(sqlPath); - expect(sql).toContain(`AND rs."aggregateScore" >= 0`); + expect(sql).not.toContain(`AND rs."aggregateScore" >= 0`); }, );