Fix: Include all terminal labels when checking sibling completion status - #785
Merged
majamassarini merged 7 commits intoAug 31, 2026
Conversation
PR Summary by QodoRecognize all terminal labels during sibling consolidation
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
Code Review by Qodo
1.
|
![qodo-for-packit[bot]](https://avatars.githubusercontent.com/in/4066991?s=60&v=4){kind=small}
Member
Author
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit 122cef2 |
Previously, when checking if all siblings had finished triaging in
check_and_queue_primary_if_ready(), we only excluded issues with
"triaged" labels (ymir_triaged_*), but not those with completion
labels (ymir_backported, ymir_rebased, ymir_rebuilt) or error labels
(ymir_backport_errored, ymir_rebase_errored, etc.).
This caused primary issues to remain stuck waiting for siblings that
had already completed their work, as seen in RHEL-248139 where siblings
finished with ymir_backported and ymir_backport_errored labels.
The fix adds all terminal state labels to both:
1. check_and_queue_primary_if_ready() - so primary can proceed when all siblings are done
2. queue_siblings_for_triage() - for consistency and to avoid re-queueing completed siblings
Terminal states now include:
- Triage decisions: ymir_triaged_{rebase,backport,rebuild,not_affected,postponed}
- Completions: ymir_{backported,rebased,rebuilt}
- Errors: ymir_{backport,rebase,rebuild}_errored
- Failures: ymir_{backport,rebase,rebuild}_failed
Also adds test documentation for expected terminal label behavior.
Fixes: https://redhat.atlassian.net/browse/RHEL-248139
Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com>
After triage retry exhaustion, issues get TRIAGE_ERRORED label which is treated as terminal by triage deduplication. However, sibling discovery did not exclude it, causing siblings that exhausted triage retries to be counted as "pending", leaving primaries permanently blocked. Add TRIAGE_ERRORED to terminal_labels in both: - queue_siblings_for_triage() - avoid re-queueing exhausted siblings - check_and_queue_primary_if_ready() - unblock primary when siblings exhaust retries Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com>
…issue
Previously, terminal label filtering happened in Python after fetching up to
50 sibling candidates from Jira. This created a critical bug: if there were
more than 50 total candidates and many of them had terminal labels, the JQL
would return 50 results that included already-processed siblings, causing us
to miss actual pending siblings and potentially fail to queue them for triage.
For example, if there were 60 sibling candidates where 40 had terminal labels
and 20 were still pending:
- Old behavior: JQL returns first 50 (say 35 terminal + 15 pending), then
Python filters to 15 pending → we missed 5 real pending siblings
- New behavior: JQL excludes the 40 terminal ones and returns the 20 pending
The fix moves ALL terminal label exclusions from post-query Python filtering
into the JQL query itself via build_rebase_siblings_jql():
Terminal labels now excluded in JQL:
- Triage decisions: ymir_triaged_{rebase,backport,rebuild,not_affected,postponed}
- Completions: ymir_{backported,rebased,rebuilt}
- Errors: ymir_{triage,backport,rebase,rebuild}_errored
- Failures: ymir_{backport,rebase,rebuild}_failed
- Sibling marker: ymir_rebase_sibling
Kept defensive post-query check for race conditions (Jira indexing delays).
Updated test to verify all terminal labels appear in generated JQL.
Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com>
…sions Previous test checked if labels existed in JiraLabels.all_labels(), which is circular - it doesn't verify the actual production code in build_rebase_siblings_jql() or the inline terminal_labels lists in queue_siblings_for_triage() and check_and_queue_primary_if_ready(). New behavioral tests verify: 1. JQL query actually excludes each category of terminal labels: - Triage decisions (triaged_*) - Completion labels (backported, rebased, rebuilt) - Error labels (triage_errored, backport_errored, etc.) - Failed labels (backport_failed, etc.) - Sibling marker (rebase_sibling) 2. Exclusions are in the JQL query itself (server-side), not post-query filtering, which is critical to avoid missing pending siblings when there are >50 total candidates Each test method verifies a specific category and will fail if someone removes a terminal label from the production code, preventing regressions like RHEL-248139. Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Code review identified critical bugs:
1. **Contradictory JQL in check_and_queue_primary_if_ready**:
- Was calling build_rebase_siblings_jql() which excludes ymir_rebase_sibling,
then adding AND labels = "ymir_rebase_sibling" → zero results
- Fix: Call with exclude_triaged=False, filter pending vs terminal separately
2. **Broke auto-retry for FAILED labels**:
- FAILED labels (backport/rebase_failed) are documented as "May auto-retry"
- Excluding them from sibling search broke the retry mechanism
- Fix: Only exclude ERRORED labels (which block retry), not FAILED
Per jira_label_workflow_routing.md:
- ERRORED labels (triage/backport/rebase_errored): Block retry, need human → terminal
- FAILED labels (backport/rebase_failed): May auto-retry → retriable
Changes:
- build_rebase_siblings_jql: Exclude ERRORED, include FAILED
- queue_siblings_for_triage: Defensive check excludes ERRORED, includes FAILED
- check_and_queue_primary_if_ready: Call with exclude_triaged=False, both
ERRORED and FAILED are terminal (won't proceed/block primary)
- Tests: Verify ERRORED excluded, FAILED included
Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com>
…elease The ymir_rebase_sibling label is a queueing marker, not a terminal triage state. Excluding it in build_rebase_siblings_jql() broke check_and_queue_primary_if_ready() because queued-but-not-started siblings were invisible to the readiness check. Before fix: - build_rebase_siblings_jql() excluded ymir_rebase_sibling in JQL - check_and_queue_primary_if_ready() added AND labels = "ymir_rebase_sibling" - Contradictory query (exclude X AND require X) returned zero results - Primary was released early while queued siblings were still pending After fix: - build_rebase_siblings_jql() does NOT exclude ymir_rebase_sibling - Queued siblings are correctly found by the pending query - Primary waits until all siblings (including queued ones) finish - queue_siblings_for_triage() still has defensive check to avoid re-queueing Changes: - Removed ymir_rebase_sibling from excluded list in build_rebase_siblings_jql() - Simplified check_and_queue_primary_if_ready() call (default params work now) - Updated test: ymir_rebase_sibling must NOT be in JQL exclusions - Added regression test proving queued siblings are found as pending Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com>
majamassarini
force-pushed
the
fix-sibling-consolidation-terminal-labels
branch
from
August 27, 2026 08:51
331ff07 to
52d5662
Compare
Member
Author
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit 52d5662 |
Per triage_agent.py: - Resolution.OPEN_ENDED_ANALYSIS maps to ymir_triaged (terminal, no automated follow-up) - Resolution.CLARIFICATION_NEEDED maps to ymir_needs_attention (blocked, needs human) These were missing from terminal label exclusions, allowing completed open-ended-analysis and clarification-needed issues to be re-queued as siblings. Changes: - Added JiraLabels.TRIAGED to JQL exclusions in build_rebase_siblings_jql() - Added JiraLabels.NEEDS_ATTENTION to JQL exclusions - Added both to defensive terminal check in queue_siblings_for_triage() - Extended test to verify both labels are excluded in JQL Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Member
Author
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit 4f69dd7 |
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
2 participants
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.
Problem
Primary issues stuck waiting for siblings that finished with
ymir_backported,ymir_backport_errored, etc. These weren't recognized as terminal states.Example: RHEL-248139 - all 7 siblings finished but primary remained stuck with
ymir_rebase_waiting_for_siblings.Root Causes Fixed
Missing terminal labels - Only checked
ymir_triaged_*, not completion/error labelsFixes: https://redhat.atlassian.net/browse/RHEL-248139