feat: automate yiacad PR review lane#18
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.
Once credits are available, push a new commit or reopen this pull request to trigger a review.
There was a problem hiding this comment.
Pull request overview
This PR wires the YiACAD “PR review lane” across the web worker, GraphQL read model, CI evidence-pack outputs, and GitHub-facing PR summary publishing so the /review and project shells can surface checks/evidence and optionally publish a sticky PR summary.
Changes:
- Route web EDA worker pipelines through
tools/cad/yiacad_backend_client.pyand persist richer CI run metadata (engine/summary/degraded reasons/timestamps). - Extend web GraphQL/types/UI to surface GitHub checks, evidence packs, PR assessment fields, and add a
publishPullRequestSummarymutation. - Add CI utilities to generate evidence packs + sticky PR summary comments, and introduce a new “YiACAD Product” workflow plus evidence-pack publishing in
kicad-exports.
Reviewed changes
Copilot reviewed 58 out of 58 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| web/workers/eda-worker.mjs | Route EDA pipelines through backend client; persist CI run metadata + artifact summaries |
| web/lib/types.ts | Expand snapshot types for checks/evidence packs/PR fields and CI run metadata |
| web/lib/graphql/schema.ts | Add new GraphQL fields and publishPullRequestSummary mutation |
| web/lib/graphql/client.ts | Update snapshot query + add PR summary publish mutation |
| web/lib/eda-queue.ts | Redis connection behavior changed to require REDIS_URL |
| web/lib/ci-enqueue.ts | Persist richer CI run metadata and ensure .ci dir exists |
| web/components/project-shell.tsx | UI: show PR/check/evidence summaries + richer CI run cards |
| web/components/pr-review-shell.tsx | UI: checks/evidence panels + publish-summary action |
| tools/test_python.sh | Add YiACAD contract tests to stable suite |
| tools/hw/* | Add KiCad Seeed MCP launcher/bridge/smoke and wire into fusion/proof tooling |
| tools/cockpit/* | Improve JSON-mode logging and add MCP smoke artifact to backend proof |
| tools/ci/* | Add evidence-pack + PR summary generator + sticky PR comment publisher |
| tools/cad/* | Harden engine baseline detection + add backend client surfaces and new backend actions |
| specs/* + specs/contracts/* | Update specs/contracts to reflect YiACAD app boundary + engine status/degraded reasons |
| .github/workflows/yiacad_product.yml | New workflow: contracts, web build, evidence pack, PR review + sticky comment |
| .github/workflows/kicad-exports.yml | Publish YiACAD evidence pack artifact for KiCad exports lane |
| function redisConnection() { | ||
| const url = new URL(process.env.REDIS_URL ?? "redis://127.0.0.1:6379"); | ||
| const rawUrl = process.env.REDIS_URL; | ||
| if (!rawUrl) { | ||
| throw new Error("REDIS_URL is not configured."); | ||
| } | ||
| const url = new URL(rawUrl); | ||
| const db = url.pathname && url.pathname !== "/" ? Number(url.pathname.slice(1)) : 0; |
There was a problem hiding this comment.
redisConnection() now throws when REDIS_URL is unset, while web/workers/eda-worker.mjs still defaults to redis://127.0.0.1:6379. This inconsistency makes local usage fragile (enqueue will fail unless REDIS_URL is exported, but the worker will happily connect to localhost). Align the behavior by either restoring the localhost default here or requiring REDIS_URL in both places (with a clearer local-dev setup path).
| actions: read | ||
| checks: read | ||
| contents: read | ||
| pull-requests: write |
There was a problem hiding this comment.
This workflow runs on pull_request while granting pull-requests: write, and it checks out + executes code from the PR branch (Python + shell + npm build). That combination is a security risk because a malicious PR can run arbitrary code with a write-capable token (e.g., alter PR state/comments or attempt token exfiltration). Consider splitting the PR-comment publishing into a pull_request_target job that checks out the base ref (or otherwise avoids executing PR code), and keep PR builds/tests on pull_request with read-only permissions.
| pull-requests: write | |
| pull-requests: read |
| comments_raw = github_request( | ||
| "GET", | ||
| f"/repos/{repository}/issues/{pull_request_number}/comments?per_page=100", | ||
| token, | ||
| ) | ||
| comments = comments_raw if isinstance(comments_raw, list) else [] | ||
| existing = find_existing_comment( | ||
| [comment for comment in comments if isinstance(comment, dict)], marker | ||
| ) |
There was a problem hiding this comment.
publish_comment() only fetches the first 100 PR comments (per_page=100) and doesn't paginate. On PRs with >100 comments, the existing sticky marker comment could be missed and the script would create a duplicate summary comment instead of updating the original. Consider paging through results (following Link headers) until the marker is found, or querying in reverse chronological order and scanning until you hit the marker.
| headers = { | ||
| "Authorization": f"Bearer {token}", | ||
| "Accept": "application/vnd.github+json", | ||
| "X-GitHub-Api-Version": "2022-11-28", | ||
| } |
There was a problem hiding this comment.
github_request() does not send a User-Agent header. GitHub's API guidance expects a user agent, and some environments/proxies can reject requests without it. Add an explicit User-Agent (e.g., YiACAD-PR-Review/1.0) to make the integration more robust.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30ec250a5c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if proc.returncode != 0: | ||
| raise RuntimeError(proc.stderr.strip() or proc.stdout.strip() or "YiACAD bridge failed") | ||
| raise RuntimeError(proc.stderr.strip() or proc.stdout.strip() or "YiACAD backend client failed") | ||
| return json.loads(proc.stdout.strip() or "{}") |
There was a problem hiding this comment.
_run_json_command raises on any non-zero return code, but YiACAD commands can legitimately return non-zero while still emitting a structured --json-output contract (for example blocked runs or direct-fallback paths). Because run_intent depends on this helper and on_run expects a payload, the KiCad plugin cannot surface blocked/error details and instead fails the action flow when those cases occur.
Useful? React with 👍 / 👎.
| response = http_json(service_url(args.host, args.port, "/projects/current")) | ||
| print(json.dumps(response, indent=2, ensure_ascii=False)) | ||
| return 0 if response.get("status") != "blocked" else 1 |
There was a problem hiding this comment.
This path calls http_json directly, but /projects/current can return HTTP 404 when no context snapshot exists. urllib.request.urlopen raises HTTPError on 404, so the command crashes instead of returning a controlled blocked result/exit code for fresh environments with no prior YiACAD run.
Useful? React with 👍 / 👎.
YiACAD PR Summary
Assessment
GitHub Checks
Evidence Packs
Changed Files
Next Steps
Generated by YiACAD review lane from GitHub checks and tracked workflow evidence. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a9d5bd346b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (evidencePacks.length === 0) { | ||
| rationale.push("No tracked evidence pack was found for the current PR head."); |
There was a problem hiding this comment.
This branch treats any attached evidence pack as sufficient, even when the pack status is failure, cancelled, or otherwise non-passing. In a CAD-affecting PR with green checks but a failed evidence workflow, the function still returns a favorable recommendation, which can mislead reviewers into merging without a successful CAD proof run. The recommendation should be based on passing evidence statuses, not just non-empty evidence.
Useful? React with 👍 / 👎.
| `/repos/${GITHUB_REPO}/actions/runs?${new URLSearchParams({ | ||
| head_sha: headSha, | ||
| event: "pull_request", | ||
| per_page: String(GITHUB_REVIEW_RUN_LIMIT) | ||
| }).toString()}` |
There was a problem hiding this comment.
The review loader only requests the first page of workflow runs with a small cap (per_page=8) and does not paginate. For PR heads with more runs than that, tracked workflows can be absent from this slice even when they succeeded, which leaves evidencePacks incomplete and can incorrectly downgrade/blocks recommendations downstream. Fetch all relevant pages (or a robust filtered set) before deciding evidence availability.
Useful? React with 👍 / 👎.
- Created INFRA_VPS_RUNBOOK_2026.md detailing operational procedures for monitoring VPS services. - Added infra_vps_healthcheck.sh script for automated health checks on DNS, TLS, TCP, and HTTP for VPS services. - Introduced infra_vps_security_audit.sh for non-intrusive security checks on external VPS services. - Established JSON schema for infra VPS inventory in infra_vps.schema.json. - Developed integration for runtime status reporting in the Next.js API route. - Implemented Playwright tests for smoke testing the application and ensuring core functionalities. - Updated Makefile for development dependencies and testing commands. - Created various test files for unit and end-to-end testing across different components.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea6d2b039f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| -not -path "*/.kicad_blocks/*" \ | ||
| -not -path "*/kicad_blocks/*" \ | ||
| -not -path "*.kicad_blocks/*" \ | ||
| -not -path "*.kicad_block/*" \ |
There was a problem hiding this comment.
In the Discover schematics and PCBs step of .github/workflows/kicad-exports.yml, the new ignore glob uses *.kicad_block/* (singular) instead of */kicad_blocks/*, so schematics inside kicad_blocks/ are no longer excluded. On repos that vendor reusable block libraries there, ERC will run on those library sources and can produce false failures/noise that block the export lane.
Useful? React with 👍 / 👎.
| ) -> dict[str, object]: | ||
| comments_raw = github_request( | ||
| "GET", | ||
| f"/repos/{repository}/issues/{pull_request_number}/comments?per_page=100", |
There was a problem hiding this comment.
The sticky-comment updater only fetches ?per_page=100 once before searching for <!-- yiacad-pr-summary -->. For PRs with more than 100 comments, the existing summary comment may be on a later page, so this path posts a new comment instead of updating the original one, breaking idempotent “single sticky summary” behavior.
Useful? React with 👍 / 👎.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Summary
Verification