From 8b193b56f23f52f25169c37aba492c71d61cab35 Mon Sep 17 00:00:00 2001 From: David Blain Date: Mon, 12 Oct 2026 11:07:36 +0200 Subject: [PATCH 1/3] Ask agents to measure performance-sensitive components before pushing The publish skill's pre-push review now lists the components that run for every task, Task SDK call, log line or scheduler loop, and asks for a before/after measurement against the base commit, with a control, when a diff touches one of them. --- .../skills/airflow-publish-changes/SKILL.md | 34 +++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/.agents/skills/airflow-publish-changes/SKILL.md b/.agents/skills/airflow-publish-changes/SKILL.md index 8f115bea26fcd..8329d9aba1210 100644 --- a/.agents/skills/airflow-publish-changes/SKILL.md +++ b/.agents/skills/airflow-publish-changes/SKILL.md @@ -115,8 +115,10 @@ code review checklist in [`.github/instructions/code-review.instructions.md`](.. and fix any failures. The skipped hooks start long-running local servers or provision the opt-in Codex environment rather than run checks that complete. 6. Run relevant individual tests and confirm they pass. -7. Run `breeze verify` and execute every command it lists. The list comes from the same selective-checks logic CI uses. `breeze verify --full` shows everything CI runs for the default matrix cell, including the full suite CI adds when a change touches CI tooling or dependency files. Static checks are not listed: steps 4 and 5 cover them. Confirm the commands pass and check for CI-specific issues. -8. Check for security issues — no secrets, no injection vulnerabilities, no unsafe patterns. +7. If the diff touches a performance-sensitive component, measure it before and after the change as + described in [Performance-sensitive components](#performance-sensitive-components). +8. Run `breeze verify` and execute every command it lists. The list comes from the same selective-checks logic CI uses. `breeze verify --full` shows everything CI runs for the default matrix cell, including the full suite CI adds when a change touches CI tooling or dependency files. Static checks are not listed: steps 4 and 5 cover them. Confirm the commands pass and check for CI-specific issues. +9. Check for security issues — no secrets, no injection vulnerabilities, no unsafe patterns. Before pushing, always rebase your branch onto the latest target branch (usually `main`) to avoid merge conflicts and ensure CI runs against up-to-date code: @@ -160,6 +162,34 @@ Remind the user to: 2. Add a brief description of the changes at the top of the body. 3. Reference related issues when applicable (`closes: #ISSUE` or `related: #ISSUE`). +## Performance-sensitive components + +Some code runs for every task, every Task SDK call, every log line or every scheduler loop, so a +slowdown of a few microseconds there multiplies across every deployment. Passing tests do not show +it. Do not skip the measurement when the diff touches one of these: + +| Component | Path | Runs for | +|---|---|---| +| Task SDK comms, supervisor and task runner | `task-sdk/src/airflow/sdk/execution_time/` | every Variable, Connection and XCom call of a task | +| Execution API | `airflow-core/src/airflow/api_fastapi/execution_api/` | the server side of every Task SDK call | +| Logging and secret masking | `shared/logging/`, `shared/secrets_masker/` | every log line (see [#71721](https://github.com/apache/airflow/issues/71721)) | +| Serialization | `airflow-core/src/airflow/serialization/` | every Dag parse, every Dag load in the scheduler and API server | +| Scheduler and triggerer loops | `airflow-core/src/airflow/jobs/` | every loop iteration | +| Dag processing | `airflow-core/src/airflow/dag_processing/` | every Dag file parse | + +To measure: + +1. Time the changed code path on the base commit and on the change with the same script. Alternate + base and head runs, at least three of each, and compare the medians. Use `timeit` for calls + under a microsecond, and the helpers in `devel-common/src/tests_common/test_utils/perf/perf_kit/` + (`timing`, `repeat`, `profiled`, `trace_memory`) for longer ones. +2. In the same runs, time something the change does not touch as a control. If the control moved by + more than about 10%, the machine was noisy: run again before drawing conclusions. +3. For code that queries the database, also pin the number of queries in the tests with + `assert_queries_count` from `tests_common.test_utils.asserts`. +4. Put a before/after table in the PR description. If the change is slower, say by how much and + why that is acceptable. Do not commit the timing script unless a reviewer asks for it. + ## Golden rule: when a fix is imminent, open the PR, not an issue If you already know how to fix the problem and you (or the user) are going to From c23b2755562f2e892a2debfbbb3c74b2228d3d57 Mon Sep 17 00:00:00 2001 From: David Blain Date: Tue, 13 Oct 2026 00:16:15 +0200 Subject: [PATCH 2/3] Update .agents/skills/airflow-publish-changes/SKILL.md Co-authored-by: Dev-iL <6509619+Dev-iL@users.noreply.github.com> --- .agents/skills/airflow-publish-changes/SKILL.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.agents/skills/airflow-publish-changes/SKILL.md b/.agents/skills/airflow-publish-changes/SKILL.md index 8329d9aba1210..4f1db9629685a 100644 --- a/.agents/skills/airflow-publish-changes/SKILL.md +++ b/.agents/skills/airflow-publish-changes/SKILL.md @@ -115,8 +115,7 @@ code review checklist in [`.github/instructions/code-review.instructions.md`](.. and fix any failures. The skipped hooks start long-running local servers or provision the opt-in Codex environment rather than run checks that complete. 6. Run relevant individual tests and confirm they pass. -7. If the diff touches a performance-sensitive component, measure it before and after the change as - described in [Performance-sensitive components](#performance-sensitive-components). +7. If the diff touches [performance-sensitive components](#performance-sensitive-components), benchmark the code before and after the change and report the outcome to the user. 8. Run `breeze verify` and execute every command it lists. The list comes from the same selective-checks logic CI uses. `breeze verify --full` shows everything CI runs for the default matrix cell, including the full suite CI adds when a change touches CI tooling or dependency files. Static checks are not listed: steps 4 and 5 cover them. Confirm the commands pass and check for CI-specific issues. 9. Check for security issues — no secrets, no injection vulnerabilities, no unsafe patterns. From 6b225cc3221a0ed9c0b0e1c4a71c5d61f9cfe1bd Mon Sep 17 00:00:00 2001 From: David Blain Date: Thu, 15 Oct 2026 09:30:15 +0200 Subject: [PATCH 3/3] Move the performance-sensitive components into a skill reference Keep the component names in step 7 so the agent knows when to read the reference, and load the table and measuring steps only when needed. --- .../skills/airflow-publish-changes/SKILL.md | 34 +++---------------- .../performance-sensitive-components.md | 30 ++++++++++++++++ 2 files changed, 35 insertions(+), 29 deletions(-) create mode 100644 .agents/skills/airflow-publish-changes/references/performance-sensitive-components.md diff --git a/.agents/skills/airflow-publish-changes/SKILL.md b/.agents/skills/airflow-publish-changes/SKILL.md index 4f1db9629685a..1fd2c1246d9d4 100644 --- a/.agents/skills/airflow-publish-changes/SKILL.md +++ b/.agents/skills/airflow-publish-changes/SKILL.md @@ -115,7 +115,11 @@ code review checklist in [`.github/instructions/code-review.instructions.md`](.. and fix any failures. The skipped hooks start long-running local servers or provision the opt-in Codex environment rather than run checks that complete. 6. Run relevant individual tests and confirm they pass. -7. If the diff touches [performance-sensitive components](#performance-sensitive-components), benchmark the code before and after the change and report the outcome to the user. +7. If the diff touches a performance-sensitive component (Task SDK comms, supervisor and task runner; + Execution API; logging and secret masking; serialization; scheduler and triggerer loops; Dag + processing), benchmark the code before and after the change as + [`references/performance-sensitive-components.md`](references/performance-sensitive-components.md) + describes, and report the outcome to the user. 8. Run `breeze verify` and execute every command it lists. The list comes from the same selective-checks logic CI uses. `breeze verify --full` shows everything CI runs for the default matrix cell, including the full suite CI adds when a change touches CI tooling or dependency files. Static checks are not listed: steps 4 and 5 cover them. Confirm the commands pass and check for CI-specific issues. 9. Check for security issues — no secrets, no injection vulnerabilities, no unsafe patterns. @@ -161,34 +165,6 @@ Remind the user to: 2. Add a brief description of the changes at the top of the body. 3. Reference related issues when applicable (`closes: #ISSUE` or `related: #ISSUE`). -## Performance-sensitive components - -Some code runs for every task, every Task SDK call, every log line or every scheduler loop, so a -slowdown of a few microseconds there multiplies across every deployment. Passing tests do not show -it. Do not skip the measurement when the diff touches one of these: - -| Component | Path | Runs for | -|---|---|---| -| Task SDK comms, supervisor and task runner | `task-sdk/src/airflow/sdk/execution_time/` | every Variable, Connection and XCom call of a task | -| Execution API | `airflow-core/src/airflow/api_fastapi/execution_api/` | the server side of every Task SDK call | -| Logging and secret masking | `shared/logging/`, `shared/secrets_masker/` | every log line (see [#71721](https://github.com/apache/airflow/issues/71721)) | -| Serialization | `airflow-core/src/airflow/serialization/` | every Dag parse, every Dag load in the scheduler and API server | -| Scheduler and triggerer loops | `airflow-core/src/airflow/jobs/` | every loop iteration | -| Dag processing | `airflow-core/src/airflow/dag_processing/` | every Dag file parse | - -To measure: - -1. Time the changed code path on the base commit and on the change with the same script. Alternate - base and head runs, at least three of each, and compare the medians. Use `timeit` for calls - under a microsecond, and the helpers in `devel-common/src/tests_common/test_utils/perf/perf_kit/` - (`timing`, `repeat`, `profiled`, `trace_memory`) for longer ones. -2. In the same runs, time something the change does not touch as a control. If the control moved by - more than about 10%, the machine was noisy: run again before drawing conclusions. -3. For code that queries the database, also pin the number of queries in the tests with - `assert_queries_count` from `tests_common.test_utils.asserts`. -4. Put a before/after table in the PR description. If the change is slower, say by how much and - why that is acceptable. Do not commit the timing script unless a reviewer asks for it. - ## Golden rule: when a fix is imminent, open the PR, not an issue If you already know how to fix the problem and you (or the user) are going to diff --git a/.agents/skills/airflow-publish-changes/references/performance-sensitive-components.md b/.agents/skills/airflow-publish-changes/references/performance-sensitive-components.md new file mode 100644 index 0000000000000..decde26564056 --- /dev/null +++ b/.agents/skills/airflow-publish-changes/references/performance-sensitive-components.md @@ -0,0 +1,30 @@ + + +# Performance-sensitive components + +Some code runs for every task, every Task SDK call, every log line or every scheduler loop, so a +slowdown of a few microseconds there multiplies across every deployment. Passing tests do not show +it. Do not skip the measurement when the diff touches one of these: + +| Component | Path | Runs for | +|---|---|---| +| Task SDK comms, supervisor and task runner | `task-sdk/src/airflow/sdk/execution_time/` | every Variable, Connection and XCom call of a task | +| Execution API | `airflow-core/src/airflow/api_fastapi/execution_api/` | the server side of every Task SDK call | +| Logging and secret masking | `shared/logging/`, `shared/secrets_masker/` | every log line (see [#71721](https://github.com/apache/airflow/issues/71721)) | +| Serialization | `airflow-core/src/airflow/serialization/` | every Dag parse, every Dag load in the scheduler and API server | +| Scheduler and triggerer loops | `airflow-core/src/airflow/jobs/` | every loop iteration | +| Dag processing | `airflow-core/src/airflow/dag_processing/` | every Dag file parse | + +To measure: + +1. Time the changed code path on the base commit and on the change with the same script. Alternate + base and head runs, at least three of each, and compare the medians. Use `timeit` for calls + under a microsecond, and the helpers in `devel-common/src/tests_common/test_utils/perf/perf_kit/` + (`timing`, `repeat`, `profiled`, `trace_memory`) for longer ones. +2. In the same runs, time something the change does not touch as a control. If the control moved by + more than about 10%, the machine was noisy: run again before drawing conclusions. +3. For code that queries the database, also pin the number of queries in the tests with + `assert_queries_count` from `tests_common.test_utils.asserts`. +4. Put a before/after table in the PR description. If the change is slower, say by how much and + why that is acceptable. Do not commit the timing script unless a reviewer asks for it.