Repository navigation
fix/df parity promote nyctaxi tsbs - #2435
Conversation
Wire three STAGED_GATES builders on top of the builder-local ID maps. nyctaxi forces offline synthetic generation and fails closed on any network download. tpch_skew aligns the shared Q11 scale seam per run. tpcds_obt untouched per the abandoned verdict. No GATES promotion, no Make or CI wiring. Probe evidence at SF=0.01: skew generator discriminating (20/22, Q11 fixed, Q15-expression/Q17/Q8/Q20 findings recorded); nyctaxi cheapest cell SF=0.01 (12k trips) with param-seam divergences to burn down; tsbs_devops 15/18 discriminating with window-alignment divergences to burn down.
Wire tpch and tpcds STAGED_GATES on the Q-prefix maps. tpch runs unseeded-vs-unseeded at SF=0.01 (41/44 cells pass; fixed-stream needs seeded DF overrides first). tpcds full-99 staged at SF=0.01 (~13s wall); only Q38/Q88 pass, so the 10-15 blocking subset is deferred to the promotion item. Module docstring states the staged status and honest oracle limits. No GATES promotion, no CI wiring.
Reuse _assemble_simple_duckdb_cell for the TPC-H, TPC-DS, and TPC-H Skew builders so the duplicate-code delta gate stays flat. The TPC-H Skew scale-factor seam stays in its builder-local query lookup. Record the three thin per-benchmark delegations in the scoped duplicate ignore list.
Reuse _assemble_simple_duckdb_cell for the TPC-H, TPC-DS, and TPC-H Skew builders so the duplicate-code delta gate stays flat. The TPC-H unseeded-defaults rationale and scale seam stay in place. Record the three thin per-benchmark delegations in the scoped duplicate ignore list.
… baselines nyctaxi, tsbs_devops, and tpch_skew report-mode gates now pass with empty-or-classified baselines. Seeded query windows align both surfaces; Postgres DOW numbering fixed on both DataFrame backends; Q15 float tolerance, Q17 NULL preservation, and deterministic skew seeding land with the gate evidence. Vacuity classified with written reasons (airport-trips, 3 tsbs thresholds, skew Q8). Cheapest discriminating cell SF=0.01 recorded per gate.
Port the expression Q15 relative-tolerance max filter and both-family Q17 empty-set NULL preservation. The tpch staged gate now runs green (44/44 cells, 0 divergent, 0 vacuous) at SF=0.01 unseeded.
Move both builders from STAGED_GATES to GATES with clean burn-downs (zero unclassified divergences and vacuous cells). Add Make targets, pr.yml correctness-gate steps, and mutation targets for both. Regenerate the oracle coverage map. Pinning guard and mutation-sensitivity drift guard pass; targeted mutation tests for both gates pass.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
… into fix/df-parity-promote-nyctaxi-tsbs # Conflicts: # benchbox/core/equivalence/builders/nyctaxi.py # benchbox/core/equivalence/builders/tpch.py # benchbox/core/equivalence/builders/tpch_skew.py # benchbox/core/equivalence/builders/tsbs_devops.py # benchbox/core/equivalence/cross_surface.py
The staged-tpch-tpcds merge took the older builder without the set_scale_factor_for_benchmark call. tpch staged gate green again (44/44, 0 divergent).
![chatgpt-codex-connector[bot]](/s/com/githubusercontent/avatars/G.https/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 629590d5be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Both move from STAGED to GATES with clean burn-downs (zero unclassified divergences and vacuous cells). Add Make targets, pr.yml correctness-gate steps, and mutation targets for both. tpcds stays staged: only Q38/Q88 pass, so no honest blocking subset exists yet. Regenerate the oracle coverage map.
Add the tpcds staged Make target and a read-only weekly scheduled workflow that runs the ~13s full-99 report and records the passing count. No blocking gate, no subset claim: Q38/Q88 pass today, so the 10-15 blocking subset waits for DF maturation with reviewer sign-off.
…15 max, scoped overrides Normalize the NYC Taxi expression day_of_week and day_type math to the documented ISO weekday contract. Detect Q17 emptiness with a backend-native count aggregate instead of the Polars-only materializer. Match Q15's maximum exactly instead of a relative tolerance band, with a greatest-row fallback for ULP splits. Scope gate parameter overrides to the gate run with cleanup restore.
|
Maintainer review: addressed all four threads in 36ca4b9. Weekday math now uses the documented ISO contract ((iso+1)%7) on the expression paths, matching pandas; Q17 emptiness uses a backend-native count aggregate via ctx.scalar instead of the Polars-only materializer; Q15 matches the SQL exact-maximum semantics with a greatest-row fallback for ULP splits instead of a tolerance band; gate overrides are scoped with a cleanup restore invoked by run_gate. Threads resolved. Note: the branch also promotes nyctaxi/tsbs/tpch_skew/tpch to enforced gates, which will need stale generated artifacts (applicability report, coverage map, Makefile inventory) regenerated once the stacked predecessors merge. CI re-running; enqueuing after green. |
The NYC Taxi and TSBS DevOps override setters share a 14-line set/restore shape over per-benchmark module globals. Record the scoped ignore with rationale instead of growing the clone count.
Record the five new cross-surface report targets in the Makefile inventory and refresh the coverage artifact timestamp.
…sion in nyctaxi Add __mod__ and __rmod__ to UnifiedExpr, correct ISO weekday to Postgres DOW conversion in NYC Taxi DataFrame queries, and update oracle coverage map unit tests.
|
Status note (takeover sweep): the medium-test failure on this branch is a stale cross-surface applicability pin. This branch predates the staged tpch/tpcds gate registration landing via the current queue batch (2440 merged; 2427/2394/2441/2443 queued). After that batch drains, rebase this branch onto develop and run make cross-surface-applicability-report to refresh the artifact and pins, then re-push. No code fix needed. |
Develop already carries the staged builders and the parameter, weekday, and NULL fixes, so take its versions. This branch now promotes the TPC-H, TPC-H Skew, NYC Taxi, and TSBS DevOps cross-surface gates from STAGED_GATES to GATES (each runs clean against develop's builders), wires their Make targets and CI steps, and adds the weekly TPC-DS maturation report.
…ote-nyctaxi-tsbs # Conflicts: # _project/analysis/oracle-coverage-map.md
…outcome The four promoted cross-surface steps ran on both correctness-gate partitions; pin them to partition a, which carries fewer gates. The weekly TPC-DS report swallowed every failure with || true; use continue-on-error and record the step outcome in the summary.
|
Soundness review: external agy, gemini-3.1-pro-high, read-only. Reviewed the promotion of the TPC-H, TPC-H Skew, NYC Taxi, and TSBS DevOps cross-surface gates from STAGED_GATES to GATES, the new Make targets and ci.yml steps, the mutation-sensitivity targets, and the weekly TPC-DS maturation workflow against the merge base with develop. Each promoted gate was also run locally against the merged tree and exited 0 with no divergent cells (tpch 44/44 compared; tpch_skew 42/44 with Q8 classified empty; nyctaxi 48/50 with airport-trips classified empty; tsbs_devops with its three classified-empty threshold queries). Result: no Critical or High findings. One Medium: the four new gate steps had no partition condition and ran on both correctness-gate partitions. One Low: the maturation workflow used Hosted checks and the queue candidate must still pass. |
tpch, tpch_skew, nyctaxi, and tsbs_devops are now enforced, so only tpcds stays staged and only joinorder is not cheaply gateable. Regenerate the applicability report and list the new TPC-DS maturation workflow in the ledger.
…ote-nyctaxi-tsbs # Conflicts: # make/inventory.json
…literals Scalar aggregates that return a single all-NULL row now count as zero reference rows, so tpch_skew's Q17 is reported as an unclassified vacuous query once the gate is blocking. Q17's default literals (Brand#23, MED BOX) match no qualifying rows in the skewed cell at SF=0.01 with seed 42. Classify it with a rationale that states the actual scope: 496 of the 1000 brand and container combinations present in the cell return a non-NULL value, so the query is not vacuous in general, only at its default parameters. Gating Q17 with a pair the cell satisfies is tracked in the rationale.
The blocking tpch gate reports Q17 as an unclassified vacuous query: a scalar aggregate that returns one all-NULL row now counts as zero reference rows, and Q17's default literals (Brand#23, MED BOX) match no qualifying rows at SF=0.01. Classify it with a rationale that states the actual scope: 856 of the 1000 brand and container combinations present in the cell return a non-NULL value, so the query is not vacuous in general, only at its default parameters.
|
Read-only review with Measured facts the brief relied on, from running Q17's reference SQL over every brand and container pair in each built cell (25 x 40 = 1000 pairs): the High: classifying Q17 as legitimately empty leaves a blind spot. A classified query is only checked for both surfaces agreeing on the empty result. A broken join, correlated subquery or
Medium: the rationale says "Tracked", and nothing is filed; it also under-states that most pairs return all-NULL in No finding on the mechanism: the |
1 participant
Soundness review: