Skip to content

fix(elliott): broaden cutoff filter to accept any post-fix status at timestamp - #3274

Open
joepvd wants to merge 2 commits into
openshift-eng:mainfrom
joepvd:fix-sweep-cutoff-statuses
Open

fix(elliott): broaden cutoff filter to accept any post-fix status at timestamp#3274
joepvd wants to merge 2 commits into
openshift-eng:mainfrom
joepvd:fix-sweep-cutoff-statuses

Conversation

@joepvd

@joepvd joepvd commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • The bug sweep's cutoff filter previously required bugs to have been in VERIFIED state at the assembly basis timestamp. This excluded bugs that were already fixed (MODIFIED/ON_QA) at the cutoff but only verified later.
  • Now the historical check accepts MODIFIED, ON_QA, VERIFIED, RELEASE_PENDING, or CLOSED at the cutoff time, while the initial search still ensures the bug is currently VERIFIED.

Test plan

  • Existing unit tests pass (test_find_bugs_sweep_cli.py — 32 tests)
  • Manual validation: run elliott find-bugs:sweep against an assembly with a basis event and confirm bugs that were ON_QA/MODIFIED at cutoff but VERIFIED now are included

Summary by CodeRabbit

  • Bug Fixes

    • Historical bug sweeps now include issues that had reached any post-fix status by the cutoff date.
    • Cutoff reporting now accurately reflects the expanded eligibility criteria.
  • Improvements

    • Bug sweeps now run immediately after image rebasing, ensuring modified issues are updated before image builds.
    • Removed the redundant later sweep during pipeline cleanup.

@joepvd

joepvd commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

/hold

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 12, 2026
@openshift-ci

openshift-ci Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign ashwindasr for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: openshift-eng/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 681f37c1-bc64-4cb8-ae10-025f9d22d5d6

📥 Commits

Reviewing files that changed from the base of the PR and between c6b68a8 and 7dd8c77.

📒 Files selected for processing (1)
  • elliott/elliottlib/cli/find_bugs_sweep_cli.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • elliott/elliottlib/cli/find_bugs_sweep_cli.py

Included review availability: Your plan includes up to 12 reviews per rolling hour; 11 remain after this review.


Walkthrough

Historical cutoff filtering now accepts bugs in any post-fix status at the cutoff. The pipeline runs sweep_bugs after image rebase and before image builds.

Changes

Bug sweep flow

Layer / File(s) Summary
Historical cutoff filtering
elliott/elliottlib/cli/find_bugs_sweep_cli.py
Historical qualification now accepts ON_QA, VERIFIED, RELEASE_PENDING, and CLOSED. The cutoff log reflects this rule.
Post-rebase sweep timing
pyartcd/pyartcd/pipelines/ocp4_konflux.py
The pipeline documents and runs sweep_bugs after image rebase and before image builds. The later finalization call was removed.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 7dd8c

The PR broadens historical cutoff eligibility to include bugs fixed before the cutoff but verified later, while retaining the current VERIFIED requirement. No actionable merge-blocking risk remains beyond normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant rebase_and_build_images
  participant rebase_images
  participant sweep_bugs
  participant image_builds
  rebase_and_build_images->>rebase_images: rebase images
  rebase_and_build_images->>sweep_bugs: sweep MODIFIED bugs
  rebase_and_build_images->>image_builds: build images
Loading
🚥 Pre-merge checks | ✅ 10 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (10 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: broadening the cutoff filter to accept any post-fix status at the timestamp.
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.
No-Weak-Crypto ✅ Passed The PR changes bug-status filtering, logging, and pipeline ordering only; the added lines introduce no weak crypto, custom crypto, or secret/token comparisons.
Container-Privileges ✅ Passed The PR changes only two Python files. The diff adds no container/Kubernetes manifests or privilege settings such as privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, or allowPrivilegeEscalation.
No-Sensitive-Data-In-Logs ✅ Passed The diff adds only bug-status filtering and count/timestamp messages, plus moves an existing sweep call; it introduces no logging of passwords, tokens, PII, hostnames, or customer data.
No-Hardcoded-Secrets ✅ Passed The PR diff adds only status strings, comments, and a sweep call; scans found no hardcoded keys, tokens, passwords, private keys, credential URLs, or long base64 literals.
No-Injection-Vectors ✅ Passed The diff adds no SQL concatenation, eval/exec, pickle.loads, unsafe yaml.load, os.system, or dangerouslySetInnerHTML; the moved command uses create_subprocess_exec with an argument list.
Ai-Attribution ✅ Passed The PR description and two introduced commits do not mention AI use, and the introduced commits have no AI attribution trailers or Co-Authored-By entries.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@joepvd
joepvd force-pushed the fix-sweep-cutoff-statuses branch from 516958e to c6b68a8 Compare August 12, 2026 15:54

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

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 `@pyartcd/pyartcd/pipelines/ocp4_konflux.py`:
- Around line 624-626: Gate sweep_bugs() on a completed, eligible rebase rather
than calling it unconditionally after rebase_images(). Update rebase_images() to
return the successfully rebased scope, account for no-image builds, skip_rebase,
partial failures, and ONLY/EXCEPT strategies, and pass that scope to limit bug
updates or skip sweeping when no valid scope exists. Revise the surrounding
documentation and add regression tests covering each listed scenario.
🪄 Autofix

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: Repository: openshift-eng/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 7a532033-ccde-4699-a756-1d8f8a2bd97e

📥 Commits

Reviewing files that changed from the base of the PR and between 516958e and c6b68a8.

📒 Files selected for processing (2)
  • elliott/elliottlib/cli/find_bugs_sweep_cli.py
  • pyartcd/pyartcd/pipelines/ocp4_konflux.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • elliott/elliottlib/cli/find_bugs_sweep_cli.py

Comment on lines +624 to +626
Find MODIFIED bugs for the target-releases, and set them to ON_QA.
Called after rebase so that ON_QA is set before builds complete, making
the bug status a reliable indicator that the fix was included in the rebase.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Gate sweep_bugs() on an eligible rebase.

rebase_images() can return without rebasing when no images are selected or skip_rebase is enabled. It also returns after handling partial rebase failures. The unconditional call at Lines [906-910] then sweeps the whole group and can move MODIFIED bugs to ON_QA even when their fixes were not rebased.

Return the successfully rebased scope from rebase_images() and use it to limit the sweep, or skip the sweep unless the required rebase completed. Update the documentation to match this behavior. Add regression tests for no-image builds, skipped rebases, partial failures, and ONLY/EXCEPT strategies.

Also applies to: 906-911, 1079-1079

🤖 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 `@pyartcd/pyartcd/pipelines/ocp4_konflux.py` around lines 624 - 626, Gate
sweep_bugs() on a completed, eligible rebase rather than calling it
unconditionally after rebase_images(). Update rebase_images() to return the
successfully rebased scope, account for no-image builds, skip_rebase, partial
failures, and ONLY/EXCEPT strategies, and pass that scope to limit bug updates
or skip sweeping when no valid scope exists. Revise the surrounding
documentation and add regression tests covering each listed scenario.

@joepvd
joepvd force-pushed the fix-sweep-cutoff-statuses branch from c6b68a8 to 8381bf8 Compare August 17, 2026 07:50
joepvd added 2 commits August 17, 2026 10:58
…timestamp

The bug sweep's cutoff filter previously required bugs to have been in
VERIFIED state at the assembly basis timestamp. This was too strict: a
bug that was MODIFIED or ON_QA at the cutoff (already fixed, just not
yet verified) would be incorrectly excluded.

Now the historical check accepts MODIFIED, ON_QA, VERIFIED,
RELEASE_PENDING, or CLOSED at the cutoff time. The initial search still
ensures the bug is currently VERIFIED, so only bugs that are both
verified now AND were already fixed at the cutoff are swept.

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
Previously sweep_bugs (MODIFIED → ON_QA) ran in the finally block after
builds completed and images were synced. This meant a bug's ON_QA
status was set well after basis.time (build completion time), making it
unreliable as a signal for the cutoff filter in elliott find-bugs:sweep.

Moving the call to right after rebase_images ensures that by the time
builds complete (and basis.time is derived), all bugs whose fixes were
incorporated in the rebase are already ON_QA. This makes the cutoff
filter's check for "ON_QA at timestamp" semantically correct: a bug
was ON_QA at basis.time iff its fix was included in the rebase.

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
@joepvd
joepvd force-pushed the fix-sweep-cutoff-statuses branch from 8381bf8 to 7dd8c77 Compare August 17, 2026 09:06
@openshift-ci

openshift-ci Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

@joepvd: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/security 7dd8c77 link false /test security

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

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

Labels

do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant