feat: automate yiacad PR review lane #18
Assignees
electron (Clément Saillant)
Clear assignees
electron (Clément Saillant)
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: electron-rare/Kill_LIFE#18
Reference in New Issue
Block a user
Summary
Verification
⚠️ Code review skipped — your organization's overage spend limit has been reached.
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.
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
/reviewand project shells can surface checks/evidence and optionally publish a sticky PR summary.Changes:
tools/cad/yiacad_backend_client.pyand persist richer CI run metadata (engine/summary/degraded reasons/timestamps).publishPullRequestSummarymutation.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
publishPullRequestSummarymutationREDIS_URL.cidir exists@@ -0,0 +39,4 @@actions: readchecks: readcontents: readpull-requests: writeThis workflow runs on
pull_requestwhile grantingpull-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 apull_request_targetjob that checks out the base ref (or otherwise avoids executing PR code), and keep PR builds/tests onpull_requestwith read-only permissions.@@ -0,0 +24,4 @@"Authorization": f"Bearer {token}","Accept": "application/vnd.github+json","X-GitHub-Api-Version": "2022-11-28",}github_request()does not send aUser-Agentheader. GitHub's API guidance expects a user agent, and some environments/proxies can reject requests without it. Add an explicitUser-Agent(e.g.,YiACAD-PR-Review/1.0) to make the integration more robust.@@ -0,0 +65,4 @@comments = comments_raw if isinstance(comments_raw, list) else []existing = find_existing_comment([comment for comment in comments if isinstance(comment, dict)], marker)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 (followingLinkheaders) until the marker is found, or querying in reverse chronological order and scanning until you hit the marker.@@ -24,2 +26,4 @@}const url = new URL(rawUrl);const db = url.pathname && url.pathname !== "/" ? Number(url.pathname.slice(1)) : 0;redisConnection()now throws whenREDIS_URLis unset, whileweb/workers/eda-worker.mjsstill defaults toredis://127.0.0.1:6379. This inconsistency makes local usage fragile (enqueue will fail unlessREDIS_URLis exported, but the worker will happily connect to localhost). Align the behavior by either restoring the localhost default here or requiringREDIS_URLin both places (with a clearer local-dev setup path).💡 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
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".
@@ -59,0 +72,4 @@}return {"source_path": str(path), "board": "", "schematic": ""}_run_json_commandraises on any non-zero return code, but YiACAD commands can legitimately return non-zero while still emitting a structured--json-outputcontract (for example blocked runs or direct-fallback paths). Becauserun_intentdepends on this helper andon_runexpects a payload, the KiCad plugin cannot surface blocked/error details and instead fails the action flow when those cases occur.Useful? React with 👍 / 👎.
This path calls
http_jsondirectly, but/projects/currentcan return HTTP 404 when no context snapshot exists.urllib.request.urlopenraisesHTTPErroron 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
feat/yiacad-pr-review-automation->mainfailedruntimehighblocking7c9c9169Assessment
GitHub Checks
in_progressyiacad-pr-reviewsuccessyiacad-evidence-packin_progressFirmware — ESP32-S3 buildsuccessFirmware — Unity tests (native)failureyiacad-backend-contractsfailurepython-stablefailurePlaywright E2Esuccessfirmware-lintin_progressHardware — KiCad ERC + exportssuccessyiacad-web-buildin_progresskicad-checkssuccesssecret_scanEvidence Packs
in_progressKiCad Exports: feat: automate yiacad PR review lane (run)failureEvidence Pack Validation: feat: automate yiacad PR review lane (run)in_progressYiACAD Product: feat: automate yiacad PR review lane (run)Changed Files
.github/agents/arch_mesh.md.github/agents/architect_agent.md.github/agents/doc_agent.md.github/agents/docs_research.md.github/agents/embedded_cad.md.github/agents/firmware.md.github/agents/firmware_agent.md.github/agents/hw_schematic_agent.md.github/agents/kill_life_bridge.md.github/agents/pm_agent.md.github/agents/pm_mesh.md.github/agents/qa_agent.mdNext Steps
Generated by YiACAD review lane from GitHub checks and tracked workflow evidence.
💡 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
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".
@@ -270,0 +738,4 @@if (pullRequest.changedFiles.length > 0) {lines.push(`### Changed Files`, ``);for (const filePath of pullRequest.changedFiles.slice(0, 8)) {lines.push(`- \`${filePath}\``);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 👍 / 👎.
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 leavesevidencePacksincomplete and can incorrectly downgrade/blocks recommendations downstream. Fetch all relevant pages (or a robust filtered set) before deciding evidence availability.Useful? React with 👍 / 👎.
💡 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
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".
In the
Discover schematics and PCBsstep of.github/workflows/kicad-exports.yml, the new ignore glob uses*.kicad_block/*(singular) instead of*/kicad_blocks/*, so schematics insidekicad_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 👍 / 👎.
@@ -0,0 +59,4 @@) -> dict[str, object]:comments_raw = github_request("GET",f"/repos/{repository}/issues/{pull_request_number}/comments?per_page=100",The sticky-comment updater only fetches
?per_page=100once 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.
Credits must be used to enable repository wide code reviews.