Strengthen Copilot review instructions to use imperative phrasing#62584
Merged
kaxil merged 1 commit intoapache:mainfrom Feb 27, 2026
Merged
Strengthen Copilot review instructions to use imperative phrasing#62584kaxil merged 1 commit intoapache:mainfrom
kaxil merged 1 commit intoapache:mainfrom
Conversation
boring-cyborg
bot
added
area:dev-tools
backport-to-v3-1-test
…hrasing Copilot's review agent converts rules phrased as "Flag any X" into active code searches but treats passive phrasing as informational context. Rewrite all critical rules (imports, assert, time.time, lru_cache, session.commit, N+1 queries, unittest, unspec'd mocks) to use imperative directives so the agent reliably flags violations.
kaxil
force-pushed
the
strengthen-copilot-review-rules
branch
from
84678cb to
585fca8
Compare
potiuk
approved these changes
Feb 27, 2026
Backport failed to create: v3-1-test. View the failure log Run detailsNote: As of Merging PRs targeted for Airflow 3.X In matter of doubt please ask in #release-management Slack channel.
You can attempt to backport this manually by running: cherry_picker a064538 v3-1-testThis should apply the commit to the v3-1-test branch and leave the commit in conflict state marking After you have resolved the conflicts, you can continue the backport process by running: cherry_picker --continueIf you don't have cherry-picker installed, see the installation guide. |
AkshayArali
pushed a commit
to AkshayArali/airflow_630
that referenced
this pull request
Feb 28, 2026
…hrasing (apache#62584) Copilot's review agent converts rules phrased as "Flag any X" into active code searches but treats passive phrasing as informational context. Rewrite all critical rules (imports, assert, time.time, lru_cache, session.commit, N+1 queries, unittest, unspec'd mocks) to use imperative directives so the agent reliably flags violations.
dominikhei
pushed a commit
to dominikhei/airflow
that referenced
this pull request
Mar 11, 2026
…hrasing (apache#62584) Copilot's review agent converts rules phrased as "Flag any X" into active code searches but treats passive phrasing as informational context. Rewrite all critical rules (imports, assert, time.time, lru_cache, session.commit, N+1 queries, unittest, unspec'd mocks) to use imperative directives so the agent reliably flags violations.
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.
Copilot's review agent (
claude-sonnet-4.5-- this model can change) converts rules phrased as "Flag any X" into activesearch_dircalls against the changed files, but treats passive phrasing (e.g., "Imports at top of file") as informational context it may or may not act on.This was confirmed by analyzing GitHub Actions logs across PRs #62528 and #62582 — rules like
session.commit()and@lru_cache(already imperative) were actively searched, while the passively-phrased import rule was skipped despite reading the relevant code.Changes: Rewrite all critical rules to use imperative "Flag any..." directives:
assertin non-test codetime.time()for durations@lru_cachewithoutmaxsizesession.commit()in airflow-coreunittest.TestCasesubclassestime.sleep/datetime.now()in testsComplements #62442.