Skip to content

fix(ci): diagnostic ratchet skips cards not supported in baseline - #3571

Merged
matthewevans merged 1 commit into
mainfrom
ship/ci-diagnostic-ratchet-baseline-skip
Jun 17, 2026
Merged

fix(ci): diagnostic ratchet skips cards not supported in baseline#3571
matthewevans merged 1 commit into
mainfrom
ship/ci-diagnostic-ratchet-baseline-skip

Conversation

@matthewevans

Copy link
Copy Markdown
Member

The D-09 diagnostic-count ratchet in coverage-regression-check.sh classified
a newly-emitting diagnostic (e.g. target-fallback) as a real regression
whenever a card's parse_details differed from the baseline — but it never
applied the support-delta section's documented guard that "cards absent from
the baseline are skipped (new cards don't count as regressions)."

During heavy MTGJSON churn (the MSH/MSC Marvel sets + OM2, releasing
2026-06-26) cards are swapped in/out while total_cards stays flat, so the
count-based new-card allowance (curr_total - base_total) reads 0 even though
brand-new cards are present by identity. A new card has a null baseline
parse_details, which reads as "changed" and is miscounted as a real
regression. Likewise a card that was already unsupported cannot regress via a
diagnostic-category change.

Fix: restrict the newly-affected set to cards that were supported in the
baseline (mirrors the support-delta guard). Absent and already-unsupported
cards are skipped. The genuine supported->unsupported case is still caught
here (was_supported true) and independently by the engine bucket.

Verified: against a churn baseline, target-fallback +3 real -> +1; against a
CI-shaped baseline (the new preview cards unsupported), +3 all honesty-only,
exit 0. Engine-regression bucket still reports 0.

The D-09 diagnostic-count ratchet in coverage-regression-check.sh classified
a newly-emitting diagnostic (e.g. target-fallback) as a real regression
whenever a card's parse_details differed from the baseline — but it never
applied the support-delta section's documented guard that "cards absent from
the baseline are skipped (new cards don't count as regressions)."

During heavy MTGJSON churn (the MSH/MSC Marvel sets + OM2, releasing
2026-06-26) cards are swapped in/out while total_cards stays flat, so the
count-based new-card allowance (curr_total - base_total) reads 0 even though
brand-new cards are present by identity. A new card has a null baseline
parse_details, which reads as "changed" and is miscounted as a real
regression. Likewise a card that was already unsupported cannot regress via a
diagnostic-category change.

Fix: restrict the newly-affected set to cards that were supported in the
baseline (mirrors the support-delta guard). Absent and already-unsupported
cards are skipped. The genuine supported->unsupported case is still caught
here (was_supported true) and independently by the engine bucket.

Verified: against a churn baseline, target-fallback +3 real -> +1; against a
CI-shaped baseline (the new preview cards unsupported), +3 all honesty-only,
exit 0. Engine-regression bucket still reports 0.
@matthewevans
matthewevans enabled auto-merge June 17, 2026 07:22

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the coverage regression check script to exclude cards that were not supported in the baseline from the newly-affected set, preventing false positive regressions. The feedback suggests simplifying the jq expression by leveraging native truthiness instead of explicit boolean comparisons and default fallbacks.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +233 to +243
($b.cards // [] | map({key: .card_name, value: (.supported == true)}) | from_entries) as $bsup |
[
($c.diagnostics // {} | keys[]) as $cat |
($c.diagnostics[$cat] // 0) as $cc |
($b.diagnostics[$cat] // 0) as $bc |
select($cc > $bc) |
($b | cards_emitting($cat)) as $base_cards |
($c | cards_emitting($cat)) as $curr_cards |
(($curr_cards - $base_cards) | unique) as $newly |
# Only cards supported in the baseline can regress (see comment above):
# absent / already-unsupported cards are skipped, not counted.
(($curr_cards - $base_cards) | unique | map(select($bsup[.] // false))) as $newly |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

[MEDIUM] Simplify the jq expression by leveraging jq's native truthiness.

Why it matters: Using .supported directly and filtering with select($bsup[.]) is more idiomatic and readable than explicit boolean comparisons and default fallbacks.

Suggested fix: See the code suggestion below.

Suggested change
($b.cards // [] | map({key: .card_name, value: (.supported == true)}) | from_entries) as $bsup |
[
($c.diagnostics // {} | keys[]) as $cat |
($c.diagnostics[$cat] // 0) as $cc |
($b.diagnostics[$cat] // 0) as $bc |
select($cc > $bc) |
($b | cards_emitting($cat)) as $base_cards |
($c | cards_emitting($cat)) as $curr_cards |
(($curr_cards - $base_cards) | unique) as $newly |
# Only cards supported in the baseline can regress (see comment above):
# absent / already-unsupported cards are skipped, not counted.
(($curr_cards - $base_cards) | unique | map(select($bsup[.] // false))) as $newly |
($b.cards // [] | map({key: .card_name, value: .supported}) | from_entries) as $bsup |
[
($c.diagnostics // {} | keys[]) as $cat |
($c.diagnostics[$cat] // 0) as $cc |
($b.diagnostics[$cat] // 0) as $bc |
select($cc > $bc) |
($b | cards_emitting($cat)) as $base_cards |
($c | cards_emitting($cat)) as $curr_cards |
# Only cards supported in the baseline can regress (see comment above):
# absent / already-unsupported cards are skipped, not counted.
(($curr_cards - $base_cards) | unique | map(select($bsup[.]))) as $newly |

@matthewevans
matthewevans added this pull request to the merge queue Jun 17, 2026
Merged via the queue into main with commit a2ae630 Jun 17, 2026
10 checks passed
@matthewevans
matthewevans deleted the ship/ci-diagnostic-ratchet-baseline-skip branch June 17, 2026 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant