feat(gate): change risk 子命令 + PR 门禁——percentile≥90 拦截，risk-accepted 放行 - #102
Conversation
Adds `code-intel change risk <revspec>`: a deterministic, git-only defect-risk score (diff shape, test asymmetry, bug-magnet overlap, churn overlap; fixed documented weights) plus its percentile against the last N sampled non-merge commits. No index, no network, no LLM — dogfoods our own CLI instead of the third-party repowise MCP. Adds .github/workflows/pr-gate.yml: builds the CLI, scores every PR against origin/<base>..HEAD, posts/updates one sticky PR comment, and fails the job when risk_percentile >= 90 unless the PR carries the risk-accepted label. Mirrors ci.yml's toolchain/action choices (same actions/checkout and actions/upload-artifact pins, no new third-party actions, no dependency cache since ci.yml itself has none). Refs #95
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
🔒 Repowise is not analyzing this repository The PR bot is free on public repositories. This one is private, which needs a Pro plan. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change adds a Git-based ChangesChange risk scoring and PR enforcement
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Intel change risk
Top signals
revspec: |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
crates/code-intel-cli/src/change_risk.rs (1)
811-893: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider deduplicating the Some/None report branches.
The
Some(scored)andNonebranches build the same nested JSON shape by hand, once from real values and once from hardcoded zero defaults. The key sets currently match exactly, but nothing enforces that they stay in sync if a field is added later to one branch and forgotten in the other.Introduce a zero-valued default for
Scored(or an intermediate struct) and always fill the JSON from it, only overriding fields whenscoredisSome, so there is a single place that defines the signal shape.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/code-intel-cli/src/change_risk.rs` around lines 811 - 893, Refactor build_report so the Some(scored) and None branches no longer duplicate the signals JSON shape. Introduce a zero-valued Scored or intermediate representation, select real values when scored is present and defaults otherwise, then construct the nested signals object once while preserving all existing fields and values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pr-gate.yml:
- Around line 108-120: Add the pull-request fork guard to the “Post or update
sticky comment” step using the condition
github.event.pull_request.head.repo.fork == false. Keep the existing comment
lookup and POST/PATCH logic unchanged, ensuring fork PRs skip only this step
while the gate evaluation still runs.
- Around line 114-120: Update the workflow job containing the sticky-comment
lookup and PATCH/POST logic to define a shared concurrency group keyed by pull
request number, with cancel-in-progress enabled. Ensure all supported trigger
types for the same PR use that group so only the latest run can execute the
EXISTING_ID comment-update path.
- Around line 122-137: Update the Evaluate gate step to inspect the report
status in risk.json before reading risk_percentile; when the status is warning,
write blocked=false to GITHUB_OUTPUT and exit the step successfully. Keep the
existing percentile and label-based blocking logic unchanged for non-warning
reports.
- Around line 39-42: Update the Checkout action configuration to explicitly set
persist-credentials to false alongside fetch-depth, while leaving GH_TOKEN usage
unchanged wherever authentication is required.
In `@crates/code-intel-cli/src/change_risk.rs`:
- Around line 958-1157: Extend the tests around execute with focused coverage
for baseline contamination by creating several commits, scoring a multi-commit
range with a sample large enough to include the target commits, and asserting
those commits are excluded from the risk_percentile baseline. Add a separate
triple-dot range test with a diverging branch and verify commits_in_range
matches exactly the commits represented by the diff. Run the focused cargo tests
and relevant integration-contract checks.
- Around line 301-345: Filter the commits returned by sample_history in execute
before scoring them, skipping any SHA present in the existing exclude BTreeSet.
Keep the sample history traversal and scoring unchanged for commits outside the
scored range, so baseline_scores contains no commits from the target range.
- Around line 438-473: Update commits_in_range to normalize revspecs containing
“...” to the equivalent “..” form before passing them to git rev-list,
preserving other range inputs unchanged. Use the normalized range in the
existing hardened_git command while retaining the current best-effort
empty-result behavior.
---
Nitpick comments:
In `@crates/code-intel-cli/src/change_risk.rs`:
- Around line 811-893: Refactor build_report so the Some(scored) and None
branches no longer duplicate the signals JSON shape. Introduce a zero-valued
Scored or intermediate representation, select real values when scored is present
and defaults otherwise, then construct the nested signals object once while
preserving all existing fields and values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 87e22a84-a8ce-441b-8e8a-a4ab67b3bdf7
📒 Files selected for processing (3)
.github/workflows/pr-gate.ymlcrates/code-intel-cli/src/change_risk.rscrates/code-intel-cli/src/main.rs
Refs #95 sentrux_gate's god-file rule (sentrux_gate.rs:738, `loc > 800 || (functions > 25 && loc > 400)`) flagged the new change_risk.rs at 1157 lines; split into change_risk/{mod,git,signals,scoring,render,tests}.rs, each well under threshold, behavior/JSON/CLI/tests unchanged.
Refs #95 - change_risk/mod.rs: execute()'s baseline loop now skips sampled commits already in the target's own `exclude` set (baseline self-contamination inflated risk_percentile). - change_risk/git.rs: commits_in_range() normalizes a...b to a..b before rev-list, matching git diff's merge-base(a,b)..b semantics instead of rev-list's symmetric difference. - change_risk/tests.rs: 2 new tests covering both fixes (11 total, up from 9). - change_risk/mod.rs + render.rs: Scored and its signal structs derive Default; build_report's Some/None match collapses to one code path (nitpick 4835249459). - pr-gate.yml: Evaluate gate checks has("warning") before the numeric comparison; checkout gets persist-credentials: false; sticky-comment step skips on fork PRs; workflow-level concurrency group added.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
crates/code-intel-cli/src/change_risk/mod.rs (1)
50-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep one canonical
hardened_gitmodule declaration.
crates/code-intel-cli/src/main.rsdeclaresmod hardened_git;, so this#[path = "../hardened_git.rs"] mod hardened_git;incrates/code-intel-cli/src/change_risk/mod.rscreates a secondcrate::change_risk::hardened_gitmodule from the same file. Reusecrate::hardened_gitfrom the sibling modules instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/code-intel-cli/src/change_risk/mod.rs` around lines 50 - 51, Remove the local `#[path] mod hardened_git` declaration from `change_risk` and update its references to use the canonical `crate::hardened_git` module declared in `main.rs`, matching the sibling modules’ usage.crates/code-intel-cli/src/change_risk/tests.rs (1)
10-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGuard against leaked temp directories on test failure.
Every integration test in this file calls
std::fs::remove_dir_all(&repo).ok()only at the end of the test body, after all assertions. If anyassert!/assert_eq!call panics (ininit_repo,commit, or the test itself), the temp repository directory understd::env::temp_dir()is never removed. This affectsempty_diff_reports_a_warning_without_erroring,scoring_is_deterministic_for_the_same_revspec,root_commit_diffs_against_the_empty_tree,a_commit_inside_the_scored_range_does_not_count_toward_its_own_files_history,baseline_sampling_excludes_commits_inside_the_scored_range, andcommits_in_range_normalizes_triple_dot_to_the_diffed_commit_set.Use a Drop-based guard (or the
tempfilecrate'sTempDir) so cleanup runs even on panic.♻️ Suggested direction
-fn init_repo(name: &str) -> PathBuf { +struct TempRepo(PathBuf); + +impl std::ops::Deref for TempRepo { + type Target = Path; + fn deref(&self) -> &Path { &self.0 } +} + +impl Drop for TempRepo { + fn drop(&mut self) { + std::fs::remove_dir_all(&self.0).ok(); + } +} + +fn init_repo(name: &str) -> TempRepo { ... - repo + TempRepo(repo) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/code-intel-cli/src/change_risk/tests.rs` around lines 10 - 34, Update init_repo and the listed integration tests to manage temporary repositories with panic-safe cleanup, using a Drop-based guard or tempfile::TempDir instead of relying on end-of-test remove_dir_all calls. Ensure cleanup still occurs when assertions panic in repository setup, commits, or test bodies, while preserving each test’s existing repository path usage.crates/code-intel-cli/src/change_risk/signals.rs (1)
127-132: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTighten the fix-commit heuristic to avoid substring false positives.
looks_like_fix_subjectuses "subject.to_lowercase().contains("fix") || subject.contains("修复") || subject.contains("修正")". This matches "fix" as a substring, so commit subjects like "add prefix support", "update fixture data", or "refactor suffix handling" count as bug fixes.bug_magnet_signalandbuild_scored_filesboth derivebugFixCommits180ddirectly from this function, and that count feeds the weighted risk score that gates PRs. Unrelated commits inflate the bug-magnet subscore and, in turn, the overall risk score and percentile.Match "fix" and its common conjugations as whole words instead of as a substring anywhere in the subject.
♻️ Proposed fix
pub(super) fn looks_like_fix_subject(subject: &str) -> bool { - subject.to_lowercase().contains("fix") || subject.contains("修复") || subject.contains("修正") + let lower = subject.to_lowercase(); + lower + .split(|c: char| !c.is_alphanumeric()) + .any(|word| matches!(word, "fix" | "fixed" | "fixes" | "fixing" | "fixup")) + || subject.contains("修复") + || subject.contains("修正") }As per coding guidelines, "Rust changes require focused cargo test coverage plus the relevant integration-contract checks," so add unit tests covering both the true positives (e.g., "fix: null check", "fixed race") and the previously mis-scored false positives (e.g., "add prefix support") when applying this change.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/code-intel-cli/src/change_risk/signals.rs` around lines 127 - 132, Update looks_like_fix_subject to match “fix” and common conjugations such as “fixed” only as whole words, while preserving detection of the Chinese markers 修复 and 修正. Add focused unit tests covering true positives like “fix: null check” and “fixed race”, plus false positives such as “add prefix support”, and run the relevant Rust integration-contract checks.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/code-intel-cli/src/change_risk/git.rs`:
- Around line 90-126: Disable Git path quoting in both run_git_diff_numstat and
file_commit_history by adding the core.quotePath=false configuration to their
subprocess commands before parsing --numstat/--name-only output, ensuring
non-ASCII paths remain usable by normalize_path and subsequent lookups.
---
Nitpick comments:
In `@crates/code-intel-cli/src/change_risk/mod.rs`:
- Around line 50-51: Remove the local `#[path] mod hardened_git` declaration
from `change_risk` and update its references to use the canonical
`crate::hardened_git` module declared in `main.rs`, matching the sibling
modules’ usage.
In `@crates/code-intel-cli/src/change_risk/signals.rs`:
- Around line 127-132: Update looks_like_fix_subject to match “fix” and common
conjugations such as “fixed” only as whole words, while preserving detection of
the Chinese markers 修复 and 修正. Add focused unit tests covering true positives
like “fix: null check” and “fixed race”, plus false positives such as “add
prefix support”, and run the relevant Rust integration-contract checks.
In `@crates/code-intel-cli/src/change_risk/tests.rs`:
- Around line 10-34: Update init_repo and the listed integration tests to manage
temporary repositories with panic-safe cleanup, using a Drop-based guard or
tempfile::TempDir instead of relying on end-of-test remove_dir_all calls. Ensure
cleanup still occurs when assertions panic in repository setup, commits, or test
bodies, while preserving each test’s existing repository path usage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 427085cb-e73c-4907-a3bd-56c397b65eb8
📒 Files selected for processing (7)
.github/workflows/pr-gate.ymlcrates/code-intel-cli/src/change_risk/git.rscrates/code-intel-cli/src/change_risk/mod.rscrates/code-intel-cli/src/change_risk/render.rscrates/code-intel-cli/src/change_risk/scoring.rscrates/code-intel-cli/src/change_risk/signals.rscrates/code-intel-cli/src/change_risk/tests.rs
1 participant
干什么
给每张 PR 装上机器风险门禁（#95 最小可行版），自家 CLI 吃自家狗粮，不依赖第三方 repowise：
code-intel change risk <revspec>（新子命令，crates/code-intel-cli/src/change_risk.rs）：确定性、纯 git、免索引、免网络、免 LLM 的 0–100 缺陷风险分。四信号定权重（30/25/25/20）：diff 形状、测试不对称、bug 磁铁重叠（180d fix 提交）、churn 重叠（90d）。百分位对照最近 50 个非 merge 提交（各自锚定提交时刻，永久可复现）。空 diff / 坏 revspec 退出码 0 +"warning":"empty_diff"，永不 panic。输出 machine-first JSON（schema: code-intel-change-risk.v1），--format text从同一 JSON 派生。.github/workflows/pr-gate.yml：PR 上构建 CLI → 打分 → 单条 sticky 评论（marker 更新，不刷屏）→risk_percentile >= 90且无risk-accepted标签则红检查，阻断 auto-merge。labeled/unlabeled事件也触发，补挂标签即可解锁。actions 均按 SHA 钉死，与 ci.yml 同源。狗粮时刻
本 PR 用自己的门禁给自己打分：score 82，90 分位，high——会被自己拦下。属实（1310 行新增、bug 磁铁区注册路由）。按设计走
risk-accepted标签放行，这本身就是门禁流程的首次真实演练。测试
cargo test -p code-intel --locked：382/382 主套件，聚合 2832/2832 全绿cargo fmt --check干净；新增代码 clippy 干净（crate 级 110+ 存量告警与本变更无关）已知局限（如实记录）
#[cfg(test)] mod tests——本 PR 自己就是例子（带 9 测试仍被记"未动测试"）。后续可做 hunk 级检测。GITHUB_TOKEN只读，评论步骤会失败（本仓单人分支流，暂不影响）。Refs #95