feat(frontend): 覆盖率 include 合同 — 未导入生产代码进分母 + 绝对 floors + uncovered ratchet (#1535) - #1552
Conversation
|
Warning Review limit reached
Next review available in: 41 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughFrontend Vitest projects now use shared coverage configuration. CI validates production-source inclusion, coverage thresholds, and uncovered-file baselines across four packages. A negative self-test verifies that unreferenced production files appear as uncovered. ChangesFrontend coverage configuration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
scripts/verify/verify-coverage-baseline.ps1 (1)
176-186: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared uncovered-file counting rule into
scripts/lib/. Both scripts independently implement the same rule — a file withlines.total > 0andlines.pct == 0.0counts as an uncovered production module — for tallyingproduction_files/uncovered_filesfrom a coverage-summary.json.
scripts/verify/verify-coverage-baseline.ps1#L176-L186: replace this inline loop with a call to a shared helper function.scripts/verify/tests/coverage-include.Tests.ps1#L51-L70: replaceGet-CoverageStats's loop body with a call to the same shared helper, keeping this file's probe-specific matching on top.As per coding guidelines, "脚本必须放在 scripts/verify/、scripts/dev/、scripts/release/、scripts/smoke/ 或 scripts/lib/", which designates
scripts/lib/as the location for shared script logic such as this.🤖 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 `@scripts/verify/verify-coverage-baseline.ps1` around lines 176 - 186, Extract the shared uncovered-production-file rule into a helper under scripts/lib/, counting files whose lines.total is greater than zero and lines.pct equals 0.0. In scripts/verify/verify-coverage-baseline.ps1 lines 176-186, replace the inline counting loop with the helper call; in scripts/verify/tests/coverage-include.Tests.ps1 lines 51-70, update Get-CoverageStats to use the same helper while preserving its probe-specific matching.Source: Coding guidelines
.github/workflows/checks.yml (1)
923-931: 🚀 Performance & Scalability | 🔵 TrivialTwo full
agenthub-mobile-rncoverage runs execute back-to-back in the same job.
Verify coverage baseline(Line 925) andSelf-test coverage include contract (negative)(Line 931) both run the full 339-testagenthub-mobile-rnsuite with coverage instrumentation, unconditionally, in the samevalidatejob. This is an accepted tradeoff per the adjacent comment ("Cheap — mobile only"), but confirm the added wall-clock cost on every PR (including PRs that touch neitherapp/mobile-rnnorapp/test-config/coverage.ts) is acceptable, sincevalidatedoes not appear to be path-filtered.🤖 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 @.github/workflows/checks.yml around lines 923 - 931, Update the validate workflow around “Verify coverage baseline” and “Self-test coverage include contract (negative)” so the full mobile coverage suite is not executed twice unconditionally on every pull request. Reuse the existing coverage run or gate the negative self-test to the relevant mobile/coverage configuration changes while preserving the required baseline verification and negative include-contract validation.
🤖 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 `@scripts/verify/coverage-baseline.json`:
- Around line 4-5: Update the masterSha field in coverage-baseline.json to the
correct 40-character commit SHA corresponding to masterShaShort 1a9f3774,
preserving the existing baseline provenance fields.
In `@scripts/verify/verify-coverage-baseline.ps1`:
- Around line 187-195: Update the uncovered-files validation around
$pkg.uncoveredFiles to fail the gate when the baseline field is missing, instead
of skipping the comparison. Add a failure message identifying $pkgFilter and the
missing uncoveredFiles baseline, while retaining the existing growth check when
the field is present.
---
Nitpick comments:
In @.github/workflows/checks.yml:
- Around line 923-931: Update the validate workflow around “Verify coverage
baseline” and “Self-test coverage include contract (negative)” so the full
mobile coverage suite is not executed twice unconditionally on every pull
request. Reuse the existing coverage run or gate the negative self-test to the
relevant mobile/coverage configuration changes while preserving the required
baseline verification and negative include-contract validation.
In `@scripts/verify/verify-coverage-baseline.ps1`:
- Around line 176-186: Extract the shared uncovered-production-file rule into a
helper under scripts/lib/, counting files whose lines.total is greater than zero
and lines.pct equals 0.0. In scripts/verify/verify-coverage-baseline.ps1 lines
176-186, replace the inline counting loop with the helper call; in
scripts/verify/tests/coverage-include.Tests.ps1 lines 51-70, update
Get-CoverageStats to use the same helper while preserving its probe-specific
matching.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 686d0369-2988-40ab-a21a-c20ba735cd94
📒 Files selected for processing (11)
.github/workflows/checks.ymlapp/desktop/vitest.desktop-ci.config.tsapp/mobile-rn/vitest.config.tsapp/shared/vitest.config.tsapp/test-config/coverage.tsapp/web/vitest.config.tsdocs/progress/MASTER.mdscripts/verify/coverage-baseline.jsonscripts/verify/tests/coverage-include.Tests.ps1scripts/verify/verify-ci-gates.ps1scripts/verify/verify-coverage-baseline.ps1
| if ($productionFiles -eq 0) { | ||
| $failures.Add("[$pkgFilter] coverage include matched 0 production files — include glob broken (fail-closed)") | ||
| } | ||
| if ($null -ne $pkg.uncoveredFiles) { | ||
| $baseUncovered = [int]$pkg.uncoveredFiles | ||
| if ($uncoveredFiles -gt $baseUncovered) { | ||
| $failures.Add("[$pkgFilter] uncovered (0%) production modules grew: $uncoveredFiles > baseline $baseUncovered (new untested code or deleted test)") | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Missing uncoveredFiles baseline field silently disables the ratchet.
The production_files == 0 check fails closed (Line 187-189). The uncoveredFiles ratchet does not: Line 190 only runs the comparison if ($null -ne $pkg.uncoveredFiles). If a package's baseline entry ever loses this field, the gate silently stops enforcing the uncovered-file ratchet for that package instead of failing.
This contradicts the script's own stated policy (Lines 12-16: "production_files == 0 ... is a failure") and the sibling self-test coverage-include.Tests.ps1, which throws when uncoveredFiles is missing (Line 47 of that file). Fail the gate when the field is absent, to keep both scripts fail-closed for the same contract.
🛡️ Proposed fix to fail closed on a missing baseline field
- if ($null -ne $pkg.uncoveredFiles) {
- $baseUncovered = [int]$pkg.uncoveredFiles
- if ($uncoveredFiles -gt $baseUncovered) {
- $failures.Add("[$pkgFilter] uncovered (0%) production modules grew: $uncoveredFiles > baseline $baseUncovered (new untested code or deleted test)")
- }
- }
+ if ($null -eq $pkg.uncoveredFiles) {
+ $failures.Add("[$pkgFilter] baseline entry is missing 'uncoveredFiles' — ratchet cannot be enforced (fail-closed)")
+ } else {
+ $baseUncovered = [int]$pkg.uncoveredFiles
+ if ($uncoveredFiles -gt $baseUncovered) {
+ $failures.Add("[$pkgFilter] uncovered (0%) production modules grew: $uncoveredFiles > baseline $baseUncovered (new untested code or deleted test)")
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ($productionFiles -eq 0) { | |
| $failures.Add("[$pkgFilter] coverage include matched 0 production files — include glob broken (fail-closed)") | |
| } | |
| if ($null -ne $pkg.uncoveredFiles) { | |
| $baseUncovered = [int]$pkg.uncoveredFiles | |
| if ($uncoveredFiles -gt $baseUncovered) { | |
| $failures.Add("[$pkgFilter] uncovered (0%) production modules grew: $uncoveredFiles > baseline $baseUncovered (new untested code or deleted test)") | |
| } | |
| } | |
| if ($productionFiles -eq 0) { | |
| $failures.Add("[$pkgFilter] coverage include matched 0 production files — include glob broken (fail-closed)") | |
| } | |
| if ($null -eq $pkg.uncoveredFiles) { | |
| $failures.Add("[$pkgFilter] baseline entry is missing 'uncoveredFiles' — ratchet cannot be enforced (fail-closed)") | |
| } else { | |
| $baseUncovered = [int]$pkg.uncoveredFiles | |
| if ($uncoveredFiles -gt $baseUncovered) { | |
| $failures.Add("[$pkgFilter] uncovered (0%) production modules grew: $uncoveredFiles > baseline $baseUncovered (new untested code or deleted test)") | |
| } | |
| } |
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[warning] Missing BOM encoding for non-ASCII encoded file 'verify-coverage-baseline.ps1'
(PSUseBOMForUnicodeEncodedFile)
🤖 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 `@scripts/verify/verify-coverage-baseline.ps1` around lines 187 - 195, Update
the uncovered-files validation around $pkg.uncoveredFiles to fail the gate when
the baseline field is missing, instead of skipping the comparison. Add a failure
message identifying $pkgFilter and the missing uncoveredFiles baseline, while
retaining the existing growth check when the field is present.
New app/test-config/coverage.ts factory: include all production src/**/*.ts(x) in the coverage denominator for all four packages. Before this, web/desktop/mobile had no coverage.include, so v8 only counted files loaded by tests — unimported production modules vanished from the denominator and coverage looked great while modules were untested. - factory defaults exclude only test categories (spec/__tests__/__e2e__); narrow per-package exclusions need explicit reasons (web/desktop main.tsx pure entry, shared keeps its 5 legacy file exclusions) - shared config rewritten via factory; numbers byte-identical to previous baseline (76.43/74.82/74.01/70.48) — behavior-equivalent refactor - absolute floors per package: shared 60/60/60/60 (kept), web 63/62/53/57, desktop 48/46/40/39, mobile 35/34/26/21 (floor(measured)-1, measured 2026-08-03 with full denominator) - uncovered modules now visible: web 5, desktop 46, mobile 25 0%-line production files (was: invisible)
verify-coverage-baseline.ps1 now reports production_files / uncovered_files per package (Issue format) and fails when: - production_files == 0 (include glob broken — fail-closed) - uncovered_files grows past baseline.uncoveredFiles (new untested code or deleted test both trip the 0% ratchet) coverage-include.Tests.ps1 (negative self-test, runs in validate job): creates an imported-by-nobody probe module, proves coverage-summary counts it as 0% (not silently excluded), proves the ratchet would trip (uncovered 26 > baseline 25), then cleans up. Baseline remeasured 2026-08-03 on master 1a9f377: shared 76.43 (unchanged), web 64.69/63.66/54.11/58.75, desktop 49.29/47.44/41.79/40.96, mobile 36.19/35.81/27.82/22.72 + uncoveredFiles + improvement targets (all dims toward 60, recorded in baseline note).
checks.yml: - remove dead 'Shared coverage (60% thresholds)' step from frontend-desktop (fully redundant with the validate job's coverage baseline gate) - mobile job: drop 'Install coverage provider' (workspace-root hoisted provider) and CLI threshold overrides (floors now live in config) - validate job: add negative self-test step verify-ci-gates.ps1 asserts the new gate + self-test steps exist.
Get-StepBlock anchors on '- name: <exact>' — the step is named 'Self-test coverage include contract (negative)'.
de97737 to
c7dc372
Compare
…atchet (#1535) (#1552) * feat(frontend): coverage include contract via shared factory (#1535) New app/test-config/coverage.ts factory: include all production src/**/*.ts(x) in the coverage denominator for all four packages. Before this, web/desktop/mobile had no coverage.include, so v8 only counted files loaded by tests — unimported production modules vanished from the denominator and coverage looked great while modules were untested. - factory defaults exclude only test categories (spec/__tests__/__e2e__); narrow per-package exclusions need explicit reasons (web/desktop main.tsx pure entry, shared keeps its 5 legacy file exclusions) - shared config rewritten via factory; numbers byte-identical to previous baseline (76.43/74.82/74.01/70.48) — behavior-equivalent refactor - absolute floors per package: shared 60/60/60/60 (kept), web 63/62/53/57, desktop 48/46/40/39, mobile 35/34/26/21 (floor(measured)-1, measured 2026-08-03 with full denominator) - uncovered modules now visible: web 5, desktop 46, mobile 25 0%-line production files (was: invisible) * feat(verify): uncovered-files ratchet + negative self-test (#1535) verify-coverage-baseline.ps1 now reports production_files / uncovered_files per package (Issue format) and fails when: - production_files == 0 (include glob broken — fail-closed) - uncovered_files grows past baseline.uncoveredFiles (new untested code or deleted test both trip the 0% ratchet) coverage-include.Tests.ps1 (negative self-test, runs in validate job): creates an imported-by-nobody probe module, proves coverage-summary counts it as 0% (not silently excluded), proves the ratchet would trip (uncovered 26 > baseline 25), then cleans up. Baseline remeasured 2026-08-03 on master 1135d58: shared 76.43 (unchanged), web 64.69/63.66/54.11/58.75, desktop 49.29/47.44/41.79/40.96, mobile 36.19/35.81/27.82/22.72 + uncoveredFiles + improvement targets (all dims toward 60, recorded in baseline note). * ci: coverage lane sync + MASTER record (#1535) checks.yml: - remove dead 'Shared coverage (60% thresholds)' step from frontend-desktop (fully redundant with the validate job's coverage baseline gate) - mobile job: drop 'Install coverage provider' (workspace-root hoisted provider) and CLI threshold overrides (floors now live in config) - validate job: add negative self-test step verify-ci-gates.ps1 asserts the new gate + self-test steps exist. * fix(verify): exact step name for coverage self-test assertion (#1535) Get-StepBlock anchors on '- name: <exact>' — the step is named 'Self-test coverage include contract (negative)'. --------- Co-authored-by: Codex <codex@vectorcontrol.tech>
目标（#1535）
Web/Desktop/Mobile/Shared 均覆盖未被测试 import 的生产模块，叠加绝对阈值 + master baseline 双重约束；删除/遗忘模块后覆盖率数字不再好看。
改动
共享 coverage factory（
app/test-config/coverage.ts）include: ['src/**/*.ts', 'src/**/*.tsx']（生产源码全量进分母）*.test.*/__tests__/__e2e__（Playwright spec）main.tsx（纯入口，shared index.ts 先例）；shared 保留 5 个既有文件排除（factory 重写行为等价：数字与旧 baseline 完全一致）绝对 floors（2026-08-03 实测，floor(实测)-1 留 v8 抖动缓冲）
低于 60 的维度为提升目标（记录在 baseline note），非永久豁免。
门禁（verify-coverage-baseline.ps1）
package coverage: production_files=N uncovered_files=M负向自测（validate job）
coverage-include.Tests.ps1：注入未导入 probe 模块 → 必须计 0% 且 uncovered 26 > baseline 25（ratchet 会响）→ 清理。揭示的真相（分母扩大）
web 66.96→64.69、desktop 71.82→49.29、mobile 80.5→36.19 lines —— 之前未导入文件不进分母，数字虚高。
验证
禁止清单遵守
Summary by CodeRabbit
Bug Fixes
Documentation