Skip to content

docs(memory): record how to reproduce CI's merged coverage number - #551

Merged
rhuanbarreto merged 1 commit into
mainfrom
rhuanbarreto/memory-coverage-merge-recipe
Aug 5, 2026
Merged

docs(memory): record how to reproduce CI's merged coverage number#551
rhuanbarreto merged 1 commit into
mainfrom
rhuanbarreto/memory-coverage-merge-recipe

Conversation

@rhuanbarreto

Copy link
Copy Markdown
Contributor

One-line refinement to an agent-memory entry, split out of #550 because it was learned after that PR merged.

Why

The entry already warned that a single-platform local coverage run is a floor rather than the CI figure. It did not say how to obtain the real one — which leaves the warning unactionable, so the natural next step is to plan from the local numbers anyway.

That is actively misleading for per-file work. src/helpers/platform.ts reads as 64 missed lines on a Windows-only run and 12 once both platforms are merged, because Linux covers the other branches. Planning from the local view means chasing ~52 lines that are already covered. Updating #522's Phase 3 list ran into exactly this — the original issue listed platform.ts in the long tail on the strength of a single-platform reading.

What

Adds the artifact-merge recipe: download both coverage-linux and coverage-windows, union the DA:<line>,<hits> records keyed by the path from src/ onward, and count a line covered when the summed hits exceed zero.

Verified — it reproduces the figure the coverage comment reports for #550 (9,414 / 9,748 = 96.6%) digit for digit, which is what makes the per-file numbers trustworthy enough to plan from.

Scope

.claude/agent-memory/** only. No source, tests, ADRs, or CI config. archgate check passes 51/51 and oxfmt --check is clean; the file is exempt from GEN-004 by convention, and I checked it carries no stray control or non-ASCII characters.

The existing entry noted that a single-platform local run is a floor rather
than the CI figure, but not how to obtain the real one. Without it, a local
run reports gaps that do not exist: `platform.ts` reads as 64 missed lines on
Windows alone and 12 once both platforms are merged, so per-file planning off
a local run chases work that is already covered.

Adds the artifact-merge recipe, verified to reproduce the figure the coverage
comment reports.

Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The coverage troubleshooting guidance now documents how to reproduce CI’s merged Linux and Windows coverage result. The procedure downloads both coverage artifacts, unions DA records by source path, and counts lines covered on either platform. The existing single-platform limitation note remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes documenting how to reproduce CI's merged coverage number.
Description check ✅ Passed The description directly explains the merged coverage recipe, its purpose, validation, and limited scope.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying archgate-cli with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9b25a1a
Status: ✅  Deploy successful!
Preview URL: https://0e62a4f2.archgate-cli.pages.dev
Branch Preview URL: https://rhuanbarreto-memory-coverage.archgate-cli.pages.dev

View logs

@rhuanbarreto
rhuanbarreto merged commit e4e6521 into main Aug 5, 2026
19 of 20 checks passed
@rhuanbarreto
rhuanbarreto deleted the rhuanbarreto/memory-coverage-merge-recipe branch August 5, 2026 20:32

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 @.claude/agent-memory/archgate-developer/MEMORY.md:
- Line 23: Update the LCOV merge logic described in the coverage workflow to
normalize Windows backslashes to forward slashes before extracting the
src-relative path and constructing merge keys. Ensure Linux and Windows records
for the same source file use the identical key, matching the CI action’s
normalization behavior.
🪄 Autofix

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: ASSERTIVE

Plan: Pro Plus

Run ID: 2362a75f-aaaa-42f7-ac5b-1ce3bbe73f64

📥 Commits

Reviewing files that changed from the base of the PR and between b9338c0 and 9b25a1a.

📒 Files selected for processing (1)
  • .claude/agent-memory/archgate-developer/MEMORY.md
📜 Review details
⏰ Context from checks skipped due to timeout. (9)
  • GitHub Check: Lint, Test & Check
  • GitHub Check: Smoke Test (Windows) / Windows
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Analyze (python)
  • GitHub Check: Analyze (csharp)
  • GitHub Check: Analyze (java-kotlin)
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Cloudflare Pages
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-06-11T12:50:28.661Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 406
File: .claude/agent-memory/archgate-developer/feedback_prefer_tests_over_adr_rules.md:8-18
Timestamp: 2026-06-11T12:50:28.661Z
Learning: In `archgate/cli`, for markdown files under `.claude/agent-memory/`, follow the established convention: use YAML frontmatter (with a `name:` field used as the document title) and do not require a top-level `#` (H1) heading. During code review, do not flag missing first-line/first-top-level H1 headings (e.g., MD041) for these agent-memory files since markdownlint is not part of the repo’s `bun run validate` lint pipeline (oxlint/oxfmt only).

Applied to files:

  • .claude/agent-memory/archgate-developer/MEMORY.md
📚 Learning: 2026-07-25T00:05:20.592Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 496
File: .claude/agent-memory/archgate-developer/project_test_isolation_gotchas.md:10-10
Timestamp: 2026-07-25T00:05:20.592Z
Learning: When reviewing documentation/agent-memory entries under `.claude/agent-memory/**`, do not enforce GEN-004’s “forward-only” comment/narrative requirement. These entries are allowed to keep historical/past-tense incident narratives and dated markers (e.g., `Found YYYY-MM-DD`) because the context is intended to help future agents evaluate edge cases. Outside this scope, GEN-004’s forward-only rule should still apply.

Applied to files:

  • .claude/agent-memory/archgate-developer/MEMORY.md
🔇 Additional comments (1)
.claude/agent-memory/archgate-developer/MEMORY.md (1)

23-23: 🎯 Functional Correctness

No change needed for the gh run download instruction.

gh run download automatically creates separate directories from artifact names and uses one --dir as the base destination, so gh run download <run-id> -n coverage-linux -n coverage-windows is equivalent to the shown artifact names while being clearer.

			> Likely an incorrect or invalid review comment.

- **Splitting a test file for `oxlint`'s 500-line `max-lines` cap: add a sibling `<name>-<suffix>.test.ts`, don't trim coverage.** Precedent already established by `check-max-warnings.test.ts` as a sibling of `check.test.ts`; followed again for `reporter-strict.test.ts`, `sync-strict.test.ts`, and the `*-strict.test.ts` integration files when ARCH-026's tests pushed their parent files over the cap.
- **A cache-busting dynamic import hides coverage: `await import(`../mod?t=${Date.now()}`)` makes Bun load a second module instance whose execution is attributed to nothing.** The source file then reports its paths uncovered while they are in fact tested, so the reporter understates and the "gap" is an illusion — check for this specifier before writing tests for any file that looks mysteriously uncovered. Removing it took `update-check.ts` from 76.47% to 100% with zero new tests. A static import is safe only when the module holds no mutable module-level state; verify that first (file-backed caches and `Bun.env` reads at call time are fine).
- **Reproduce CI's coverage number yourself — bun's `All files` summary line is not it.** CI filters the merged lcov to `src/*` and computes `sum(LH)/sum(LF)`; bun's own table includes `tests/` and averages differently, so the two disagree by several points. Locally: `awk -F: '/^SF:/{p=($2~/^src[\\\/]/)} /^LF:/{if(p)f+=$2} /^LH:/{if(p)h+=$2} END{printf "%.2f\n",h/f*100}' coverage/lcov.info`. A single-platform local run is a floor, not the CI figure — CI unions Linux and Windows and counts a line covered if either platform hit it.
- **Reproduce CI's coverage number yourself — bun's `All files` summary line is not it.** CI filters the merged lcov to `src/*` and computes `sum(LH)/sum(LF)`; bun's own table includes `tests/` and averages differently, so the two disagree by several points. Locally: `awk -F: '/^SF:/{p=($2~/^src[\\\/]/)} /^LF:/{if(p)f+=$2} /^LH:/{if(p)h+=$2} END{printf "%.2f\n",h/f*100}' coverage/lcov.info`. A single-platform local run is a floor, not the CI figure — CI unions Linux and Windows and counts a line covered if either platform hit it, and the gap is large enough to send you chasing phantom work (`platform.ts` reads as 64 missed on Windows alone, 12 merged). To get the real per-file picture, `gh run download <run-id> -n coverage-linux -D a -n coverage-windows -D b`, then union the `DA:<line>,<hits>` records across both files keyed by the path from `src/` onward and count a line covered when the summed hits exceed zero; that reproduces the number the PR comment reports, digit for digit.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Normalize Windows LCOV paths before building the merge key.

The CI action converts \ to / before merging the Windows tracefile. Apply the same normalization before extracting the src/-relative path. Otherwise, Windows and Linux records for the same file can remain under different keys, producing a coverage result that does not match CI.

🤖 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 @.claude/agent-memory/archgate-developer/MEMORY.md at line 23, Update the
LCOV merge logic described in the coverage workflow to normalize Windows
backslashes to forward slashes before extracting the src-relative path and
constructing merge keys. Ensure Linux and Windows records for the same source
file use the identical key, matching the CI action’s normalization behavior.

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Code Coverage

Metric Value
Lines 96.6% (9414 / 9748)
Threshold 95% minimum — met
Platforms Linux + Windows

Full HTML report available in workflow artifacts.

Per-directory breakdown
Directory Coverage Lines
src/commands/ 98.3% 2318 / 2358
src/engine/ 98.7% 2560 / 2594
src/formats/ 98.7% 149 / 151
src/helpers/ 94.4% 4387 / 4645

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