Repository navigation
[ci-scan-feedback] Add build-break KBE dedup variation - #133149
Conversation
Co-authored-by: Copilot App <[email protected]>
|
Azure Pipelines: Successfully started running 1 pipeline(s). 15 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @dotnet/runtime-infrastructure |
|
Azure Pipelines: Successfully started running 1 pipeline(s). 15 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
🟡 Changes recommended
The new variation text introduces an internal documentation inconsistency (separator rendering and range punctuation) that can mislead readers and should be corrected before merge.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a new “variation 8” to the create-kbe authoring instructions to improve deduplication for build breaks that don’t have a stable test/method identifier, by searching for a bounded invariant error phrase plus a bounded build-leg root in existing open KBEs.
Changes:
- Document a new “build-break invariant + leg root” search variation (variation 8) under “Search for an existing KBE”.
- Extend the per-variation summary paragraph to describe what variation 8 is intended to catch.
File summaries
| File | Description |
|---|---|
.github/workflows/shared/create-kbe.instructions.md |
Adds variation 8 guidance and updates the summary to cover build-break dedup when titles/prose drift. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
Co-authored-by: Copilot App <[email protected]>
There was a problem hiding this comment.
🟡 Changes recommended
Minor wording/spelling and an ambiguity in the new extraction guidance should be corrected to avoid inconsistent or incorrect signature/leg-root derivation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 3
- Review effort level: Lite
Co-authored-by: Copilot App <[email protected]>
There was a problem hiding this comment.
🔵 Needs a closer look
A newly introduced term in the operational instructions is undefined and can lead to inconsistent dedup behavior in practice.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
.github/workflows/shared/create-kbe.instructions.md:89
- The phrase “stable element” is introduced here but not defined elsewhere in the instructions. Since these are operational prompts, this ambiguity can lead to inconsistent behavior (e.g., different interpretations of what counts as “stable”). Consider rewording to reference the already-defined specificity criteria instead of the undefined term.
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
PureWeen
left a comment
PureWeen
left a comment
There was a problem hiding this comment.
Both original findings are addressed: the invariant/leg search now covers closed and recurrence paths, and an unbounded fragment skips only variation 8. One interaction remains in shared/create-kbe.instructions.md:199-200: the new unlabeled pair search records linked-tracker, which ci-failure-scan.md:343 treats as terminal: record and emit nothing. This conflicts with the shared guidance that plain trackers are not KBE substitutes.
For a stable actionable build break with a matching unlabeled tracker but no open or closed KBE and no matching PR, this newly reachable path suppresses creation of the labeled KBE needed by Build Analysis. The scanner contradiction predates this PR; the new pair-only search routes additional failures into it. PALTests is not a reproducer for this case because its closed predecessors independently trigger recurrence handling.
Please keep tracker-only hits as nonterminal cross-link context and align the scanner so Branch A creates a KBE with Tracking unless an actual KBE, PR, or independent skip condition suppresses it. Do not treat an unlabeled title match as an existing KBE.
Note
This review was generated by GitHub Copilot.
Co-authored-by: Copilot App <[email protected]>
|
@PureWeen Thanks for the feedback. I changed the scanner so a tracker-only match does not stop KBE creation. It keeps the tracker for the Note This response was generated by GitHub Copilot. |
There was a problem hiding this comment.
🟢 Approval recommended
The changes are documentation/workflow-guidance clarifications with low risk; only a minor wording ambiguity was found with an inline fix suggestion.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
.github/workflows/shared/create-kbe.instructions.md:89
- The phrase “each stable element” is undefined in this document, which can lead to inconsistent interpretation (and therefore inconsistent search behavior) when
ErrorMessageis an array. If the intent is simply to try each array entry independently, removing “stable” makes the guidance unambiguous.
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
PureWeen
left a comment
PureWeen
left a comment
There was a problem hiding this comment.
Holistic Review
Motivation: Tracker-only matches should preserve tracking context without suppressing the labeled KBE needed by Build Analysis.
Approach: The scanner now keeps linked-tracker nonterminal and adds the tracker link when creating a KBE. An actual KBE, handling PR, or independent skip condition still prevents emission.
Summary: LGTM from this focused code re-review at 8ce59c50. The remaining tracker-only suppression warning in my previous review is addressed. No new actionable findings; unlabeled trackers are not promoted into existing KBEs.
Scope: static code review only, not an executed workflow. CI/check status and merge readiness were not evaluated.
Note
This review was generated by GitHub Copilot.
Triggering problem
SYSTEM_ACCESSTOKENandJIT.Generics_roexamples.Proposed fix
.github/workflows/shared/create-kbe.instructions.mdfor build breaks without a test or method identifier.is:issue is:open label:"Known Build Error" in:body.ambiguous dup/weak signatureoutcomes rather than adding a new tally vocabulary.Why this works
The stable invariant phrase catches title and prose drift, while the leg root prevents unrelated build breaks sharing a generic diagnostic from colliding. Requiring both fragments in the same KBE body and refusing to guess among multiple candidates prevents false reuse; the prescribed query is expressible through the workflow's integrity-gated GitHub issue search.
Validation
github-agentic-workflows-compilepassed forci-failure-scan.mdandci-failure-fix.mdwith no compilation errors.in:bodysearch syntax was verified against the referenced KBE body shape using the authenticated GitHub API.Note
This pull request was generated by GitHub Copilot.