feat(validate): gate merged child binary coverage at 97.8% by mrwogu · Pull Request #29 · comarch/git-byline

mrwogu · 2026-09-15T23:48:23Z

Summary

The coverage stage now counts statements that only run in the shipped binaries. go test cannot see package main, so the gate builds the binaries with -cover, runs them under GOCOVERDIR through happy and error paths, converts the counters with go tool covdata textfmt, and merges that profile with the unit profile through the new internal/covermerge package and the tools/covermerge command. cmd/git-byline moves its exit code mapping into an exitCode helper so the logic is unit-testable.

New table-driven tests across engine, model, store, gitcmd, hooks, app, provenance, notes, ci, report, dashboard, interop, transcript, and lock close the reachable branches: invalid UTF-8 at validation boundaries, oversized records, symlink and permission rejections, concurrent writers, malformed notes, and CLI usage errors.

The merged total lands at 97.8% (197 of 8760 statements uncovered) and the gate floor is 97.5%. The remaining statements are defensive: sync or close failures on healthy files, TOCTOU rechecks after atomic operations, invariant guards subsumed by earlier validation, and one encode-error wrap whose scenario needs a 500-file Git pipeline. Covering them would need injection seams across many packages, gaming the gate, or a fork storm, so the floor is honest instead of a literal 100%.

Portability notes for the new suite:

Write failures use read-only directories instead of RLIMIT_FSIZE, because the process-wide limit also blocks Go's internal testlog.txt appends in the same process. Working directory failure tests switch through t.Chdir so parallel tests never inherit a deleted directory, and the strict assertions use a path deeper than PATH_MAX, which fails the same way on macOS and Linux.
POSIX fake Git, chmod tricks, symlinks, and pipe handles stay behind windows and root guards, so the Windows matrix skips unix-only mechanisms instead of failing them.
Fake-heavy gitcmd cases run sequentially and the oversized rewritten note scenario is dropped: its 500-file pipeline forked around 1500 git subprocesses and tripped macOS with EINVAL and runner fork limits.
noteDecodeErrorClass treats plain io.EOF as malformed JSON because Go 1.24's decoder reports it for truncated notes while newer toolchains report "unexpected end of JSON input".
SonarCloud copy-paste detection now skips test sources through sonar-project.properties: the project mandates table-driven tests, and every duplicated line the quality gate counted sat in test files while the new production code carries zero duplication.

Scope

User-visible behavior: none
Interfaces or compatibility: none, persisted formats untouched
Documentation: not needed, gate internals only
Generated artifacts: none
Security and privacy: none, the binaries run locally inside tests only

Related issue

N/A

Validation

$ go run ./tools/validate
ok   gofmt
ok   vet
ok   test
ok   coverage
ok   build
ok   deps
ok   imports
ok   installers
ok   promptscript
ok   scans
validate: all 10 stages passed

$ merged unit + GOCOVERDIR profile
total: 97.8% of statements, 197 uncovered of 8760
floor: 97.5%

$ CI on the branch
Tests ubuntu (1.24.x, stable), Tests macos, Tests windows, Coverage,
Validate repository, Release snapshot: pass

Checklist

Focused change with unrelated refactors excluded
Tests added or updated
Documentation updated when behavior changed
Generated artifacts regenerated and reviewed
PromptScript sources updated instead of generated files
No secrets, private data, or unsafe fixtures included
Security impact considered
Breaking changes documented
Release impact understood

coderabbitai · 2026-09-15T23:48:38Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

Summary by CodeRabbit

New Features
- Added a tool to merge Go coverage reports and display coverage totals.
- Added validation stage filtering while preserving execution order.
- Expanded coverage collection to include command-line binaries.
Bug Fixes
- Improved exit-code handling for inconsistent results.
- Added safeguards for malformed input, repository mismatches, and file-operation failures.
Tests
- Significantly expanded edge-case and failure-path coverage across commands and integrations.
Chores
- Raised coverage requirements and strengthened CI coverage checks.

Walkthrough

Changes

The pull request adds merged unit and binary coverage collection, raises the coverage floor, and adds stage filtering to the validation tool. It also adds extensive boundary and failure-path tests across application commands, Git operations, hooks, storage, provenance, reporting, and interoperability.

Changes

Coverage validation

Layer / File(s)	Summary
Merged coverage pipeline `.github/workflows/ci.yml`, `internal/covermerge/`, `tools/covermerge/`, `tools/validate/*`	CI runs the coverage validation stage. The validator collects instrumented binary coverage, merges profiles, writes `coverage.out`, and enforces a 97.5% floor.
Command and repository boundary tests `internal/app/`, `internal/ci/`, `internal/gitcmd/`, `internal/hooks/`	New tests cover command failures, fake Git responses, filesystem limits, hook configuration errors, installer validation, and CI reconstruction paths.
Domain and persistence tests `internal/provenance/`, `internal/store/`, `internal/notes/`, `internal/report/`, `internal/interop/*`	New tests cover provenance rewrites, checkpoint and state persistence, note validation, aggregation limits, Git AI conversion, and import/export failures.
Validation and parser tests `internal/engine/`, `internal/model/`, `internal/preset/`, `internal/rewrite/`, `internal/dashboard/`, `internal/disclosure/`, `internal/transcript/`, `internal/lock/`	Additional tests cover UTF-8 validation, range guards, parser errors, rendering limits, rewrite input validation, transcript file checks, and lock target handling.

Priority: ⬇️ Low

Estimated code review effort: 5 (Critical) | ~120 minutes

Merge Risk: 🟡 Moderate · up to 39b75

The validation suite can fail or hang on supported CI environments, and its coverage checks can report incorrect results. These issues should be corrected before merge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 7.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 826 functions across 50 files. (5 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the validation coverage change and the merged binary coverage result.
Description check	✅ Passed	The description directly explains merged binary coverage, the new coverage tooling, tests, coverage floor, and validation results.
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.

Full details: Docstring Coverage

Explanation

Docstring coverage is 7.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 826 functions across 50 files. (5 skipped: 3 unsupported, 2 over the file limit.)

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch chore/coverage

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.}

coderabbitai

Actionable comments posted: 10

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: e0ffcb23-0a67-41f6-8926-7bc59b9946e1

📥 Commits

Reviewing files that changed from the base of the PR and between 9d6306f and ecd4b56.

📒 Files selected for processing (52)

.github/workflows/ci.yml
cmd/git-byline/main.go
cmd/git-byline/main_test.go
codecov.yml
internal/app/app_round2_test.go
internal/app/command_boundary_coverage_test.go
internal/app/coverage_additional_test.go
internal/app/update_coverage_additional_test.go
internal/app/update_round2_unix_test.go
internal/app/verify_coverage_additional_test.go
internal/ci/coverage_more_test.go
internal/covermerge/covermerge.go
internal/covermerge/covermerge_test.go
internal/dashboard/coverage_additional_test.go
internal/disclosure/disclosure_test.go
internal/engine/engine_test.go
internal/gitcmd/coverage_more_test.go
internal/gitcmd/coverage_tempfile_unix_test.go
internal/gitcmd/coverage_test.go
internal/hooks/coverage_edges_test.go
internal/hooks/coverage_round2_test.go
internal/hooks/coverage_test.go
internal/interop/coverage_additional_test.go
internal/lock/lock_test.go
internal/model/model_test.go
internal/notes/coverage_additional_test.go
internal/notes/notes_test.go
internal/preset/preset_test.go
internal/provenance/coverage_rewrite_more_test.go
internal/provenance/coverage_rewrite_pending_more_test.go
internal/provenance/coverage_rewrite_test.go
internal/provenance/coverage_service_annotate_more_test.go
internal/provenance/coverage_service_more_test.go
internal/provenance/coverage_service_test.go
internal/report/coverage_test.go
internal/rewrite/rewrite_test.go
internal/store/coverage_additional_test.go
internal/store/coverage_unix_test.go
internal/store/store_test.go
internal/transcript/identity_unix_test.go
internal/transcript/open_unix_test.go
tools/covermerge/main.go
tools/covermerge/main_test.go
tools/validate/checks.go
tools/validate/checks_more_test.go
tools/validate/checks_test.go
tools/validate/installers_more_test.go
tools/validate/main.go
tools/validate/main_more_test.go
tools/validate/main_test.go
tools/validate/promptscript_more_test.go
tools/validate/scan_more_test.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

codecov · 2026-09-16T11:55:56Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

coderabbitai

Actionable comments posted: 3

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: ae69368b-623b-44c0-894a-ecd484c73fc2

📥 Commits

Reviewing files that changed from the base of the PR and between 39f961a and 2add39a.

📒 Files selected for processing (24)

internal/app/app_round2_test.go
internal/app/command_boundary_coverage_test.go
internal/app/coverage_additional_test.go
internal/app/update_coverage_additional_test.go
internal/app/verify_coverage_additional_test.go
internal/ci/coverage_more_test.go
internal/gitcmd/coverage_more_test.go
internal/gitcmd/coverage_test.go
internal/hooks/coverage_edges_test.go
internal/hooks/coverage_test.go
internal/interop/coverage_additional_test.go
internal/provenance/coverage_rewrite_more_test.go
internal/provenance/coverage_rewrite_pending_more_test.go
internal/provenance/coverage_rewrite_test.go
internal/provenance/coverage_service_annotate_more_test.go
internal/provenance/coverage_service_test.go
internal/report/coverage_test.go
internal/report/report.go
internal/store/coverage_additional_test.go
internal/store/store_test.go
tools/validate/checks_more_test.go
tools/validate/checks_test.go
tools/validate/promptscript_more_test.go
tools/validate/scan_more_test.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

coderabbitai

Actionable comments posted: 1

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 2aae2ab0-46b4-4bf1-929b-d63778e08638

📥 Commits

Reviewing files that changed from the base of the PR and between 2add39a and 5513d20.

📒 Files selected for processing (12)

internal/app/coverage_additional_test.go
internal/covermerge/covermerge_test.go
internal/gitcmd/coverage_test.go
internal/interop/coverage_additional_test.go
internal/interop/interop_test.go
internal/provenance/coverage_rewrite_more_test.go
internal/provenance/coverage_service_test.go
sonar-project.properties
tools/validate/checks_more_test.go
tools/validate/installers_more_test.go
tools/validate/promptscript_more_test.go
tools/validate/scan_more_test.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

go test cannot see statements that only run in package main, so the gate under-counted the real binary. The coverage stage now builds the shipped binaries with -cover, exercises happy and error paths under GOCOVERDIR, converts counter data with go tool covdata textfmt, and merges that profile with the unit profile through the new covermerge package and tools/covermerge command. cmd/git-byline moves its exit code mapping into an exitCode helper so the logic is unit-testable. The floor is 97.5: about 196 statements are defensive guards (file sync or close failures on healthy files, TOCTOU rechecks, invariants subsumed by earlier validation) that cannot run without injection seams or gaming the gate. CI runs the stage and uploads the merged profile; codecov expects a full patch.

notes.Decode reports plain io.EOF for a truncated note on Go 1.24 ("decode note end: EOF") while newer toolchains report "unexpected end of JSON input". Both mean the note was cut short, so the error class now covers every EOF variant instead of depending on the Go version.

Table-driven cases across engine, model, store, gitcmd, hooks, app, provenance, notes, ci, report, dashboard, interop, transcript, and lock close the reachable branches: invalid UTF-8 at every validation boundary, oversized records, symlink and permission rejections, concurrent writers, malformed notes, and CLI usage errors. Three patterns keep the suite honest on every runner: - Write failures use read-only directories instead of RLIMIT_FSIZE, because the process-wide limit also blocks Go internal testlog.txt appends and can fail the whole package. Working directory failure tests switch through t.Chdir so parallel tests never inherit a deleted directory. - POSIX fake Git, chmod tricks, and symlinks stay behind windows and root guards, so the Windows matrix skips unix-only mechanisms instead of failing them. - Fake-heavy gitcmd cases run sequentially and the oversized rewritten note scenario is gone: its 500-file pipeline forked around 1500 git subprocesses and tripped macOS EINVAL and runner fork limits. That branch stays uncovered by choice. What stays uncovered is defensive only: sync or close failures on healthy files, TOCTOU rechecks after atomic operations, and invariant guards that earlier validation already makes unreachable.

The quality gate failed on new-code duplication, and every duplicated line it counted sits in test files: table-driven cases repeat their arrange-act blocks by convention, which the project rules mandate. Production code added by this branch carries zero duplicated lines, so copy-paste detection now skips test sources while everything else stays analyzed.

SonarCloud flagged 4.8% duplicated lines on the new test sources, above the 3% quality gate. Extract the repeated repository, note, and capture fixtures into shared helpers in gitcmd, provenance, interop, covermerge, and validate. Scenarios and assertions stay identical; only setup structure moved.

SonarCloud reported 24 new issues on the pull request. Extract constants for the shared GOPROXY and -stages literals in validate, rename the installer helper parameter that shadowed the predeclared new, inline three needless condition variables in interop, and split twenty cognitive-complexity hotspots into named subtests and helpers. Scenarios and assertions are unchanged.

Main moved commitExistsQuiet from *Repo to *BranchScanner in the guided checkpoint recovery change. Call the scanner the production code now uses so the merged tree compiles.

coderabbitai

Actionable comments posted: 9

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 02c98bf7-7473-41d7-ab4b-018af941f849

📥 Commits

Reviewing files that changed from the base of the PR and between 5513d20 and 39b7579.

📒 Files selected for processing (55)

.github/workflows/ci.yml
cmd/git-byline/main.go
cmd/git-byline/main_test.go
codecov.yml
internal/app/app_round2_test.go
internal/app/command_boundary_coverage_test.go
internal/app/coverage_additional_test.go
internal/app/update_coverage_additional_test.go
internal/app/update_round2_unix_test.go
internal/app/verify_coverage_additional_test.go
internal/ci/coverage_more_test.go
internal/covermerge/covermerge.go
internal/covermerge/covermerge_test.go
internal/dashboard/coverage_additional_test.go
internal/disclosure/disclosure_test.go
internal/engine/engine_test.go
internal/gitcmd/coverage_more_test.go
internal/gitcmd/coverage_tempfile_unix_test.go
internal/gitcmd/coverage_test.go
internal/hooks/coverage_edges_test.go
internal/hooks/coverage_round2_test.go
internal/hooks/coverage_test.go
internal/interop/coverage_additional_test.go
internal/interop/interop_test.go
internal/lock/lock_test.go
internal/model/model_test.go
internal/notes/coverage_additional_test.go
internal/notes/notes_test.go
internal/preset/preset_test.go
internal/provenance/coverage_rewrite_more_test.go
internal/provenance/coverage_rewrite_pending_more_test.go
internal/provenance/coverage_rewrite_test.go
internal/provenance/coverage_service_annotate_more_test.go
internal/provenance/coverage_service_more_test.go
internal/provenance/coverage_service_test.go
internal/report/coverage_test.go
internal/report/report.go
internal/rewrite/rewrite_test.go
internal/store/coverage_additional_test.go
internal/store/coverage_unix_test.go
internal/store/store_test.go
internal/transcript/identity_unix_test.go
internal/transcript/open_unix_test.go
sonar-project.properties
tools/covermerge/main.go
tools/covermerge/main_test.go
tools/validate/checks.go
tools/validate/checks_more_test.go
tools/validate/checks_test.go
tools/validate/installers_more_test.go
tools/validate/main.go
tools/validate/main_more_test.go
tools/validate/main_test.go
tools/validate/promptscript_more_test.go
tools/validate/scan_more_test.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

The go 1.24 toolchain CI uses counts a few more uncovered blocks than 1.27, and the merged guided recovery code added fresh gaps. Together they pushed the merged total below the floor. Cover the recover operational edges: discovery and preview failures, the pre-root base label, plain and JSON drop output, the droppable skip in the refusal listing, and the conflicting-note annotate failure. Reject unsupported CI providers in ci as well. Merged total on the 1.24 toolchain now measures 97.6 percent on macOS and passes the coverage stage in a linux container.

Act on the CodeRabbit review: compare the exact merged coverage percentage against the floor instead of the tool output rounded to one decimal, and recalibrate the floor to 97.4 for the Go 1.24 toolchain CI builds with, which splits more coverage blocks than newer toolchains. Check the covermerge summary write and take an io.Writer so the failure is testable. Restore the offline env pins on the workflow steps that compile validate. Align the coverage policy in the validation docs with the merged gate and the 100 percent codecov patch target. Test fixes: pair malformed-output cases with their expected output, stop sharing a Ranges backing array in the provenance invalid note, close the archive watcher channel on the error path, return the verify exit code from the helper, assert the garbage-note verify issue, move the Windows fake-git skip into the helper, guard the profile slice in failure messages, skip permission cases for root, and replace the RLIMIT_FSIZE tempfile injection with a read-only parent directory. Add scanner noise, exclusive output, missing install root, symlink loop, FIFO session, and truncated checkpoint coverage.

sonarqubecloud · 2026-09-16T23:41:54Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
1.3% Duplication on New Code

See analysis details on SonarQube Cloud

coderabbitai Bot requested changes Sep 16, 2026

View reviewed changes

mrwogu force-pushed the chore/coverage branch 2 times, most recently from 39f961a to 69cb126 Compare September 16, 2026 11:52

mrwogu force-pushed the chore/coverage branch from 69cb126 to 2add39a Compare September 16, 2026 12:46

coderabbitai Bot requested changes Sep 16, 2026

View reviewed changes

Comment thread internal/app/command_boundary_coverage_test.go

Comment thread tools/validate/checks_test.go Outdated

Comment thread tools/validate/promptscript_more_test.go Outdated

mrwogu force-pushed the chore/coverage branch from 2add39a to 521f312 Compare September 16, 2026 13:01

coderabbitai Bot requested changes Sep 16, 2026

View reviewed changes

Comment thread internal/provenance/coverage_rewrite_more_test.go

mrwogu added 7 commits September 16, 2026 20:46

test(gitcmd): adapt containment tests to branch scanner

39b7579

Main moved commitExistsQuiet from *Repo to *BranchScanner in the guided checkpoint recovery change. Call the scanner the production code now uses so the merged tree compiles.

mrwogu force-pushed the chore/coverage branch from 50b6557 to 39b7579 Compare September 16, 2026 19:01

coderabbitai Bot requested changes Sep 16, 2026

View reviewed changes

mrwogu added 2 commits September 17, 2026 00:04

mrwogu merged commit ea11733 into main Sep 17, 2026
19 checks passed

mrwogu deleted the chore/coverage branch September 17, 2026 07:53

mrwogu mentioned this pull request Sep 16, 2026

chore(main): release 1.2.0 #27

Merged

Conversation

mrwogu commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Scope

Related issue

Validation

Checklist

Uh oh!

coderabbitai Bot commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Summary by CodeRabbit

Walkthrough

Changes

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Sep 16, 2026

Codecov Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud Bot commented Sep 16, 2026

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

mrwogu commented Sep 15, 2026 •

edited

Loading

coderabbitai Bot commented Sep 15, 2026 •

edited

Loading