Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 90 additions & 0 deletions docs/POC.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
# POC: measured test plans — codecov → `ready-tests` → QualityFlow → review

Status: proof of concept, 2026-08-18. Nothing here is on `main` yet.

## The problem

Agent PRs *claim* tests; nothing *measures* them. On the last 40 fullsend PRs,
90% of test-plan checkboxes were `[x]`, codecov posted a patch number on almost
every one, and 0 of 88 agent reviews read it. PR #6285 shipped with 6/6 boxes
ticked and `codecov/patch` at 28% against an 80% target. The 80% gate in
`AGENTS.md` did not hold because prose gates are advisory; a data path is not.

## The loop

```
codecov/patch check_run (head SHA)
│ conclusion=failure conclusion=success
▼ ▼
check-run-ready-label.yml check-run-ready-label.yml
add label `ready-tests` remove label + comment
▼ (label bridge: check_run never reaches CEL, labels do)
.fullsend/harness/qualityflow-ready-tests.yaml
trigger: label_changed && name == "ready-tests" && action == "added"
QualityFlow (COVERAGE_MODE=auto)
reads codecov via `gh api .../check-runs`, "measured overrides static":
STP gets a coverage-gap report, test-generator targets uncovered file:lines,
commits tests to the PR branch
pr-review (this branch)
step 2c fetches `codecov/*` check-runs for HEAD_SHA → `coverage` in the
correctness context package; claim < measurement → `test-inadequate` (medium)
code-implementation (this branch)
pr_body Testing section names the command that ran and the printed coverage;
no checkbox claims; unrun = `not measured`
```

Each hop is one boring mechanism (a check run, a label, a CEL expression, one
`gh api` call) so every step is observable in the PR timeline.

## Where each piece lives

| Hop | Repo / branch | What |
|-----|---------------|------|
| check_run → label | fullsend core `feat/checkrun-ready-label` | `check-run-ready-label.yml`: on `check_run` completed with name `codecov/*`, add/remove `ready-tests` (labels must start with `ready-`), plus a `check_run` shape in normevent |
| label → agent | `.fullsend` `feat/qf-ready-tests-trigger` | `harness/qualityflow-ready-tests.yaml` (`base:` = pinned QF harness, own CEL trigger) registered in `config.yaml`; per-org `dispatch.yml` mirrors it until the org leaves per-org mode |
| agent plans at the gap | QualityFlow engine `feat/coverage-gap-mode` (PR #1); `qualityflow-fullsend` `feat/coverage-gap-mode` | scenario-builder rule "measured overrides static", STP coverage-gap report, `COVERAGE_MODE: "auto"` |
| review reads the number | this repo, this branch | `skills/pr-review/SKILL.md` §2c, `sub-agents/correctness.md`, `skills/code-implementation/SKILL.md` |

## What this branch changes (agents)

- `skills/code-implementation/SKILL.md`: the Testing section of `pr_body`
reports what step 9c ran (`go test ./internal/harness/... — 47 passed`),
the coverage number only if the repo's test command already prints one,
and `not measured` for anything not run. Checkbox claims are banned.
- `skills/pr-review/SKILL.md`: new step 2c — one `gh api
repos/$REPO/commits/$HEAD_SHA/check-runs` call filtered to `codecov*`;
result (or `no coverage signal`) is passed as `coverage` to `correctness`.
- `skills/pr-review/sub-agents/correctness.md`: body claims tested + patch
below target → `test-inadequate`, medium, quoting both sides. No signal is
neither a finding nor a pass.

No new tokens: the codecov call is a single REST request with the token the
review already has.

## Try it

```bash
# 1. the number the review will see (any PR head)
gh api "repos/fullsend-ai/fullsend/commits/<HEAD_SHA>/check-runs?per_page=100" \
--jq '[.check_runs[] | select(.name|startswith("codecov")) | {name,conclusion,summary:.output.summary}]'

# 2. the routing decision, offline (fullsend built from main, .fullsend checkout)
fullsend dispatch --input-driver json --output-driver json \
--config-dir ~/Desktop/dot-fullsend \
--input-file <normevent with transition.label.name=ready-tests>
# → [{"agent":"qualityflow-ready-tests","role":"coder",...}]

# 3. label a non-fork PR `ready-tests` in an enrolled repo and watch dispatch → QualityFlow
```

## Not in this POC

- Codecov flags / coverport runtime coverage feeding tier-classifier (phase 2).
- Removing the `/fs-plan-tests` slash command — it stays; the label is additive.
- Model routing per event — harness overlays cannot pick a model; use two
harnesses if a cheaper QF pass is wanted.
16 changes: 16 additions & 0 deletions skills/code-implementation/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -836,6 +836,22 @@ from step 6 and structure `pr_body` to match its sections. Otherwise,
write a best-effort description covering what changed, why, testing
approach, and any caveats.

**Testing section — measured, not claimed.** Whatever the template
calls it (`## Testing`, `## Test plan`), it reports what step 9c
actually ran. Do not write checkbox claims like
`- [x] Unit tests for ResolveOverlays`: a checked box asserts a result
nobody measured, and it reaches the reviewer looking like evidence.

- Name the command and its outcome:
`go test ./internal/harness/... — 47 passed, 0 failed`.
- If the test command already prints coverage (the repo configures
`go test -cover`, `pytest --cov`, `jest --coverage`, or similar),
report the number it printed. Do not compute one by hand, and do not
enable a coverage flag the repo does not use.
- Anything you did not run — a suite that would not install, a manual
path you did not exercise — is `not measured`, with the reason. This
is the same disclosure step 9c requires in the commit message.

**Cross-repo ordering:** When the motivating issue references a
different repository (e.g., a failure observed in an upstream repo),
lead the summary with what files in **this** repository are changing
Expand Down
25 changes: 25 additions & 0 deletions skills/pr-review/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,29 @@ The PR description is a starting point, not a source of truth. Do not
treat its claims about the change as verified facts — confirm them
against the diff.

### 2c. Fetch patch coverage

The PR body's test plan is the author's claim about testing. Codecov
publishes the measurement as a check run on the PR head commit. Fetch
it so `correctness` can check the claim against a number instead of
trusting it:

```bash
COVERAGE=$(gh api \
"repos/${REPO_FULL_NAME}/commits/${HEAD_SHA}/check-runs?per_page=100" \
--jq '[.check_runs[]
| select(.name | startswith("codecov"))
| {name, conclusion, summary: .output.summary}]')
```

The `codecov/patch` entry carries the measurement in `summary` — e.g.
`28.24% of diff hit (target 80.00%)` — and `conclusion` is `failure`
when the patch is below the repo's target.

If the array is empty (the repo has no Codecov integration, or the
check has not reported yet), set `COVERAGE` to the string
`no coverage signal`. Never substitute a number read from the PR body.

### 2a. Prior review context (re-reviews)

Check if `/sandbox/workspace/prior-review.txt` exists and is non-empty:
Expand Down Expand Up @@ -583,6 +606,8 @@ For each selected sub-agent, assemble a context package containing:
- `prior_review_sha`: the SHA of the prior review (from 2a)
- `changed_since_prior`: file set that changed since prior review
- `pr_metadata`: title, body, author, labels, draft status
- `coverage`: patch coverage for the PR head from step 2c, or the
string `no coverage signal` (for `correctness`)
- `issue_context`: linked issue title, body, comments (for
`intent-coherence`)
- `cross_repo_context`: findings from 3a for `cross-repo-contracts`
Expand Down
19 changes: 19 additions & 0 deletions skills/pr-review/sub-agents/correctness.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,25 @@ Exclude the files already in the diff. Any hit outside the diff is a
medium-severity finding: "stale reference to removed/renamed
`<identifier>` in `<file>:<line>`."

### Test-plan claims vs measured coverage

A test plan in `pr_metadata.body` is a claim, not evidence. A checked
box (`- [x] Unit tests for ResolveOverlays`) records what the author
asserts, not what ran. The `coverage` field in your context package
carries the measured patch coverage for the PR head, taken from
Codecov's `codecov/patch` check run.

- Patch coverage below the target Codecov reports, while the body
claims the change is tested → `test-inadequate`, medium. Quote both
sides: "PR body claims unit tests for `ResolveOverlays`;
`codecov/patch` reports 28.24% of diff hit (target 80.00%)."
- `coverage` is `no coverage signal` → the claim is unverified. That
alone is not a finding, and it is not a pass either: judge test
adequacy from the diff, as you would with no test plan at all.

Never convert a test-plan claim into a statement that the change is
adequately tested. Adequacy comes from the diff and the measurement.

### CI coverage regression severity

When a change reduces which file paths trigger CI checks (e.g.,
Expand Down
Loading