Repository navigation
D-GLOBAL-WHERE P0 follow-up: fail parity gate on missing comparisons; EXPLAIN after cold (lab) - #1946
Merged
Merged
Conversation
…fter cold
Review fixes (Codex P2 x2):
- compare.py exits 1 on any missing comparison in either direction and on a
thread set other than {1,4}; DuckDB's W8 (W2 before DELETE) is checked
against ours W2. Verified: complete results exit 0, empty ours.json and a
threads=1-only run exit 1, a corrupted W8 answer exits 1.
- bench_duckdb.py collects EXPLAIN ANALYZE after measurement on a separate
connection, so it no longer pre-warms the cold figure (and the engine-time
profiling mode cannot turn the plan into JSON). The 20 affected records
(dr256_medium, multi_skewed, t=1) were re-measured and spliced; answers
identical, no cold verdict changed, one warm verdict WIN -> PARITY.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XAqKMNZqKXSaeb9g2ypywF
…mparisons) With W8 (W2 before DELETE) now checked against ours W2, the gate compares 360 answers, not 336. REPORT.md and the P0 board entry updated. Verified on this tree: complete data exit 0; empty ours, empty duckdb, a dropped ours record, missing thread-1 and missing thread-4 coverage, and a corrupted W8 answer each exit 1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XAqKMNZqKXSaeb9g2ypywF
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
AdaWorldAPI
pushed a commit
that referenced
this pull request
Nov 3, 2026
…ompare_big Same rule as the P0 fix (#1946): coverage is enforced in both directions, DuckDB results must cover exactly threads {1,4}, and any missing comparison exits 1 instead of becoming a note under an all-agree verdict. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XAqKMNZqKXSaeb9g2ypywF
AdaWorldAPI
marked this pull request as ready for review
November 3, 2026 21:20
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. |
AdaWorldAPI
pushed a commit
that referenced
this pull request
Nov 3, 2026
…ompare_big Same rule as the P0 fix (#1946): coverage is enforced in both directions, DuckDB results must cover exactly threads {1,4}, and any missing comparison exits 1 instead of becoming a note under an all-agree verdict. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XAqKMNZqKXSaeb9g2ypywF
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Lab only. No production change. This is a narrow follow-up to #1943, which merged before its review fix was pushed. It cherry-picks
b7abb74e6(as54db91208) onto freshmain; the cherry-pick applied without conflicts. One small commit then recomputes the coverage count.All changes are under
.claude/probes/2026-11-03-d-global-where/, plus the count in the P0 board entry.What changes
The fixes address the two Codex P2 findings on #1943. Benchmark semantics are unchanged: same SQL, same data, same measurement procedure for every cell.
duckdb/compare.py: exits 1 on any missing comparison, in either direction, and when the DuckDB results do not cover exactly threads {1, 4}. DuckDB'sW8is W2 measured before the DELETE, so its answer is now checked against oursW2; those 24 comparisons were previously skipped.duckdb/bench_duckdb.py:EXPLAIN ANALYZEnow runs aftermeasure()andfloor_us(), on its own fresh connection. It therefore cannot pre-warm the cold figure, and the engine-time profiling mode set on the measuring connection cannot turn the plan text into JSON.results/duckdb.json: the 20 records affected by the pre-warm (dr256_mediumandmulti_skewedat threads=1) were re-measured with the fixed order and replaced; their answers are identical.multi_skewedW7i,route_each).results/explain/was regenerated, and the3,726,779intermediate-row figure still holds.REPORT.md: carries a correction note in §3.2; the two re-measured t=1 figures and the coverage count are updated.Verified on this tree
compare.pyours.jsonduckdb.jsonmulti_denseW4)run_one,measure(line 191) andfloor_us(192) run beforeEXPLAIN ANALYZE(201), which uses a separate connection (econ, line 198).results/duckdb.json, within 1 µs of rounding.No merge requested.
Generated by Claude Code