perf(ci): keep verdicts and trim checks - #5244
Conversation
e886be4 to
c99cb88
Compare
f2865b3 to
18a5fd0
Compare
18a5fd0 to
3f87033
Compare
hqhq1025
left a comment
There was a problem hiding this comment.
Re-reviewed the CI verdict/concurrency changes, affected-surface planner, AST caches, and icon reproducibility path at the current head. The main-push concurrency key and the narrowed DeepSeek/icon planner inputs preserve their intended authority boundaries, and the AST caches retain the existing traversal results. I found one P2 in the new PNG comparison, described inline.\n\nValidation: build:test; full typecheck; planner/workflow/icon/App Update focused tests (92/92, including App Update 8/8); renderer architecture checks (112/112); Windows inventory; full lint and format; ASF headers; git diff --check; hosted test/label; and a clean merge with current main b37eb96398ace9a9935dbfd87999b12d0af865b8. The PNG failure was reproduced with both Sharp and Electron nativeImage, and the same mutation was rejected by the exact base implementation.\n\n> Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
3f87033 to
71e8da4
Compare
Preserve a verdict for every main commit while avoiding asset, toolchain, and AST work whose inputs did not change. Generated-by: Codex Generated-by: Claude Code
71e8da4 to
ab912d9
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Reviewed ab912d9. The concurrency change preserves verdicts for main commits, and the gate pruning follows the actual inputs. I also rechecked the updated PNG validation: it rejects the metadata-changing case while accepting the generated format. The focused planner, workflow, icon, and boundary checks passed. I have no remaining findings.
hqhq1025
left a comment
There was a problem hiding this comment.
Technical GO on the current head; I found no remaining P0-P3 issues.
The previous PNG false-green is fixed. The checker now requires exactly the generator's IHDR/IDAT/IEND structure, validates every CRC and the complete zlib stream, and compares the decompressed scanlines. I reran the production --check path with a CRC-valid unknown critical chunk and it correctly failed as out of date; recompressing the same scanlines at a different zlib level still passed. The added regression covers the rejected critical-chunk case.
I also rechecked the main-push concurrency identity, affected-surface planner narrowing, renderer AST caches, and App Update boundary scan. Local validation passed build:test, full typecheck/lint/format, ASF headers, git diff --check, 97 focused planner/workflow/icon/App Update tests, renderer architecture 112/112, and the Windows inventory. Hosted test passed on this exact head, and the branch merges cleanly with current main 4cd71eaed26dbf296f1142db3296b27136d7d131.
I did not run the workflows on native Windows/macOS or rebuild the external DeepSeek Harness locally; those paths are covered by the hosted workflow selection rather than this Linux re-review.
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
Summary
Core CI reused one concurrency group for all main pushes, so a later push could evict an earlier pending commit before it received a verdict. Main pushes and manual runs now key by event and commit, while pull requests retain one superseding group per PR. Full-suite planning skips unchanged icon and DeepSeek Harness gates, and AST-heavy checks reuse parsed structure; icon drift compares decompressed scanlines so zlib output alone cannot report false drift.
Main pushes that land close together now run in parallel, temporarily using more runner capacity in exchange for a verdict on every commit.
Verification
The latest GitHub Actions run passed in 21 min 02 s. It spent 72 s on the two gates changed by this PR (43 s for the DeepSeek Harness toolchain and 29 s for icons); dependency-only full-suite changes now skip both. App Update boundary analysis falls 370.66 ms (13.4%), and renderer architecture traversal is about twice as fast locally.
test: passed in 21 min 02 sorigin/main: passednpm run format:check,npm run lint,npm run windows:inventory, andgit diff --check: passedAI use
Select exactly one:
Tool(s) and scope: Codex implemented the CI planning, icon, and AST optimizations. Claude Code reviewed and refined the concurrency key, gate boundaries, scanline comparison, and AST caches.
Checklist
Does this PR entail a change in behavior?