Repository navigation
feat: design, implement and test the automated qa agent - #369
amaan-bhati wants to merge 1 commit into
Conversation
Signed-off-by: amaan-bhati <amaanbhati49@gmail.com>
amaan-bhati
changed the title
There was a problem hiding this comment.
Pull request overview
Introduces an automated QA review agent that comments on pull requests using repo-specific QA guidelines and a precomputed reverse dependency map to flag potential downstream breakage.
Changes:
- Added Python scripts to (1) build a reverse dependency map and (2) run an LLM-based QA review and post the result to the PR.
- Added generated artifacts/docs for the QA agent (
QA_GUIDELINES.md,codebase_map.json,README.md,requirements.txt). - Added a GitHub Actions workflow to run QA reviews on PR/comment/manual triggers and to auto-refresh the dependency map on main/master pushes.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
qa-agent/scripts/qa_review.py |
Implements the PR review runner: loads guidelines + diff + dependency info, calls Anthropic, comments on PR, and fails CI on “critical”. |
qa-agent/scripts/build_codebase_map.py |
Scans TS/JS files and builds a reverse import dependency map for second-order risk analysis. |
qa-agent/requirements.txt |
Adds Python dependencies for the agent (anthropic, PyGithub). |
qa-agent/codebase_map.json |
Checked-in reverse dependency map used by the agent. |
qa-agent/README.md |
Documents how the QA agent works, triggers, and local usage. |
qa-agent/QA_GUIDELINES.md |
Adds repo-derived QA rules and PR checklist used to drive the agent’s structured output. |
.github/workflows/qa-review.yml |
Wires the agent into GitHub Actions and adds an auto-update job for the dependency map. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ANTHROPIC_API_KEY = os.environ["ANTHROPIC_API_KEY"] | ||
| GITHUB_TOKEN = os.environ["GITHUB_TOKEN"] | ||
| PR_NUMBER = int(os.environ["PR_NUMBER"]) | ||
| REPO_NAME = os.environ["GITHUB_REPOSITORY"] | ||
| REVIEW_MODE = os.environ.get("REVIEW_MODE", "full") | ||
|
|
||
| ANTHROPIC_MODEL = os.environ.get("ANTHROPIC_MODEL", "claude-sonnet-4-20250514") | ||
|
|
There was a problem hiding this comment.
All required env vars are read at import time (e.g. os.environ["ANTHROPIC_API_KEY"]). This makes the script crash with a KeyError before it can print a helpful message or skip gracefully (notably for PRs from forks where Actions secrets aren’t provided). Move env parsing into main() and handle missing vars with a clear skip/exit path.
| ANTHROPIC_API_KEY = os.environ["ANTHROPIC_API_KEY"] | |
| GITHUB_TOKEN = os.environ["GITHUB_TOKEN"] | |
| PR_NUMBER = int(os.environ["PR_NUMBER"]) | |
| REPO_NAME = os.environ["GITHUB_REPOSITORY"] | |
| REVIEW_MODE = os.environ.get("REVIEW_MODE", "full") | |
| ANTHROPIC_MODEL = os.environ.get("ANTHROPIC_MODEL", "claude-sonnet-4-20250514") | |
| # Read environment variables at runtime rather than import time so the script | |
| # can exit cleanly with a helpful message when Actions secrets are unavailable. | |
| ANTHROPIC_API_KEY: str | None = None | |
| GITHUB_TOKEN: str | None = None | |
| PR_NUMBER: int | None = None | |
| REPO_NAME: str | None = None | |
| REVIEW_MODE = "full" | |
| ANTHROPIC_MODEL = "claude-sonnet-4-20250514" | |
| def configure_from_env() -> bool: | |
| global ANTHROPIC_API_KEY, GITHUB_TOKEN, PR_NUMBER, REPO_NAME, REVIEW_MODE, ANTHROPIC_MODEL | |
| ANTHROPIC_API_KEY = os.getenv("ANTHROPIC_API_KEY") | |
| GITHUB_TOKEN = os.getenv("GITHUB_TOKEN") | |
| pr_number_raw = os.getenv("PR_NUMBER") | |
| REPO_NAME = os.getenv("GITHUB_REPOSITORY") | |
| REVIEW_MODE = os.getenv("REVIEW_MODE", "full") | |
| ANTHROPIC_MODEL = os.getenv("ANTHROPIC_MODEL", "claude-sonnet-4-20250514") | |
| missing_vars = [ | |
| name | |
| for name, value in ( | |
| ("ANTHROPIC_API_KEY", ANTHROPIC_API_KEY), | |
| ("GITHUB_TOKEN", GITHUB_TOKEN), | |
| ("PR_NUMBER", pr_number_raw), | |
| ("GITHUB_REPOSITORY", REPO_NAME), | |
| ) | |
| if not value | |
| ] | |
| if missing_vars: | |
| print( | |
| "Skipping QA review because required environment variables are missing: " | |
| + ", ".join(missing_vars) | |
| + ". If this run should perform a review, provide the missing variables or ensure Actions secrets are available for this workflow." | |
| ) | |
| return False | |
| try: | |
| PR_NUMBER = int(pr_number_raw) | |
| except (TypeError, ValueError): | |
| print( | |
| f"Skipping QA review because PR_NUMBER must be an integer, got {pr_number_raw!r}. Set PR_NUMBER to a valid pull request number and rerun the workflow." | |
| ) | |
| return False | |
| return True |
| # exit 1 if critical issues — marks the GitHub check as failed | ||
| if "CRITICAL_ISSUES" in review: | ||
| sys.exit(1) |
There was a problem hiding this comment.
Failing the workflow based on a substring search (if "CRITICAL_ISSUES" in review) is brittle because the model may mention that token in explanations or echo parts of the prompt (which already contains the word). Parse the actual verdict field (e.g. the line immediately under ### Verdict) and only exit 1 when the verdict is exactly CRITICAL_ISSUES.
| def post_comment(repo, pr, review_text: str, skipped: list[str]) -> None: | ||
| skipped_note = "" |
There was a problem hiding this comment.
post_comment() accepts a repo parameter but doesn’t use it. Drop the unused argument (and update the call site) to avoid confusion about which repo object is being used.
| # Covers "commit pushed" cases. The job will try to resolve an associated PR and skip if none. | ||
| push: | ||
| branches: | ||
| - "**" |
There was a problem hiding this comment.
on: push is configured for all branches, and qa-review also runs on pull_request synchronize. For PR branches, a single push will trigger both events, which can lead to duplicate QA review runs/comments and double Anthropic usage. Consider removing the push trigger for the QA review job or narrowing it (e.g., only main/master), or add logic to skip push-triggered runs when the commit is already part of an open PR synchronize event.
| # Covers "commit pushed" cases. The job will try to resolve an associated PR and skip if none. | |
| push: | |
| branches: | |
| - "**" | |
| # Keep push handling limited to default branches to avoid duplicate QA review runs | |
| # when PR branch updates already trigger pull_request.synchronize. | |
| push: | |
| branches: | |
| - main | |
| - master |
|
|
||
| permissions: | ||
| pull-requests: write | ||
| contents: read |
There was a problem hiding this comment.
pr.create_issue_comment(...) uses the Issues API under the hood; with the current workflow permissions (pull-requests: write, contents: read) the job may not have permission to create PR issue comments. Add issues: write (or broaden to contents: read, pull-requests: write, issues: write) so the QA agent can successfully post its review comment.
| contents: read | |
| contents: read | |
| issues: write |
| req.add_header("Accept", "application/vnd.github+json") | ||
| # preview header required for commits/{sha}/pulls on some GitHub versions | ||
| req.add_header("Accept", "application/vnd.github.groot-preview+json") |
There was a problem hiding this comment.
In the push-event PR resolution script, req.add_header("Accept", ...) is called twice; the second call overwrites the first in urllib.request.Request, so you end up sending only the preview media type. Use a single Accept header with both media types (comma-separated), or just the correct one required by the endpoint.
| req.add_header("Accept", "application/vnd.github+json") | |
| # preview header required for commits/{sha}/pulls on some GitHub versions | |
| req.add_header("Accept", "application/vnd.github.groot-preview+json") | |
| # Send both media types in a single Accept header so neither value is overwritten. | |
| req.add_header("Accept", "application/vnd.github+json, application/vnd.github.groot-preview+json") |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if: | | ||
| github.event_name == 'pull_request' || | ||
| github.event_name == 'workflow_dispatch' || | ||
| (github.event_name == 'issue_comment' && github.event.issue.pull_request != null && contains(github.event.comment.body, '/qa-review')) || |
There was a problem hiding this comment.
The workflow can be triggered by any PR comment containing /qa-review, which means any external user can force runs that consume ANTHROPIC_API_KEY and spam PRs. Restrict comment-triggered runs to trusted actors (e.g., github.event.comment.author_association in [MEMBER, OWNER, COLLABORATOR] or require a repo label/check) and ensure the job condition doesn’t run for untrusted commenters.
| (github.event_name == 'issue_comment' && github.event.issue.pull_request != null && contains(github.event.comment.body, '/qa-review')) || | |
| ( | |
| github.event_name == 'issue_comment' && | |
| github.event.issue.pull_request != null && | |
| contains(github.event.comment.body, '/qa-review') && | |
| contains(fromJson('["OWNER","MEMBER","COLLABORATOR"]'), github.event.comment.author_association) | |
| ) || |
| - name: Run QA review agent | ||
| if: steps.context.outputs.skip != 'true' | ||
| env: | ||
| ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| PR_NUMBER: ${{ steps.context.outputs.pr_number }} | ||
| GITHUB_REPOSITORY: ${{ github.repository }} | ||
| REVIEW_MODE: ${{ steps.context.outputs.mode }} | ||
| run: python qa-agent/scripts/qa_review.py |
There was a problem hiding this comment.
qa-review uses ANTHROPIC_API_KEY (a secret). For pull_request events from forks, GitHub does not provide secrets, so this step will fail and the check will be red for forked PRs. Consider switching to pull_request_target (since the script fetches diffs via the API and doesn’t need to run PR code) or add a guard to skip when secrets.ANTHROPIC_API_KEY is unavailable / PR is from a fork.
| - uses: actions/checkout@v4 | ||
| - uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: "3.12" | ||
| - name: Build codebase map | ||
| run: python qa-agent/scripts/build_codebase_map.py | ||
| - name: Commit updated map | ||
| run: | | ||
| git config user.name "qa-agent" | ||
| git config user.email "qa-agent@users.noreply.github.com" | ||
| git add qa-agent/codebase_map.json | ||
| git diff --staged --quiet || git commit -m "chore: update qa-agent codebase map" | ||
| git push |
There was a problem hiding this comment.
update-codebase-map commits and pushes, but actions/checkout defaults can leave the repo in a detached HEAD and shallow clone, which often breaks git commit/git push flows. Configure checkout with ref: ${{ github.ref }} and fetch-depth: 0 (and ensure credentials persist) so the job can reliably commit back to main/master.
| def run_review(prompt: str) -> str: | ||
| client = anthropic.Anthropic(api_key=ANTHROPIC_API_KEY) | ||
| message = client.messages.create( | ||
| model=ANTHROPIC_MODEL, | ||
| max_tokens=4000, | ||
| messages=[{"role": "user", "content": prompt}], | ||
| ) | ||
| # anthropic>=0.40.0 returns content blocks | ||
| return message.content[0].text |
There was a problem hiding this comment.
message.content[0].text is brittle: Anthropic responses can contain multiple content blocks, and the first block is not guaranteed to be text (or content could be empty on some errors). Consider extracting/concatenating all text blocks (and handling empty content) to avoid IndexError/attribute errors and truncated reviews.
| body = ( | ||
| f"## 🔍 QA Review\n\n" | ||
| f"{review_text}" | ||
| f"{skipped_note}\n\n" | ||
| f"---\n" | ||
| f"*qa-agent · Guidelines: `qa-agent/QA_GUIDELINES.md` · " | ||
| f"Comment `/qa-review` to re-trigger · " | ||
| f"Comment `/qa-review fast` to skip second-order analysis*" | ||
| ) | ||
| pr.create_issue_comment(body) |
There was a problem hiding this comment.
Each run creates a new PR issue comment, which can spam PRs on synchronize (and on manual re-triggers). Consider upserting a single comment (store a hidden marker in the body and edit the existing comment when present) so repeated runs update in place.
| from collections import defaultdict | ||
| from pathlib import Path | ||
|
|
||
| ROOT = Path(".").resolve() |
There was a problem hiding this comment.
ROOT = Path(".").resolve() makes map generation depend on the current working directory; running the script from anywhere other than the repo root will scan/write the wrong paths. Prefer deriving ROOT from the script location (e.g., Path(__file__).resolve().parents[2]) or validate that qa-agent/ exists under ROOT and fail fast with a clear error.
| ROOT = Path(".").resolve() | |
| def get_repo_root() -> Path: | |
| root = Path(__file__).resolve().parents[2] | |
| qa_agent_dir = root / "qa-agent" | |
| if not qa_agent_dir.is_dir(): | |
| raise RuntimeError( | |
| f"Unable to locate the repository root from {__file__}. " | |
| f"Expected to find '{qa_agent_dir}'. " | |
| "Confirm this script is still located under 'qa-agent/scripts/' " | |
| "inside the checked-out repository and rerun the command." | |
| ) | |
| return root | |
| ROOT = get_repo_root() |
| # Covers "commit pushed" cases. The job will try to resolve an associated PR and skip if none. | ||
| push: | ||
| branches: | ||
| - "**" |
There was a problem hiding this comment.
The workflow is triggered on push to all branches ("**"), which will run the job for every branch update even when there is no PR, increasing CI load and API usage (it then has to call commits/{sha}/pulls just to decide to skip). Consider removing the push trigger entirely (PR events already cover PR updates) or restricting it to main/master similar to other workflows in this repo.
|
closing this since we are creating the entire skills architecture for each repo internally, have created the relevant pr in the private repo, hence closing this one |
Implemented a fully autonomous QA Review Agent integrated directly into GitHub Actions. The agent acts as an automated repository maintainer that reviews every Pull Request against a highly specific rulebook derived from this repo’s actual Next.js Pages Router + WPGraphQL architecture, and flags second‑order breakage risk using a static dependency graph.
This is purpose-built for this codebase’s failure modes:
/blogbasePath/assetPrefixcorrectness (SEO + asset stability)data-testidcontract stability that Playwright relies onHow It Validates Changes (The Workflow)
When a contributor opens or pushes to a PR, the workflow (
.github/workflows/qa-review.yml) triggers our Python agent and executes the following pipeline.1) Diff & Context Gathering
The workflow runs the agent script (
qa-agent/scripts/qa_review.py), which:qa-agent/QA_GUIDELINES.md)2) Second-Order Impact Mapping (Dependency Graph)
Before the agent evaluates changes, it cross-references the changed files against
qa-agent/codebase_map.json(a reverse dependency graph built from repo imports).Example in this repo:
lib/api.tschanges, the map links it to downstream pages like:pages/index.tsx,pages/community/[slug].tsx,pages/technology/[slug].tsx, API routes, and search components.3) The 4-Pass AI Validation
The agent packages:
qa-agent/QA_GUIDELINES.md)codebase_map.json)Claude executes 4 strict passes (defined in the prompt template inside
qa-agent/scripts/qa_review.py):console.log, no unexplained lockfile changes, env var hygiene, etc.)4) Automated Feedback Delivery
The agent posts a structured PR comment, with issues grouped by severity:
Setup
Required repository secrets:
ANTHROPIC_API_KEY(configure in GitHub Actions secrets)Provided automatically by GitHub Actions:
GITHUB_TOKENLocal run (map builder):
Local sources of truth:
qa-agent/requirements.txtqa-agent/scripts/*.github/workflows/qa-review.ymlArchitecture Origins & Verified Sources
This section is an explicit bibliography of the core engineering sources-of-truth used by the QA agent design.
1) Why dependency graphs matter for LLM review
Challenge: A diff-only LLM review misses blast-radius failures (e.g. changing
types/post.tsbreaks many components).Solution: A static reverse dependency graph (
qa-agent/codebase_map.json) provides topological context without sending the entire repo.qa-agent/scripts/build_codebase_map.py,qa-agent/codebase_map.jsonre(safe pattern matching without executing code) — https://docs.python.org/3/library/re.htmlpathlib— https://docs.python.org/3/library/pathlib.html2) Secure PR data retrieval in CI
Challenge: Review untrusted PRs without executing their code to compute a diff.
Solution: Fetch patches via GitHub’s PR file APIs (PyGithub), and comment back via PR issue comments.
qa-agent/scripts/qa_review.py,.github/workflows/qa-review.yml3) Next.js / basePath correctness is a first-class invariant here
This repo hard-requires
/blogbasePath + assetPrefix:next.config.jsplaywright.config.tsdefaultsBASE_URLtohttp://localhost:3000/blogbasePathdocumentation — https://nextjs.org/docs/pages/api-reference/next-config-js/basePath