Skip to content

[MINOR][cudf_polars] Validation summary skips query failures - #23508

Open
nirandaperera wants to merge 2 commits into
rapidsai:mainfrom
nirandaperera:fix/validation-summary-skipped-on-query-failure
Open

[MINOR][cudf_polars] Validation summary skips query failures#23508
nirandaperera wants to merge 2 commits into
rapidsai:mainfrom
nirandaperera:fix/validation-summary-skipped-on-query-failure

Conversation

@nirandaperera

Copy link
Copy Markdown
Contributor

Description

When there are query failures, the validation summary shows,

Validation Summary
==================
✅ All validated queries passed.

This is technically correct, but masks the fact that some queries failed. Need to inspect the json to find out what really happened. This is misleading.

This PR, fixes the reporting string when there are failed queries.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

Signed-off-by: niranda perera <niranda.perera@gmail.com>
@nirandaperera
nirandaperera requested review from a team as code owners August 2, 2026 18:45
@nirandaperera
nirandaperera requested review from rjzamora and vyasr August 2, 2026 18:45
@github-actions github-actions Bot added Python Affects Python cuDF API. cudf.pandas Issues specific to cudf.pandas cudf-polars Issues specific to cudf-polars labels Aug 2, 2026
@nirandaperera nirandaperera added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change and removed Python Affects Python cuDF API. cudf.pandas Issues specific to cudf.pandas cudf-polars Issues specific to cudf-polars labels Aug 2, 2026
@GPUtester GPUtester moved this to In Progress in cuDF Python Aug 2, 2026
@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved benchmark validation summaries to clearly report queries skipped because setup or execution failed.
    • Included affected query identifiers when validation is skipped due to execution failures.
    • Updated copyright notices to include “All rights reserved.”

Walkthrough

Changes

Benchmark validation reporting

Layer / File(s) Summary
Skipped-validation reporting
python/cudf/cudf/pandas/_benchmarks/utils.py, python/cudf_polars/cudf_polars/streaming/benchmarks/utils.py
Validation summaries report queries skipped because setup or execution failed, including their query IDs.
Copyright notice update
python/cudf/cudf/pandas/_benchmarks/utils.py
The SPDX copyright notice now includes “All rights reserved.”

Estimated code review effort: 1 (Trivial) | ~5 minutes

Suggested reviewers: vyasr, mroeschke

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: validation summaries now report query failures instead of skipping them.
Description check ✅ Passed The description explains the misleading validation output and the fix for reporting failed queries.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@python/cudf/cudf/pandas/_benchmarks/utils.py`:
- Around line 759-763: The validation summary currently uses mutually exclusive
branches, hiding skipped queries when validation failures exist. In both
`python/cudf/cudf/pandas/_benchmarks/utils.py` lines 759-763 and
`python/cudf_polars/cudf_polars/streaming/benchmarks/utils.py` lines 1235-1239,
make failure and skipped-query reporting independent, preserve the non-zero exit
code, and emit the success message only when both `validation_failures` and
`query_failures` are empty.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 428166eb-4614-4324-b17d-3f08d0d63ed6

📥 Commits

Reviewing files that changed from the base of the PR and between 3b3cea4 and 008da93.

📒 Files selected for processing (2)
  • python/cudf/cudf/pandas/_benchmarks/utils.py
  • python/cudf_polars/cudf_polars/streaming/benchmarks/utils.py

Comment on lines +759 to +763
elif query_failures:
print( # noqa: T201
"Validation was skipped for queries that failed to run: "
f"{sorted({q_id for q_id, _ in query_failures})}"
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Report both validation failures and skipped queries.

Both branches use elif, so mixed failures produce an incomplete validation summary. Keep the non-zero exit code, but also list every affected query category.

  • python/cudf/cudf/pandas/_benchmarks/utils.py#L759-L763: print skipped query IDs even when validation_failures is non-empty, and show the success message only when both failure collections are empty.
  • python/cudf_polars/cudf_polars/streaming/benchmarks/utils.py#L1235-L1239: apply the same independent reporting logic.
📍 Affects 2 files
  • python/cudf/cudf/pandas/_benchmarks/utils.py#L759-L763 (this comment)
  • python/cudf_polars/cudf_polars/streaming/benchmarks/utils.py#L1235-L1239
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@python/cudf/cudf/pandas/_benchmarks/utils.py` around lines 759 - 763, The
validation summary currently uses mutually exclusive branches, hiding skipped
queries when validation failures exist. In both
`python/cudf/cudf/pandas/_benchmarks/utils.py` lines 759-763 and
`python/cudf_polars/cudf_polars/streaming/benchmarks/utils.py` lines 1235-1239,
make failure and skipped-query reporting independent, preserve the non-zero exit
code, and emit the success message only when both `validation_failures` and
`query_failures` are empty.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

2 participants