You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "andreahlert (via GitHub)" <gi...@apache.org> on 2026/04/01 16:27:44 UTC
[PR] [DRAFT] Add vault data layer for auto-triage [airflow]
andreahlert opened a new pull request, #64590:
URL: https://github.com/apache/airflow/pull/64590
## Summary
The auto-triage TUI currently fetches all PR data live from the GitHub API on every run. Author profiles are lost when the process exits, check status is re-queried for unchanged commits, and workflow runs make 4+ REST calls per PR with no caching. This adds up on larger batches and makes the tool slower than it needs to be.
This PR introduces a local vault layer that materializes triage data on disk so it can be reused across sessions. The vault sits between the existing code and the GitHub API: it tries a local lookup first and falls back to the API when data is missing or stale. No new external dependencies.
## Proposed approach
### Phase 1: Author profile persistence
Persist author profiles (account age, PR history, merge rates, contributed repos) to disk using the existing `CacheStore` pattern from `pr_cache.py`. Today these live only in an in-memory dict and are re-fetched every session. With persistence they survive across runs and only refresh after a configurable TTL (default 7 days).
### Phase 2: PR metadata vault
Materialize PR metadata (number, title, author, labels, head_sha, checks state) as structured files when the auto-triage processes them. On subsequent runs, known PRs are loaded from the vault and only refreshed if the head_sha changed. This eliminates redundant GraphQL calls for PRs that haven't been updated.
### Phase 3: Hybrid API/vault lookups
Replace the hottest API call paths with vault-first lookups:
- **Check status counts**: GraphQL per commit SHA, currently no caching. If the vault has results for the same SHA, skip the API call.
- **Workflow runs**: 4+ REST calls per PR, currently no caching. Materialize on first fetch, invalidate on SHA change.
A `--use-vault` flag enables the vault layer (opt-in). Without the flag, behavior is unchanged.
### Phase 4: Directed review questions (future)
Use vault data to generate verification questions for the LLM prompt: version field checks, test coverage detection, large PR warnings, breaking change signals. Instead of the LLM analyzing the diff from scratch, it receives pointed questions based on what the vault already knows about the PR and its context.
## Current status
This is a direction proposal. Looking for feedback on the approach before writing the implementation. Specifically:
- Does the phased approach make sense?
- Any concerns with the `CacheStore`-based persistence for the vault?
- Preference on `--use-vault` opt-in vs always-on with graceful fallback?
## Test plan
- Phase 1: Unit tests for profile persistence and TTL invalidation
- Phase 2: Unit tests for vault write/read/invalidation
- Phase 3: Integration test showing API call reduction with vault populated
- Each phase will be a separate commit for easy review
---
##### Was generative AI tooling used to co-author this PR?
- [ ] No
---
* Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
* For fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
* When adding dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add vault data layer for auto-triage [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #64590:
URL: https://github.com/apache/airflow/pull/64590#issuecomment-4215068587
> > Actually - the assumption here is wrong - the data is cached mostly - with 4 hr TTL already and stored in ~/.build directory. Or there is something wrong with it and we need to fix it?
>
> Maybe the PR description wasn't clear enough on this, I'll update it.
>
> Imho, No overlap actually. status_cache stores branch-level stuff (recent failures on main, canary builds) to detect systemic CI issues. Different key space.
>
> What I'm caching here is per-commit: check counts keyed by SHA with no TTL since completed checks never change for the same commit, saves the paginated GraphQL query on re-runs.
>
> Workflow runs with a 10min TTL (up to 3 REST calls per PR). Author profiles with 7d TTL since they barely change between sessions.
>
> Also added generate_review_questions which extracts deterministic checks from the local diff (large PR warnings, missing tests, version field validation) and feeds them into the LLM prompt without any extra API call.
Makes perfect sense ! Thanks for also updating the description - it was indeed the main source of confusion :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add vault data layer for auto-triage [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #64590:
URL: https://github.com/apache/airflow/pull/64590#issuecomment-4214808356
Actually - the assumption here is wrong - the data is cached mostly - with 4 hr TTL already and stored in ~/.build directory. Or there is something wrong with it and we need to fix it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add vault data layer for auto-triage [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #64590:
URL: https://github.com/apache/airflow/pull/64590
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add vault data layer for auto-triage [airflow]
Posted by "andreahlert (via GitHub)" <gi...@apache.org>.
andreahlert commented on PR #64590:
URL: https://github.com/apache/airflow/pull/64590#issuecomment-4215016258
> Actually - the assumption here is wrong - the data is cached mostly - with 4 hr TTL already and stored in ~/.build directory. Or there is something wrong with it and we need to fix it?
Maybe the PR description wasn't clear enough on this, I'll update it.
Imho, No overlap actually. status_cache stores branch-level stuff (recent failures on main, canary builds) to detect systemic CI issues. Different key space.
What I'm caching here is per-commit: check counts keyed by SHA with no TTL since completed checks never change for the same commit, saves the paginated GraphQL query on re-runs.
Workflow runs with a 10min TTL (up to 3 REST calls per PR). Author profiles with 7d TTL since they barely change between sessions.
Also added generate_review_questions which extracts deterministic checks from the local diff (large PR warnings, missing tests, version field validation) and feeds them into the LLM prompt without any extra API call.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add vault data layer for auto-triage [airflow]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #64590:
URL: https://github.com/apache/airflow/pull/64590#discussion_r3025320184
##########
dev/breeze/src/airflow_breeze/commands/pr_commands.py:
##########
@@ -1842,6 +1850,12 @@ def _fetch_author_profile(token: str, login: str, github_repository: str) -> dic
"contributed_repos_total": contrib_total,
}
_author_profile_cache[login] = profile
+
+ # Persist to disk for reuse across sessions
+ from airflow_breeze.utils.pr_cache import save_author_profile
+
+ save_author_profile(github_repository, login, profile)
Review Comment:
New imports were added inside `_fetch_author_profile`. Per project guidelines, keep imports at module scope unless there's a specific reason (circular import / worker isolation / lazy loading). Please move `save_author_profile` into the existing top-level `airflow_breeze.utils.pr_cache` import list and remove this in-function import.
##########
dev/breeze/src/airflow_breeze/utils/pr_cache.py:
##########
@@ -76,6 +76,7 @@ def save(self, github_repository: str, key: str, data: dict) -> None:
classification_cache = CacheStore("classification_cache")
triage_cache = CacheStore("triage_cache")
status_cache = CacheStore("status_cache", ttl_seconds=4 * 3600)
+author_cache = CacheStore("author_cache", ttl_seconds=7 * 24 * 3600)
Review Comment:
The PR description currently frames this as a direction proposal “before writing the implementation”, but the diff includes an implemented author-profile disk cache plus new tests. Please update the PR description (and/or title) to reflect the implemented Phase 1 scope so reviewers and release notes are aligned.
##########
dev/breeze/src/airflow_breeze/commands/pr_commands.py:
##########
@@ -1773,11 +1773,19 @@ def _fetch_single_pr_graphql(token: str, github_repository: str, pr_number: int)
def _fetch_author_profile(token: str, login: str, github_repository: str) -> dict:
"""Fetch author profile info via GraphQL: account age, PR counts, contributed repos.
- Results are cached per login so the same author is only queried once.
+ Results are cached in memory (per session) and on disk (across sessions, 7-day TTL).
"""
if login in _author_profile_cache:
return _author_profile_cache[login]
+ # Try disk cache before hitting the API
+ from airflow_breeze.utils.pr_cache import get_cached_author_profile
+
+ disk_profile = get_cached_author_profile(github_repository, login)
Review Comment:
New imports were added inside `_fetch_author_profile`. Project guidelines require imports at module scope (unless for circular imports / isolation / lazy loading). Since this module already imports from `airflow_breeze.utils.pr_cache` at the top, add `get_cached_author_profile` to that existing import block and remove this in-function import.
##########
dev/breeze/src/airflow_breeze/utils/pr_cache.py:
##########
@@ -131,6 +132,16 @@ def save_status_cache(github_repository: str, cache_key: str, payload: dict | li
status_cache.save(github_repository, cache_key, {"payload": payload})
+def get_cached_author_profile(github_repository: str, login: str) -> dict | None:
+ """Load a cached author profile. Returns None if missing or expired (7-day TTL)."""
+ return author_cache.get(github_repository, f"author_{login}")
+
+
+def save_author_profile(github_repository: str, login: str, profile: dict) -> None:
+ """Persist an author profile to disk."""
+ author_cache.save(github_repository, f"author_{login}", profile)
Review Comment:
`get_cached_author_profile` currently returns the raw JSON dict from `CacheStore`, which includes the internal `cached_at` field when TTL is enabled. That means callers get a different shape depending on whether the profile came from the API (no `cached_at`) or disk (has `cached_at`). Consider storing the profile under a dedicated key (e.g. `{ "profile": ... }`) and returning only that key, matching the pattern used by `status_cache` and avoiding leaking cache metadata into the runtime profile object.
```suggestion
data = author_cache.get(github_repository, f"author_{login}")
return data.get("profile") if data else None
def save_author_profile(github_repository: str, login: str, profile: dict) -> None:
"""Persist an author profile to disk."""
author_cache.save(github_repository, f"author_{login}", {"profile": profile})
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add vault data layer for auto-triage [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #64590:
URL: https://github.com/apache/airflow/pull/64590#issuecomment-4215074068
Good job :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org