Skip to content

docs: add a regression check against the latest release tag to bug triage - #5924

Open
andygrove wants to merge 1 commit into
apache:mainfrom
andygrove:triage-regression-check
Open

andygrove wants to merge 1 commit into
apache:mainfrom
andygrove:triage-regression-check

Conversation

@andygrove

@andygrove andygrove commented Sep 14, 2026

Copy link
Copy Markdown
Member

Which issue does this PR close?

Closes #5925.

Rationale for this change

Triage currently records how bad a bug is but not whether it is new. A defect that shipped in the last release and one introduced since it read identically on the issue tracker, even though the second one hits users who upgrade without changing anything on their side. There is no label for that distinction today and no procedure for establishing it.

This came out of a pass over the 19 open priority:critical issues, checking each against 1.0.0. Eighteen turned out to be pre-existing. The one that was not — #5783 — was only identifiable by building the tag and running the reproducer, because the behavior change came from removing a assert_eq! guard in #5602: 1.0.0 failed loudly on a struct with duplicate Parquet field names, and main silently returns 6 rows where Spark returns 3. Nothing short of running it would have found that, which is why the procedure below ends where it does.

What changes are included in this PR?

A regression label, documented in bug_triage.md, plus the procedure for deciding it:

  • The comparison point is resolved from the release tags at triage time rather than hard-coded, so it moves forward on its own as Comet ships. It is the tag's commit date, not the release's publication date — 1.0.0 was tagged 2026-08-04 and published 2026-08-07, and commits landing in that window are not in the release.
  • The evidence steps are ordered cheapest-first: the issue's creation date settles most of the backlog for free, then reading the implicated code at the tag, then checking whether the path was reachable at all, and only then building the tag and running the reproducer.
  • regression is orthogonal to priority and is added as an escalation trigger rather than a priority of its own.

Two traps are called out explicitly, both of which came up in the pass:

  • Issues found during PR review routinely say "this is pre-existing, not caused by this PR". That is a claim about the pull request under review, not about the last release; a defect can be genuinely pre-existing relative to the PR that surfaced it and still have landed after the tag.
  • A query that failed loudly on the release and now returns silently wrong data is a regression, even though it never produced the right answer on either version. That is exactly the Native Parquet scan multiplies rows for a struct with duplicate field names #5783 shape, and a narrower definition would have missed it.

The bug-triage skill gets the matching step: it runs the check for every issue it classifies as a bug, requires a Regression: line per bug in its summary issue, and collects everything it labelled into one section so a release manager can read them without scanning the priority sections. It applies the label only on positive evidence — an inconclusive result is recorded as unclear rather than guessed, and does not otherwise change how the issue is triaged.

How are these changes tested?

Documentation and skill instructions only; there is no code to test. The procedure was exercised against all 19 open priority:critical issues before being written down, including building 1.0.0 and main and running reproducers for the two cases that code reading could not settle (#5783, #5456). Markdown is prettier-clean.

…iage

Triage currently records how bad a bug is but not whether it is new. A defect
that shipped in the last release and one introduced since it read identically
on the issue tracker, even though the second one hits users who upgrade without
changing anything on their side.

Add a `regression` label and the procedure for deciding it. The comparison
point is resolved from the release tags at triage time rather than hard-coded,
so it moves forward on its own as Comet ships, and it is the tag's commit date
rather than the release's publication date because commits landing between the
two are not in the release.

The procedure is ordered cheapest-evidence-first: the issue's creation date
settles most of the backlog for free, then reading the implicated code at the
tag, then checking whether the path was reachable at all, and only then
building the tag and running the reproducer. That last step is the only way to
settle a case that turns on a dependency bump rather than on Comet's own code.

Two traps are called out explicitly. Issues found during PR review routinely
say "this is pre-existing, not caused by this PR", which is a claim about the
pull request under review and not about the last release. And a query that
failed loudly on the release and now returns silently wrong data is a
regression, even though it never produced the right answer on either version.

Update the bug-triage skill to run the check and to report regression status
per bug in its summary issue.
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Sep 14, 2026

@sunchao sunchao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Correctness

Reviewed 812b4c318137 against 481aefea9c60. The existing triage flow assigns type, priority and area labels. This change adds a latest-release comparison, a regression label and evidence in the summary issue. Resolving the baseline at triage time and distinguishing release regressions from defects that predate the PR under review are useful additions.

I found two P2 issues in the ordered decision procedure, detailed inline. An issue's creation date does not prove that the release contains the affected behavior. Identical implicated source also does not prove that the workload reached it on the release. Both currently terminate the investigation before the later checks that could identify a regression.

The Spark compatibility concern here is classification: a query that previously used Spark fallback and now returns a wrong native result must remain eligible for regression. The new definition includes that case, but the unchanged-code shortcut can exclude it. Both changed files are triage documentation. They add no expression, operator, type rule, null/overflow handling or ANSI-mode behavior. No version-specific Spark implementation change requires a semantic comparison.

Validation

Read both complete files and the full authored diff. The live latest stable release resolves to 1.0.0, commit 3a7a2c437cc7, dated August 4 and published August 7. Its history diverges from the reviewed head. I also ran small constructed counterexamples for the decision rules, including an unchanged kernel with different reachability and an unchanged wrapper with a different dependency. These validate the procedural concern. They are not Comet/Spark execution or a reclassification of the issue backlog.

CI preflight passed the license and Markdown-format checks on merge commit 69d8e29242a4. Its parents are the assigned base/head and its tree equals the reviewed head. The snapshot has seven successful checks, twelve skipped checks and one canceled older title check, with the replacement title check successful. Product builds, Spark suites and benchmarks were skipped. I ran no local product build or benchmark. The author's reported release/main reproductions and 19-issue triage results remain separately unverified.

Performance

The change adds no runtime overhead. Ordering cheap source inspection before a release build is reasonable for triage cost, and resolving one baseline for the batch avoids repeated version selection. Those savings require conclusive evidence before an early exit. Keeping age and file identity as hints still permits cheap exclusions when released behavior is established. No runtime performance claim or new expression requires a microbenchmark here.

Design

Keeping regression separate from priority and recording the baseline plus per-bug evidence makes the decision reviewable. The unclear outcome is useful when reproduction is unavailable, and preserving existing regression labels avoids silently overturning earlier triage. The necessary improvement is to make each definitive negative answer depend on released behavior. Issue age, code presence, reachability and dependencies can guide that investigation, but the first two cannot independently settle it.

Abstraction & complexity

This extends the existing skill and contributor guide without adding tooling, a new label family or a second priority mechanism. The summary table exposes the evidence behind each decision. The guide is explicitly the source of truth, so the two early-exit fixes should be applied consistently in both files. The existing yes/no/unclear representation is sufficient once its decision conditions are sound.

Comment on lines +120 to +121
1. **Issue creation date.** An issue opened before the tag was cut describes behavior that shipped
in that release. Not a regression.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Correctness

[P2] Could we use the issue date only as a hint and verify the affected behavior at the tag before concluding not a regression? A bug can be reported against main before the release commit is cut on a separate branch, without that change being included in the release. The current 1.0.0 tag and main do in fact have diverged histories. An older issue can also have been fixed in the release and then recur. This unconditional first exit skips every source/reproducer check and can miss the regression label and escalation for either case. Please make the same correction in Step 4b of the skill.

Comment on lines +122 to +124
2. **Is the defective code present at the tag?** `git show "$LATEST_RELEASE:<path>"`,
`git grep <pattern> "$LATEST_RELEASE"`, or `git diff "$LATEST_RELEASE"..HEAD -- <path>`. If the
defective logic is there verbatim, not a regression.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Correctness

[P2] Could we check released reachability before treating identical defective code as conclusive? An existing Rust kernel can be unreachable on the release because the expression falls back to Spark, then become reachable after a serde or support-level change. The kernel is still verbatim at the tag, so this rule stops at not a regression and never reaches the fallback-to-native case that the definition explicitly counts as a regression. The same shortcut can bypass the dependency comparison for unchanged callers. Please require evidence that the affected path actually behaved the same on the release, or continue as unclear, in both this guide and Step 4b of the skill.

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

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Track whether a bug is a regression from the most recent release

2 participants