Skip to content

docs(#2810): add patch coverage verification guidance for agents - #6138

Merged
ifireball merged 3 commits into
mainfrom
agent/2810-patch-coverage-guidance
Aug 13, 2026
Merged

docs(#2810): add patch coverage verification guidance for agents#6138
ifireball merged 3 commits into
mainfrom
agent/2810-patch-coverage-guidance

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

Add agent-visible guidance so code and fix agents verify approximate Go patch coverage against the 80% Codecov threshold before committing, instead of treating make go-test exit 0 as sufficient. This addresses frequent codecov/patch failures on first push that required human-initiated /fs-fix rounds.

Related Issue

Changes

  • AGENTS.md: Added a "Go coverage gate" bullet in the high-visibility "How to work in this repo" section, making the requirement hard to miss for any agent reading AGENTS.md
  • docs/contributing/go-code.md: Replaced the advisory coverage note with actionable instructions and a new "Verifying patch coverage locally" section containing step-by-step commands (go test -coverprofile, go tool cover -func, threshold assessment)
  • skills/check-patch-coverage/SKILL.md: New discoverable skill with the full coverage self-check procedure, including when to use/skip, the approximation model (function-level vs line-level), and threshold reference from .codecov.yml

Testing

  • All changes are documentation/guidance only — no Go production code modified
  • Secret scan passed
  • gitlint passed on commit message
  • make lint could not run in sandbox (pre-commit network restriction) — post-script validates

Checklist

  • PR title follows Conventional Commits (correct type, ! for breaking changes)
  • No secrets or sensitive data in changes
  • Changes scoped to in-repo agent guidance only — no harness, post-script, or CI threshold changes

Closes #2810

Post-script verification

  • Branch is not main/master (agent/2810-patch-coverage-guidance)
  • Secret scan passed (gitleaks — 55bd9f61ce57f9f2151a29d59a04d42b664110cf..HEAD)
  • PR body secret scan passed (gitleaks — no-git)

Code and fix agents treated `make go-test` exit 0 as sufficient
verification for Go changes, but make go-test does not enforce
the 80% Codecov patch coverage threshold. This led to frequent
codecov/patch failures on first push, requiring human-initiated
/fs-fix rounds.

Changes:
- AGENTS.md: Add a "Go coverage gate" bullet in the always-read
  "How to work in this repo" section, directing agents to verify
  approximate patch coverage before considering Go work done.
- docs/contributing/go-code.md: Replace the advisory coverage
  note with actionable instructions and a new "Verifying patch
  coverage locally" section with step-by-step commands (go test
  -coverprofile, go tool cover -func, threshold assessment).
- skills/check-patch-coverage/SKILL.md: New discoverable skill
  with the full coverage self-check procedure, including when
  to use/skip, the approximation model, and threshold reference.

Note: make lint could not run in sandbox (pre-commit failed due
to network restrictions on git fetch). The post-script runs an
authoritative pre-commit check on the runner.

Closes #2810
@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner August 12, 2026 09:47
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Triggers review agent dispatch label Aug 12, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 12, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:49 AM UTC · Completed 10:05 AM UTC

Commit: 9242fe6 · View workflow run →

@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown

Site preview

Preview: https://0d18e447-site.fullsend-ai.workers.dev

Commit: d6ce8a5f737fd5422ece09fb5e3ddffeb4216007

@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review

Findings

Medium

  • [edge-case] docs/contributing/go-code.md — The "Verifying patch coverage locally" step 1 uses git diff --name-only main -- '*.go' without noting that untracked (unstaged) new files are invisible to this command. An agent that creates a brand-new Go file and follows these steps without first running git add will get an empty file list and silently skip the entire coverage check. The parallel procedure in skills/check-patch-coverage/SKILL.md correctly warns "Stage new files first" — consider adding the same caveat to go-code.md step 1 for consistency.

  • [protected-path] AGENTS.md, skills/check-patch-coverage/SKILL.md — PR modifies files under protected paths (AGENTS.md, skills/). The PR links to issue Code agent should validate patch coverage meets repo threshold before opening PR #2810 which explicitly authorizes changes to these paths, and the description explains the rationale. Human approval is always required for protected-path changes, regardless of context.

Previous run

Review

Findings

Medium

Low

  • [list-label-verbosity] docs/contributing/go-code.md:29 — The numbered list label was expanded from Coverage: to Coverage — patch threshold (80%):. Sibling items use short one-or-two-word bold labels: Unit tests:, Vet:, E2E tests:. The em-dash qualifier and parenthetical threshold value make this label significantly longer and structurally inconsistent with its neighbors. Consider keeping the label short and letting the body text explain the threshold.

Labels: PR modifies docs/contributing/go-code.md alongside agent guidance files

Previous run (2)

Review

Findings

Medium

Low

  • [emphasis-style] AGENTS.md:17 — The new bullet uses **80%** mid-sentence. Existing bullets in AGENTS.md use bold only for imperatives (**must**, **prefer**, **not**) or subject markers at the start. The threshold value does not need additional bold emphasis.

  • [edge-case] skills/check-patch-coverage/SKILL.md:37git diff --name-only main -- '*.go' will not list newly created Go files that have not been staged with git add. A brand-new production file written but not yet staged would be invisible to this command, causing the procedure to skip the coverage check entirely. Consider documenting that files must be staged before running the check.

  • [technical-accuracy] skills/check-patch-coverage/SKILL.md:65 — Using basename $f to grep go tool cover -func output could produce false positive matches when multiple packages contain files with the same basename (e.g., internal/foo/config.go and internal/bar/config.go would both match grep 'config.go'). Grepping by a longer path suffix matching the git-relative path would be more precise.


Labels: PR adds Go coverage verification guidance and a new skill file under skills/

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment go Pull requests that update go code component/skills labels Aug 12, 2026
@ifireball

Copy link
Copy Markdown
Member

/fs-fix

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 11:26 AM UTC · Completed 11:32 AM UTC

Commit: 9242fe6 · View workflow run →

- Remove bold emphasis from 80% threshold value in AGENTS.md to match
  existing style convention (bold for imperatives only)
- Document staging requirement before running coverage check in SKILL.md
  so newly created files are not silently skipped
- Replace basename grep with git-relative path grep in SKILL.md to avoid
  false matches on same-named files across packages

Addresses review feedback on #6138
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 1 (human-triggered)

Addressed 3 of 4 review findings with code fixes. Disagreed with 1 (protected-path is informational, no code action needed). Changes: removed bold emphasis from threshold value in AGENTS.md, documented staging requirement and fixed basename grep precision in SKILL.md.

Fixed (3):

  1. emphasis-style: bold on 80% in AGENTS.md:17 does not match existing style (AGENTS.md): Removed bold from 80% threshold value to match existing convention where bold is used only for imperatives (must, prefer, not) and subject markers, not plain values.
  2. edge-case: git diff --name-only main misses unstaged new files in SKILL.md (skills/check-patch-coverage/SKILL.md): Added documentation note that new files must be staged with git add before running the coverage check, since git diff --name-only only sees tracked or staged files.
  3. technical-accuracy: basename grep can false-match same-named files across packages in SKILL.md (skills/check-patch-coverage/SKILL.md): Changed grep from basename to git-relative path ($f instead of basename $f), which is a unique substring match against go tool cover -func output and avoids false positives from same-named files in different packages.

Disagreed (1):

  1. protected-path: AGENTS.md and skills/ are protected paths: Informational finding only — no code fix needed. The reviewer acknowledges human approval is required for protected paths. The remaining findings explicitly reference these files and justify the changes.

Tests: passed

Next steps:

  • /fs-review — request a re-review of the changes
  • /fs-fix <your instruction> — run another fix pass with specific guidance
  • Push commits directly — review re-runs automatically on push
    Updated by fullsend fix agent

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 12, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:34 AM UTC · Completed 11:49 AM UTC

Commit: 2533c76 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added the component/docs User-facing documentation label Aug 12, 2026
@ifireball

Copy link
Copy Markdown
Member

/fs-fix

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 1:35 PM UTC · Completed 1:40 PM UTC

Commit: 2533c76 · View workflow run →

Revert the expanded label "Coverage — patch threshold (80%):" back to
"Coverage:" to match the short one-or-two-word bold labels used by its
sibling list items (Unit tests, Vet, E2E tests). The threshold details
are already explained in the body text.

Addresses review feedback on #6138
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 2 (human-triggered)

Addressed 1 of 2 review findings with a code fix. Disagreed with 1 (informational protected-path finding requiring no code change). Shortened the verbose coverage list label to match sibling style.

Fixed (1):

  1. list-label-verbosity: Coverage label too verbose compared to siblings (docs/contributing/go-code.md): Shortened the label from "Coverage — patch threshold (80%):" back to "Coverage:" to match the short one-or-two-word bold labels used by sibling list items (Unit tests, Vet, E2E tests). The threshold details are already explained in the body text.

Disagreed (1):

  1. protected-path: AGENTS.md and skills/ are protected paths: Informational finding only — no code fix needed. The reviewer acknowledges human approval is required for protected-path changes. This PR is authorized by issue Code agent should validate patch coverage meets repo threshold before opening PR #2810.

Tests: passed

Next steps:

  • /fs-review — request a re-review of the changes
  • /fs-fix <your instruction> — run another fix pass with specific guidance
  • Push commits directly — review re-runs automatically on push
    Updated by fullsend fix agent

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 12, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:41 PM UTC · Completed 1:56 PM UTC

Commit: d6ce8a5 · View workflow run →

@ifireball
ifireball added this pull request to the merge queue Aug 13, 2026
Merged via the queue into main with commit 3c7f5d0 Aug 13, 2026
16 checks passed
@ifireball
ifireball deleted the agent/2810-patch-coverage-guidance branch August 13, 2026 05:28
@fullsend-ai-retro

fullsend-ai-retro Bot commented Aug 13, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 5:31 AM UTC · Completed 5:50 AM UTC

Commit: d6ce8a5 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #6138 — patch coverage verification guidance

Workflow: issue #2810 → triage → code → 3 reviews × 2 fix iterations → human approval → merge.

Timeline: Code agent produced solid docs across 3 files in ~7 minutes (AGENTS.md, go-code.md, SKILL.md). Three review/fix cycles followed over ~4 hours of agent time (with ~5 hours of human wait time between triggers). The PR was merged ~20 hours after opening.

What went well

  • Code agent delivered a well-structured implementation matching the triage recommendation (3 files, clear step-by-step procedures, appropriate scope).
  • Review agent caught legitimate style issues (emphasis-style, label verbosity), real edge cases (unstaged files, basename false matches), and eventually identified a cross-file consistency gap between SKILL.md and go-code.md.
  • Fix agent correctly addressed findings in both iterations, appropriately disagreed with the informational protected-path finding, and produced clean commits.
  • Human reviewer provided appropriate oversight on a protected-path PR, triggering fixes and approving after the substantive issues were resolved.

What could go better

  1. Shipped inconsistency: The third review correctly identified that go-code.md step 1 lacks the staging warning present in SKILL.md step 1. This medium-severity finding was not addressed before merge. See proposal below.

  2. Evidence for #1213 (review cross-file pattern completeness): The first review found the git diff --name-only staging edge case in SKILL.md but not in go-code.md, where the identical command appeared in the same diff. Had both instances been flagged together, fix 1 could have addressed both files, potentially eliminating the third review cycle. Also relevant: agents#380, agents#466.

  3. Evidence for #5256 (fix agent generalizing fixes): Fix 1 added the staging warning to SKILL.md but didn't propagate it to go-code.md, which contained the same procedure in the same PR. Generalizing pattern-based fixes to parallel instances would have prevented the inconsistency.

  4. Evidence for #4970 (review single-pass completeness): The first review missed the list-label-verbosity issue ("Coverage — patch threshold (80%):" vs sibling labels like "Unit tests:", "Vet:"). The second review caught it, adding a fix iteration that could have been avoided.

  5. Evidence for #2794 (protected-path repetition): The protected-path finding appeared identically in all 3 review iterations, requiring the fix agent to disagree with it twice.

Cost

7 agent runs total (1 code, 3 review, 2 fix, 1 retro). The two fix cycles were human-triggered with ~1.5–2 hour gaps each, adding wall-clock latency. For a docs-only PR, one fix cycle would have been sufficient if the review agent had been more complete on the first pass and the fix agent had generalized across files.

Proposals filed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/docs User-facing documentation component/skills go Pull requests that update go code ready-for-review Triggers review agent dispatch requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Code agent should validate patch coverage meets repo threshold before opening PR

1 participant