Skip to content

Drop seed matrix cells that never produce committable bundles - #2971

Merged
joeharris76 merged 2 commits into
developfrom
fix/seed-matrix-drop-ineligible-cells
Oct 15, 2026
Merged

joeharris76 merged 2 commits into
developfrom
fix/seed-matrix-drop-ineligible-cells

Conversation

@joeharris76

@joeharris76 joeharris76 commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator

tpch/polars-df at sf0.01 and sf0.1 always validate uncertain through the
structural SQL-translation fallback, so their bundles are mirror-ineligible
and hand-dropped from every seed PR. tpch/datafusion at sf1.0 fails
deterministically when TPC-H Q18 exhausts the engine memory pool. Removing
all three cells keeps every remaining cell committable and every Seed run
green; the per-cohort platform test now pins the exact matrix.

Verification: uv run -- pytest tests/unit/workflows/test_seed_corpus_pr_base.py -q passes 11/11. Matrix goes from 18 to 15 cells; the three removed cells are tpch/polars-df sf0.01, tpch/polars-df sf0.1 (permanently uncertain, mirror-ineligible) and tpch/datafusion sf1.0 (deterministic Q18 memory-pool exhaustion confirmed in Seed run 38331461283 logs).

@benchbox-oracle benchbox-oracle Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

oracle-verdict: failure for 4079a7cfcb4edf9ac0438db49457d402823d960d

This result supersedes those for earlier heads; their open review threads still apply.

Tier: low-medium (data-only or small single-area change).

Decision: SHIP WITH FIXES.

Reviewer: muse (muse muse-spark-1.3-contributor, effort medium).

PR 2971 drops three TPC-H seed cells and rewrites the workflow test to expect two-identity TPC-H cohorts, contradicting the three-identity validator floor and the seed spec that still lists those cells as supported. Fix by restoring or replacing the third identity per affected cohort with evidence, and correcting the test/spec to enforce the floor.

Defects without a review thread

  • High .github/workflows/seed-corpus.yml:24-49: D1: Seed matrix leaves TPC-H cohorts below three-identity corpus floor
    After removal the seed workflow generates only two local identities for three TPC-H cohorts: SF 0.01 and SF 0.1 have only duckdb+datafusion, and SF 1.0 has only duckdb+clickhouse-local. results-data/validate_corpus.py sets MINIMUM_PLATFORMS_PER_COHORT=3 and shallow_cohorts() fails any cohort with 1-2 rankable identities, and results-data/SEED_CORPUS_SPEC.md requires three local identities per supported cohort so it stays publishable without cloud secrets. A monthly refresh of only these two cells cannot sustain the depth gate; TPC-H SF 0.01/0.1 have no other rankable third identity since sub-scale cloud bundles are unofficial and version-matrix coverage is SF 10. Restore the removed cells or add an admitted third local identity per affected cohort.

Defects outside the diff count like the others: fix them before merge.

Reviewers not available

  • luna: absent (error)

Comment thread tests/unit/workflows/test_seed_corpus_pr_base.py
@joeharris76

Copy link
Copy Markdown
Collaborator Author

D1 (seed matrix below the three-identity floor) is addressed in 4f1ba92: instead of dropping to two identities per affected cohort, the three ineligible cells are replaced with evidence-backed third identities, keeping all six cohorts at three monthly-refreshable local identities. Evidence: tpch/polars-df SF 0.01/0.1 rankable=False with validation_not_clean (corpus validator output; run-3093 bundles dropped as mirror-ineligible in f4ef449); tpch/datafusion SF 1.0 fails deterministically on Q18 memory-pool exhaustion (seed run 38331461283 logs). Replacements each have a rankable committed bundle with summary.validation=passed: clickhouse-local at TPC-H SF 0.01/0.1 and polars-df at TPC-H SF 1.0. The per-cohort floor assertion is restored and the spec records the ineligibility evidence. Requesting oracle re-review of the new head.

@benchbox-oracle benchbox-oracle Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

oracle-verdict: success for 4f1ba927dd0811a139e9ea976b82eb5b94b88a41

This result supersedes those for earlier heads; their open review threads still apply.

Tier: low-medium (data-only or small single-area change).

Decision: SHIP.

Reviewer: muse (muse muse-spark-1.3-contributor, effort medium).

All TPC-H cohorts now have three local identities and the matrix test plus spec document the ineligibility evidence and replacement coverage. No new merge-blocking correctness, gate, or security defects found in the three changed files.

Verified fixed: D1, D2. Resolve their review threads.

tpch/polars-df at sf0.01 and sf0.1 always validate uncertain through the
structural SQL-translation fallback, so their bundles are mirror-ineligible
and hand-dropped from every seed PR. tpch/datafusion at sf1.0 fails
deterministically when TPC-H Q18 exhausts the engine memory pool. Removing
all three cells keeps every remaining cell committable and every Seed run
green; the per-cohort platform test now pins the exact matrix.
Drop tpch/polars-df at SF 0.01/0.1 (always validation_not_clean via the
SQL-translation fallback; run-3093 bundles dropped as mirror-ineligible)
and tpch/datafusion at SF 1.0 (deterministic Q18 memory-pool exhaustion in
seed run 38331461283). Substitute clickhouse-local at SF 0.01/0.1 and
polars-df at SF 1.0, each backed by a rankable committed bundle, so every
cohort keeps three monthly-refreshable local identities. Restore the
per-cohort floor assertion and record the ineligibility evidence in the
seed corpus spec.
@joeharris76
joeharris76 force-pushed the fix/seed-matrix-drop-ineligible-cells branch from 4f1ba92 to 2f25685 Compare October 14, 2026 23:14
@joeharris76

Copy link
Copy Markdown
Collaborator Author

Rebased onto current develop in 2f25685: the visual-regression mismatch was renderer drift, not this diff. Six explorer/results changes merged after this branch's base (including #2941 stream exclusions, #2922 compare views, and #2913 execution variants), which repaint the /results/* pages the baseline captures. No content changes in the rebase; seed matrix, floor test, and spec evidence are unchanged. Requesting fresh CI and oracle review of the new head.

@benchbox-oracle benchbox-oracle Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

oracle-verdict: success for 2f25685a601b5ef8fb9f27d46724e2e0605dbca2

This result supersedes those for earlier heads; their open review threads still apply.

Tier: low-medium (data-only or small single-area change).

Decision: SHIP, carried from head 4f1ba927dd0811a139e9ea976b82eb5b94b88a41.

The patch is unchanged since head 4f1ba927dd0811a139e9ea976b82eb5b94b88a41 was reviewed, so its decision is carried to this head without running a reviewer. Its open review threads still apply.

@joeharris76
joeharris76 enabled auto-merge (squash) October 15, 2026 00:12
@joeharris76
joeharris76 merged commit b2096f6 into develop Oct 15, 2026
67 of 68 checks passed
@joeharris76
joeharris76 deleted the fix/seed-matrix-drop-ineligible-cells branch October 15, 2026 00:31

This branch was successfully deployed

1 active deployment
oracle — 2f25685a Deployed Oct 14, 2026 by joeharris76 via post #665
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