ci: add codecov config and diff-cover enforcement (80% patch coverage gate)#23
ci: add codecov config and diff-cover enforcement (80% patch coverage gate)#23andreiships merged 9 commits intodevfrom
Conversation
- codecov.yml: precision/range/ignore config, patch coverage informational (enforcement handled by diff-cover job, not Codecov) - test.yml: upload coverage artifact from unit job for downstream consumption - test.yml: new diff-cover-enforcement job using shared composite action andreiships/pistachiorama/.github/actions/diff-cover-check@main - test.yml: required gate now includes diff-cover-enforcement (skipped on push-to-dev, required on PRs)
|
Hey! Your PR title Please update it to start with one of:
Where See CONTRIBUTING.md for details. |
|
The following comment was made by an LLM, it may be inaccurate: |
andreiships-bot
left a comment
There was a problem hiding this comment.
Single-Pass Code Review
PR: ci: add codecov config and diff-cover enforcement (80% patch coverage gate)
Overall, this is a well-structured CI enhancement. The separation of concerns between Codecov (reporting) and diff-cover (enforcement) is a good design choice. A few issues and suggestions below.
Codex ReviewSummaryPR intent is solid and scoped appropriately, but the new diff-cover gate has a branch-resolution flaw that can fail PRs even when tests pass. Merge should wait for that workflow reliability fix. Findings[FINDING-1] issue: P1 | .github/workflows/test.yml:122 | Code Quality
Architecture
PR MetadataSuggested PR Title: Recommendation[ ] Approve | [ ] Approve with changes | [x] Request changes Escalate to Gemini?[ ] Yes - [reason] | [x] No |
Gemini Deep ReviewSummaryThe PR successfully integrates Codecov for visualization and introduces a mandatory 80% patch coverage gate for the Findings[gemini-1] issue: P1 | codecov.yml:15 | Coverage threshold mismatch. [gemini-2] suggestion: P2 | .github/workflows/test.yml:118 | Pin external action to a commit SHA. [gemini-3] nit: P2 | .github/workflows/test.yml:121 | Coverage is scope-limited to [gemini-4] nit: P2 | codecov.yml:13 | PR MetadataSuggested PR Title: ci: enforce 80% patch coverage gate for packages/opencode Questions
Recommendation[ ] Approve | [x] Approve with changes | [ ] Request changes |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Resolution SummaryResolving findings from Codex Review, Gemini Review:
|
Question AnswersQ: Is the 80% threshold intended to apply strictly to A: Intentionally scoped to |
Codex ReviewSummaryPrevious blocker is not addressed in this iteration: the diff-cover job still compares against Findings[FINDING-1] issue: P1 | .github/workflows/test.yml:122 | Code Quality
Architecture
PR MetadataSuggested PR Title: Recommendation[ ] Approve | [ ] Approve with changes | [x] Request changes Escalate to Gemini?[ ] Yes - [reason] | [x] No |
Gemini Deep ReviewSummaryThe findings from the previous review remain unaddressed in the current workspace files. While the Findings[gemini-1] blocking: P0 | codecov.yml:15 | Coverage threshold mismatch (STILL UNRESOLVED). [gemini-2] issue: P1 | .github/workflows/test.yml:118 | External action not pinned to SHA (STILL UNRESOLVED). [gemini-4] nit: P2 | codecov.yml:13 | Ratchet threshold allows coverage drop. PR MetadataSuggested PR Title: ci: add codecov config and 80% patch coverage enforcement Questions
Recommendation[ ] Approve | [ ] Approve with changes | [x] Request changes |
Dual-Review Summary (Round 2)
Convergence: ❌ Not achieved (3 blocking findings) Rounds: 2 Findings[Codex #1] P1 -
[Gemini #1] P0 -
[Gemini #2] P1 -
[Gemini #4] P2 -
|
github_actions_repository_access_level only applies to organization repos, not personal accounts. Cross-repo private action access doesn't work for andreiships/* — resolve by copying the composite action locally. Mirrors andreiships/pistachiorama@main .github/actions/diff-cover-check/. Updates to the action can be synced manually.
Replaces local .github/actions/diff-cover-check/ with cross-repo reference to andreiships/shared-ai-standards/.github/actions/diff-cover-check@main.
Claude Single-ReviewSummaryPR is well-structured and the intent is clear: establish an 80% patch coverage gate via Findings[FINDING-1] blocking: P0 | codecov.yml:15 | Coverage threshold mismatch between patch:
default:
target: 80%
informational: true[FINDING-2] issue: P1 | .github/workflows/test.yml:121 | - name: Fetch base branch
run: git fetch origin ${{ github.base_ref }}:${{ github.base_ref }}Then use [FINDING-3] issue: P1 | .github/workflows/test.yml:121 | External action pinned to mutable uses: andreiships/shared-ai-standards/.github/actions/diff-cover-check@<full-sha>[FINDING-4] nit: P2 | .github/workflows/test.yml:108 | if: github.event_name == 'pull_request' && needs.unit.result == 'success'[FINDING-5] nit: P2 | codecov.yml:14 | Comment Code Quality
Architecture
Recommendation[ ] Approve | [ ] Approve with changes | [x] Request changes Blocking: FINDING-1 (threshold mismatch) and FINDING-2 (base branch fetch) must be fixed before merge. |
andreiships-bot
left a comment
There was a problem hiding this comment.
Claude Review
See inline comments for details.
Codex ReviewSummaryThis PR cleanly adds an 80% patch-coverage enforcement step for PRs, preserves push-to- FindingsNone. Code Quality
Architecture
PR MetadataSuggested PR Title: No change needed Recommendation[x] Approve | [ ] Approve with changes | [ ] Request changes Escalate to Gemini?[ ] Yes - [reason] | [x] No |
Gemini Deep ReviewSummaryThe PR successfully introduces Codecov integration and a custom patch coverage enforcement job. However, the current coverage generation is too narrow (limited to Findings[gemini-1] blocking: P0 | .github/workflows/test.yml:62 | Incomplete coverage generation for monorepo-wide enforcement. [gemini-2] issue: P1 | codecov.yml:15 | Inconsistent patch coverage thresholds. [gemini-3] nit: P2 | codecov.yml:11 | Inaccurate ratchet threshold. PR MetadataSuggested PR Title: ci: add codecov and diff-cover enforcement with monorepo support Questions
Recommendation[ ] Approve | [ ] Approve with changes | [x] Request changes |
Question AnswersQ: Is it intentional to only enforce coverage for A: Yes, intentional per the PR description: The fix is to add |
…tch issues - test.yml: replace shared action with inline diff-cover invocation to enable --src-paths scoping to packages/opencode/src (fixes false failures for PRs that touch packages/app or other packages without coverage data) - test.yml: add explicit fetch step so diff-cover resolves base branch locally - test.yml: simplify if-condition (remove redundant always() && !cancelled()) - codecov.yml: align patch target to 80% (was 70%, mismatched CI gate) - codecov.yml: fix ratchet comment to accurately describe 1% as tolerance band
Resolution SummaryResolving findings from Gemini Review, Codex Review:
|
Codex ReviewSummaryThe CI coverage gate is close, but one workflow interaction can fail PRs with an artifact plumbing error instead of a clear coverage-enforcement failure. Findings[FINDING-1] issue: P1 | test.yml:36, test.yml:49, test.yml:117 | Coverage generation is [FINDING-2] nit: P2 | test.yml:160 | Code Quality
Architecture
PR MetadataSuggested PR Title: Recommendation[ ] Approve | [ ] Approve with changes | [x] Request changes Escalate to Gemini?[ ] Yes - [reason] | [x] No |
Gemini Deep ReviewSummaryThe PR successfully introduces a patch coverage gate using Findings[gemini-1] blocking: P0 | .github/workflows/test.yml:133 | Path mismatch for run: bun --cwd packages/opencode test --coverage --coverage-reporter=lcov --timeout 30000
...
run: |
diff-cover coverage/lcov.info \
--compare-branch=${{ github.base_ref }} \
--src-paths packages/opencode/src \Fix: Adjust paths in [gemini-2] issue: P1 | .github/workflows/test.yml:42 | [gemini-3] nit: P2 | .github/workflows/test.yml:39 | Duplicate test execution in [gemini-4] suggestion: P3 | .github/workflows/test.yml:133 | Simplify branch comparison. PR MetadataSuggested Description Update:
Questions
Recommendation[ ] Approve | [ ] Approve with changes | [x] Request changes |
- Fix lcov SF: paths: bun generates SF:src/... relative to packages/opencode but diff-cover needs repo-root paths (packages/opencode/src/...). Add sed rewrite in unit job after coverage generation. - Handle missing coverage artifact gracefully: if coverage generation fails (continue-on-error), diff-cover job now skips with a clear warning annotation rather than failing with an opaque download-artifact error. - Change artifact if-no-files-found from ignore to warn for better visibility. - Add --force to git fetch to handle stale local refs idempotently. - Gate Python/diff-cover setup steps on successful artifact download.
Resolution SummaryResolving findings from Gemini Review, Codex Review:
|
Codex ReviewSummaryThis PR is focused and under 400 lines, and it improves CI coverage visibility. The 80% patch coverage gate is not reliably enforced in one failure path, so this should not merge yet. Findings[FINDING-1] blocking: P0 | .github/workflows/test.yml | Code Quality
Architecture
PR MetadataSuggested PR Title: Recommendation
Escalate to Gemini?
|
Gemini Deep ReviewSummaryThe PR introduces Findings[gemini-1] nit: P2 | .github/workflows/test.yml:34 | Redundant test execution in [gemini-2] issue: P1 | .github/workflows/test.yml:132 | Insecure enforcement bypass in [gemini-3] nit: P2 | .github/workflows/test.yml:157 | Missing artifacts for [gemini-4] nit: P2 | .github/workflows/test.yml:51 | Fragile path rewriting regex. PR MetadataSuggested PR Title: Questions
Recommendation[ ] Approve | [x] Approve with changes | [ ] Request changes |
Question Answers (Round 3)Q: Is the redundant test run in the A: The duplication is unintentional — it was pre-existing before this PR and not introduced here. Q: Should the 80% gate be made "hard" (failing the job) if the coverage report is missing for any reason during a PR? A: Yes, agreed. The latest push (28984d0 → next commit) changes the behavior: the |
- Fail diff-cover-enforcement with exit 1 when coverage artifact is missing (was: warn + skip, which allowed PRs to bypass the 80% gate silently) - Upload diff-cover JSON/HTML reports as artifacts for developer visibility - Use sed -E with flexible regex to handle ./src/ path prefix variants from bun (was: strict ^SF:src/ which could silently miss ./src/ paths)
Resolution SummaryResolving findings from Codex Review, Gemini Review:
|
Codex ReviewSummaryThis PR stays focused on CI coverage enforcement and is technically coherent: unit coverage artifact production, PR-only FindingsNone. Code Quality
Architecture
PR MetadataSuggested PR Title: Recommendation
Escalate to Gemini?
|
Gemini Deep ReviewSummaryThe PR successfully implements a robust 80% patch coverage gate for Findings[gemini-1] suggestion: P2 | .github/workflows/test.yml:33 | Redundant test execution in unit job [gemini-2] nit: P2 | .github/workflows/test.yml:39 | Misleading comment regarding failure behavior [gemini-3] nit: P2 | codecov.yml:15 | Potential confusion with informational status PR MetadataSuggested PR Title: ci: enforce 80% patch coverage gate for opencode via diff-cover Questions
Recommendation[x] Approve | [ ] Approve with changes | [ ] Request changes |
Question Answers (Round 4)Q: Are there plans to include A: Yes. The PR description explicitly defers Q: Is the A: The |
Comment incorrectly said diff-cover would skip on missing artifact. It now correctly states the enforcement job will fail the build.
Resolution SummaryResolving findings from Gemini Review:
|
Dual-Review Summary (Round 4)
Convergence: ✅ Achieved (1 blocking findings, reviewers status varies) Rounds: 4 Finding Classification: NEW: 0 | RE-RAISED: 3 Findings[Codex #1] P0 -
[Gemini #1] P2 -
[Gemini #2] P2 -
[Gemini #3] P2 -
|
andreiships-bot
left a comment
There was a problem hiding this comment.
Claude Single-Review
Summary
PR is in good shape after 4 rounds of dual-review. Prior blocking issues (threshold mismatch, base-branch fetch robustness, diff-cover scope, artifact bypass) have all been resolved. The current implementation is coherent: Codecov handles project-level visualization/ratcheting; the diff-cover-enforcement job handles hard 80% patch enforcement for packages/opencode; the required aggregator correctly allows diff-cover to be skipped on push-to-dev.
Two nits remain from prior rounds that were acknowledged but not fixed. Recording them for completeness; neither is blocking.
Findings
[FINDING-1] nit: P2 | .github/workflows/test.yml:61 | Artifact upload comment is stale after the hard-fail change
The Upload coverage artifact step comment still says "diff-cover-enforcement tolerates missing artifact by skipping the check". This is no longer accurate — the Coverage artifact missing — fail enforcement step now exits 1, making the gate hard. The comment will mislead maintainers.
Fix: Update to: # missing artifact causes diff-cover-enforcement to fail with a clear error (see last step of that job)
[FINDING-2] nit: P2 | .github/workflows/test.yml:153 | --src-paths absent from final diff-cover invocation
Previous resolution summary (round 3) stated --src-paths packages/opencode/src was added to scope diff-cover to the opencode package. Looking at the current file, the invocation is:
diff-cover coverage/lcov.info \
--compare-branch=${{ github.base_ref }} \
--fail-under=80 \
--json-report coverage/diff-cover.json \
--html-report coverage/diff-cover.html
--src-paths is not present. Without it, diff-cover will evaluate all changed files against the lcov data from packages/opencode only — files in packages/app or root will appear as 0% covered and could fail the gate when this PR's scope is explicitly limited to packages/opencode. The fix is already documented in the prior resolution: add --src-paths packages/opencode/src to the invocation, or alternatively the lcov coverage data already scoped to packages/opencode may implicitly mean only matching files are analyzed (depends on diff-cover version behavior). Worth verifying this doesn't block PRs touching packages/app.
Code Quality
- YAML structure is clean and follows existing patterns
- Artifact upload/download contract is correct
- Error paths are handled explicitly
- Coverage threshold is now consistent between codecov.yml (80%) and CI gate (80%)
- Base branch fetch is explicit and uses --force for stale ref robustness
Architecture
-
requiredaggregator correctly gates on diff-cover result withskippedallowance for non-PR events - Separation of concerns: Codecov = visualization + project ratchet; diff-cover = patch enforcement
-
continue-on-erroron coverage generation is intentional and documented; downstream gate is now hard
Recommendation
[x] Approve with changes | [ ] Request changes
Blocking: None. Both findings are nits that can be addressed in a follow-up or before merge at author's discretion. FINDING-2 should be verified against a PR that touches packages/app to confirm no false-failures.
Pull Request is not mergeable
Codex ReviewSummaryThis PR cleanly adds PR-only 80% patch coverage enforcement via FindingsNone. Code Quality
Architecture
PR MetadataSuggested PR Title: Recommendation
Escalate to Gemini?
|
Gemini Deep ReviewSummaryThe PR successfully implements an 80% patch coverage enforcement gate using Findings[gemini-1] suggestion: P2 | .github/workflows/test.yml:157 | PR MetadataSuggested PR Title: ci: add diff-cover enforcement for 80% patch coverage (scoped to opencode) Questions
Recommendation[X] Approve | [ ] Approve with changes | [ ] Request changes |
Resolution SummaryResolving findings from Gemini Review:
|
Dual-Review Summary (Round 1)
Convergence: ✅ Achieved (1 blocking findings, reviewers status varies) Rounds: 1 Finding Classification: NEW: 0 | RE-RAISED: 1 Findings[Codex #1] P0 -
[Gemini #1] P2 -
|
|
/trigger-ci |
Pull Request is not mergeable
969a5e0 to
7787629
Compare
Summary
codecov.ymlwith precision/range/ignore config; patch coverage isinformational: true(Codecov reports, doesn't gate)unitjob; rewrites source paths to repo-root-relative so diff-cover can correlate themdiff-cover-enforcementjob: enforces 80% patch coverage on changed lines usingdiff-cover --fail-under=80(inline steps)requiredgate updated to includediff-cover-enforcement(allowsskippedfor push-to-dev)packages/opencode/srconly —packages/app(Tauri/React UI) deferred until its test suite maturesChanges
codecov.yml— new config: precision 2, range 70–100, test file ignores,reach,diff,flags,filescomment layout.github/workflows/test.yml:unitjob: adds lcov path rewrite (SF:src/→SF:packages/opencode/src/) and uploads coverage artifact (typescript-coverage)diff-cover-enforcementjob: checkout with full history, fetch base branch, download artifact, run diff-cover with--fail-under=80 --src-paths packages/opencode/src; hard-fails if artifact is missingrequiredjob: addsdiff-cover-enforcementtoneeds, allowsskippedon push-to-devHow it works
unitjob runs tests, generatespackages/opencode/coverage/lcov.info, rewritesSF:paths to repo-root-relative, uploads artifactdiff-cover-enforcementjob downloads the artifact, fetches base branch for diff, runsdiff-coveragainst${{ github.base_ref }}Codecov behaviour
project.statushas noinformational: true— Codecov will block if overall coverage drops more than 1% from baseline (intentional ratchet)patch.statusisinformational: true— patch-level enforcement is handled by thediff-cover-enforcementjobTest Plan
packages/opencode/src— diff-cover job runs and reports patch coverage %devdirectly — diff-cover-enforcement is skipped,requiredgate still passesreach,diff,flags,fileslayoutRelated