Skip to content

Commit f7b0459

Browse files
committed
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.
1 parent e3be399 commit f7b0459

4 files changed

Lines changed: 30 additions & 12 deletions

File tree

sql/reports/challenges/submitters.sql

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,10 @@ submission_metrics AS (
2626
'FAILED_CHECKPOINT_REVIEW',
2727
'DELETED'
2828
) THEN NULL
29-
WHEN provisional_review.provisional_score IS NOT NULL THEN provisional_review.provisional_score
29+
WHEN provisional_review.has_provisional_review THEN CASE
30+
WHEN provisional_review.provisional_score >= 0 THEN provisional_review.provisional_score
31+
ELSE NULL
32+
END
3033
WHEN s."initialScore"::double precision >= 0 THEN s."initialScore"::double precision
3134
ELSE NULL
3235
END AS provisional_score,
@@ -63,11 +66,12 @@ submission_metrics AS (
6366
LIMIT 1
6467
) AS final_review ON TRUE
6568
LEFT JOIN LATERAL (
66-
SELECT rs."aggregateScore" AS provisional_score
69+
SELECT
70+
TRUE AS has_provisional_review,
71+
rs."aggregateScore" AS provisional_score
6772
FROM reviews."reviewSummation" AS rs
6873
WHERE rs."submissionId" = s.id
6974
AND rs."isProvisional" IS TRUE
70-
AND rs."aggregateScore" >= 0
7175
ORDER BY COALESCE(rs."reviewedDate", rs."createdAt") DESC NULLS LAST, rs.id DESC
7276
LIMIT 1
7377
) AS provisional_review ON TRUE

sql/reports/challenges/valid-submitters.sql

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,10 @@ submission_metrics AS (
2626
'FAILED_CHECKPOINT_REVIEW',
2727
'DELETED'
2828
) THEN NULL
29-
WHEN provisional_review.provisional_score IS NOT NULL THEN provisional_review.provisional_score
29+
WHEN provisional_review.has_provisional_review THEN CASE
30+
WHEN provisional_review.provisional_score >= 0 THEN provisional_review.provisional_score
31+
ELSE NULL
32+
END
3033
WHEN s."initialScore"::double precision >= 0 THEN s."initialScore"::double precision
3134
ELSE NULL
3235
END AS provisional_score,
@@ -67,11 +70,12 @@ submission_metrics AS (
6770
LIMIT 1
6871
) AS final_review ON TRUE
6972
LEFT JOIN LATERAL (
70-
SELECT rs."aggregateScore" AS provisional_score
73+
SELECT
74+
TRUE AS has_provisional_review,
75+
rs."aggregateScore" AS provisional_score
7176
FROM reviews."reviewSummation" AS rs
7277
WHERE rs."submissionId" = s.id
7378
AND rs."isProvisional" IS TRUE
74-
AND rs."aggregateScore" >= 0
7579
ORDER BY COALESCE(rs."reviewedDate", rs."createdAt") DESC NULLS LAST, rs.id DESC
7680
LIMIT 1
7781
) AS provisional_review ON TRUE

sql/reports/challenges/winners.sql

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,10 @@ submission_metrics AS (
2424
'FAILED_CHECKPOINT_REVIEW',
2525
'DELETED'
2626
) THEN NULL
27-
WHEN provisional_review.provisional_score IS NOT NULL THEN provisional_review.provisional_score
27+
WHEN provisional_review.has_provisional_review THEN CASE
28+
WHEN provisional_review.provisional_score >= 0 THEN provisional_review.provisional_score
29+
ELSE NULL
30+
END
2831
WHEN s."initialScore"::double precision >= 0 THEN s."initialScore"::double precision
2932
ELSE NULL
3033
END AS provisional_score,
@@ -60,11 +63,14 @@ submission_metrics AS (
6063
LIMIT 1
6164
) AS final_review ON TRUE
6265
LEFT JOIN LATERAL (
63-
SELECT MAX(rs."aggregateScore") AS provisional_score
66+
SELECT
67+
TRUE AS has_provisional_review,
68+
rs."aggregateScore" AS provisional_score
6469
FROM reviews."reviewSummation" AS rs
6570
WHERE rs."submissionId" = s.id
6671
AND rs."isProvisional" IS TRUE
67-
AND rs."aggregateScore" >= 0
72+
ORDER BY COALESCE(rs."reviewedDate", rs."createdAt") DESC NULLS LAST, rs.id DESC
73+
LIMIT 1
6874
) AS provisional_review ON TRUE
6975
),
7076
winner_members AS MATERIALIZED (

src/reports/challenges/challenge-export-sql.spec.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,22 +34,26 @@ describe("Challenge export SQL", () => {
3434
const sql = sqlLoader.load(sqlPath);
3535

3636
expect(sql).toContain(
37-
"WHEN provisional_review.provisional_score IS NOT NULL THEN provisional_review.provisional_score",
37+
"WHEN provisional_review.has_provisional_review THEN CASE",
38+
);
39+
expect(sql).toContain(
40+
"WHEN provisional_review.provisional_score >= 0 THEN provisional_review.provisional_score",
3841
);
3942
expect(sql).toContain(
4043
`WHEN s."initialScore"::double precision >= 0 THEN s."initialScore"::double precision`,
4144
);
45+
expect(sql).toContain("TRUE AS has_provisional_review");
4246
expect(sql).toContain(`'FAILED_REVIEW'`);
4347
expect(sql).toContain(`'DELETED'`);
4448
},
4549
);
4650

4751
it.each(challengeUserSqlPaths)(
48-
"filters failed negative Marathon Match provisional scores in %s",
52+
"guards failed Marathon Match provisional reviews before falling back in %s",
4953
(sqlPath) => {
5054
const sql = sqlLoader.load(sqlPath);
5155

52-
expect(sql).toContain(`AND rs."aggregateScore" >= 0`);
56+
expect(sql).not.toContain(`AND rs."aggregateScore" >= 0`);
5357
},
5458
);
5559

0 commit comments

Comments
 (0)