Skip to content

chore(ci): add high-precision holistic review - #396

Open
CloudeaSoft wants to merge 1 commit into
masterfrom
chore/high-precision-ai-review
Open

chore(ci): add high-precision holistic review#396
CloudeaSoft wants to merge 1 commit into
masterfrom
chore/high-precision-ai-review

Conversation

@CloudeaSoft

Copy link
Copy Markdown
Member

No description provided.

@CloudeaSoft
CloudeaSoft requested a review from Omnielement July 27, 2026 09:01

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Holistic Review

Motivation: The PR adds a second, on-demand high-precision Holistic Review workflow using Kimi K3 (with reasoning_effort=high) as a complement to the existing automatic DeepSeek-based review. This is a well-motivated CI improvement — maintainers can request deeper analysis for complex PRs without paying the cost on every push. The architecture mirrors the existing workflow, shares the same review skill, and keeps independent incremental history, which is the right approach.

Approach: The implementation is sound — a new gh-aw source (.md) and its compiled lock (.lock.yml), plus documentation updates to README-ai-review.md. Key security properties are correctly enforced: only admin/maintainer/write roles can trigger via /holistic-review comment; fork PRs are rejected; safeoutputs is locked to COMMENT event only; HEAD check before publishing prevents stale output; and the trust overlay strips PR-controlled config paths. The lock.yml was compiled with gh-aw v0.83.1, matching the stated toolchain version.

Summary: ✅ LGTM. This is a clean, well-documented CI infrastructure addition with no game-code impact. All security boundaries are correctly configured, and the design follows the established patterns of the existing Holistic Review workflow. No C# code changes, no module system impact, no localization concerns.


Detailed Findings

✅ Build & Module System — No game-code impact

This PR modifies only .github/workflows/ files. No Sources/ C# code, no .csproj, no Directory.Build.props, no localization files, no resource assets are touched. All Everglow-specific checks for Modules, tML auto-loading, template inheritance, LocalizationCategory, Main.dedServ guards, ModAsset.* paths, and networking are N/A.

✅ Safe Outputs Configuration — Correctly locked down

The compiled holistic-review-high-precision.lock.yml correctly configures submit_pull_request_review with allowed_events: ["COMMENT"] only — no REQUEST_CHANGES or APPROVE is possible. create_pull_request_review_comment is capped at 10, side RIGHT. The noop tool is available for correct completion signaling.

✅ Authorization — Role-based access enforced

The lock.yml includes a pre-activation step that checks github.event.comment.author_association against OWNER, MEMBER, COLLABORATOR (mapped from roles: [admin, maintainer, write]). The workflow also requires the comment body to start with /holistic-review (exact match as first word) and the issue to be a PR comment. These are the right guards.

✅ HEAD staleness check — Prevents stale review output

The safe_outputs job includes a pre-step that re-queries the PR HEAD via gh api and compares it to the fixed EXPECTED_HEAD_SHA. If the PR HEAD has changed during analysis, the entire safe-output run is rejected (exit 1). This is a strong guard against publishing outdated line numbers and conclusions.

✅ Independent review history — No cross-contamination

The pre-agent step queries only reviews whose body starts with ## High-Precision Holistic Review (not the automatic ## Holistic Review) and includes the high-precision metadata marker. Automatic DeepSeek reviews are never used as the incremental baseline, and vice versa. The metadata persistence via <!-- everglow-high-precision-review: {...} --> HTML comment embeds head_sha, base_sha, merge_base_sha, and patch_id for accurate incremental detection.

💡 Documentation — Compiler version stated but not enforced in CI

The README-ai-review.md and the high-precision .md source both state that lock files must be compiled with gh-aw v0.83.1. However, the lock.yml's # gh-aw-manifest hardcodes compiler_version: v0.83.1 in its metadata header. If a contributor compiles with a newer/older version and commits the regenerated lock, the metadata mismatch would be visible in git diff but there is no CI check that rejects mismatched compiler versions. This is an observation, not a blocking concern — the existing agentics-maintenance.yml auto-regenerates lock files on the default branch after merge, which is the intended reconciliation path.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement human review.

Generated by Everglow Holistic Review for #396 · ⊞ 20.9K

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant