Skip to content

docs: fold the coverage census into the note that already existed - #349

Merged
rainmanjam merged 7 commits into
mainfrom
docs/coverage-census
Aug 14, 2026
Merged

docs: fold the coverage census into the note that already existed#349
rainmanjam merged 7 commits into
mainfrom
docs/coverage-census

Conversation

@rainmanjam

@rainmanjam rainmanjam commented Aug 14, 2026

Copy link
Copy Markdown
Owner

docs/notes/live-test-coverage-gaps.md was written on 2026-08-13 and already makes the same argument, better: the defects worth finding are composition bugs, where both halves are individually correct and no unit test can reach the seam.

I wrote a second document without finding the first. Three subagents told me so in a row — each was pointed at a census that doesn't exist on main, and each found this note instead.

Two documents making one argument is how both stop being read. So the census is deleted, and only what it genuinely added is folded in:

The unit-coverage measurement

43 packages, zero without tests, three under 60% — and all three are scripts (build-ignored tools), internal/testenv (the guard harness itself), and cmd (mostly main()).

This is what turns "raise test coverage" from an appetite into a decision: there's no percentage worth chasing. Same conclusion the note already reached from the other direction.

The automation count, corrected

Grepping ci.yml alone said seven suites were unautomated. chat, oauth and automod each run from their own workflow on a weekly cron with a path-scoped pull_request trigger — deliberate, and argued in those files. Four are genuinely unautomated, and install is no longer one of them after #353.

The chat gap, narrowed

acceptance-chat.sh is credential-free for 15 of 17 checks and covers Twitch and Kick. internal/chat ships five adapters — YouTube, Facebook and Rumble have no live coverage anywhere. That's the untested half of the five-platform claim the site now makes, and it's new checks to write rather than CI to wire.

Plus the owner's decisions, so they aren't asked again.

…rage is not the gap

Written so "maximize coverage" becomes a finite list rather than an appetite,
and so the next person does not start by re-deriving what is already known.

THE HEADLINE IS THAT PRIORITY A IS THE LEAST VALUABLE WORK AVAILABLE, and the
measurement says so rather than an opinion. 43 packages, ZERO with no tests at
all, and only three under 60%: scripts at 8.5% (build-ignored operational
tools), internal/testenv at 47.3% (the guard harness itself), and cmd at 54.3%
(mostly main and flag wiring). Everything else is 70%+. A coverage-percentage
push would move a number and find nothing.

THE REAL GAP IS THAT THE WEBSITE NOW ADVERTISES CAPABILITIES NOTHING EXERCISES
END TO END. Two mixes to one destination has a whole /features section and has
never run once: the negotiation requires a GPU and the staging box has none, so
Twitch refuses it by design. Capped VBR is asserted through argv and never
measured through an encode. Five-platform chat and multistream both have
acceptance suites that are not in CI.

THE SUITE MATRIX is the actionable part: 22 suites, 15 in CI, and the 7 that are
not are excluded for credentials or hardware rather than neglect. Each is listed
with what it needs, so the question stops being "why is this not tested" and
becomes "which environment can run it".

Records the owner's decisions so they do not have to be asked again: credentials
are available and may be used, priority is C then B then A, a stack of PRs is
preferred over one large one, and the OVH box may be disrupted -- that is what
it is for.

Ends with the failure mode this repository keeps finding, because a coverage
push is exactly where it recurs: a test that passes without asserting anything.
A mistyped `go test -run` name exits 0. A mechanism can report success while the
thing it exists to do is broken -- a sticky column reported position:sticky
while rendering transparent, and a VFR stream looked like it had lost 1.6
seconds because ffprobe derives durations MPEG-TS does not store.
Copilot AI lite review requested due to automatic review settings August 14, 2026 17:56

Copilot AI 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.

Pull request overview

Adds a documented, measured inventory of Go package coverage and acceptance-suite execution environments so that “maximize coverage” becomes a bounded, actionable plan rather than a vague goal, and so future work focuses on end-to-end gaps that aren’t exercised.

Changes:

  • Introduces a dated coverage census summarizing package-level unit coverage and highlighting outliers.
  • Records an acceptance suite × environment matrix, distinguishing CI-runnable suites from those gated by credentials/hardware.
  • Captures prioritized testing work (C → B → A) and a proposed PR stack to close end-to-end coverage gaps.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread docs/TEST-COVERAGE-CENSUS.md Outdated
Comment on lines +94 to +95
One per area, in this order. Each should be independently reviewable and
independently revertable.
Comment thread docs/TEST-COVERAGE-CENSUS.md Outdated
| synth | ✅ | — | |
| browser | ✅ | docker, host | |
| docker | ✅ | docker | |
| encoders | ✅ | **gpu** | CI has none — runs degraded |
… the chat gap is a different one

Grepping ci.yml alone produced a wrong number. chat, oauth and automod each run
from a dedicated workflow on a weekly cron plus a path-scoped pull_request
trigger, and those files argue the placement: a third-party network dependency
on every push is a flake generator, and the failures they catch arrive with no
commit of ours at all. Four suites are genuinely unautomated, not seven.

The chat entry was wrong in a more useful way. acceptance-chat.sh is already
credential-free for 15 of its 17 checks with both skips stating their reason, so
there was nothing to wire. What it does not cover is three of the five adapters:
YouTube, Facebook and Rumble have no live coverage anywhere. That is the real
untested half of the five-platform claim the site now advertises.

Found by a subagent that checked the claim instead of inheriting it, which is
the same correction loop the rest of this document argues for.
…d delete it

docs/notes/live-test-coverage-gaps.md was written on 2026-08-13 and makes the
same argument the census did, better: the defects worth finding are COMPOSITION
bugs, where both halves are individually correct and no unit test can reach the
seam. I wrote a second document without finding the first, and three subagents
told me so in a row -- each was pointed at a census that did not exist on main
and found this note instead.

Two documents making one argument is how both stop being read, so the census is
deleted and only what it genuinely added is kept:

  - THE UNIT-COVERAGE MEASUREMENT. 43 packages, zero without tests, three under
    60%, and all three are scripts, the guard harness itself, and main(). This
    is what turns "raise test coverage" from an appetite into a decision: there
    is no percentage worth chasing here, which is the same conclusion this note
    already reached from the other direction.

  - THE AUTOMATION COUNT, CORRECTED. Grepping ci.yml alone said seven suites
    were unautomated. chat, oauth and automod each run from their own workflow
    on a weekly cron with a path-scoped pull_request trigger -- deliberate, and
    argued in those files. Four are genuinely unautomated, and install is no
    longer one of them.

  - THE CHAT GAP, NARROWED. acceptance-chat.sh is credential-free for 15 of 17
    checks and covers Twitch and Kick. YouTube, Facebook and Rumble have no live
    coverage anywhere, which is the untested half of the five-platform claim the
    site now makes. That is new checks to write, not CI to wire.

  - The owner's decisions, so they are not asked again.
@rainmanjam rainmanjam changed the title docs: a measured test-coverage census, and the finding that unit coverage is not the gap docs: fold the coverage census into the note that already existed Aug 14, 2026
rainmanjam added a commit that referenced this pull request Aug 14, 2026
…cs-only PRs unmergeable (#357)

* ci: a skipped matrix job creates no per-leg contexts, so #351 made docs PRs unmergeable

#351 gated the acceptance matrix with a job-level `if:` on the reasoning that a
skipped job still reports and a skip satisfies a required check. That is true of
an ordinary job and false of a matrix one: a skipped matrix never expands, so
the per-leg contexts are not skipped, they are never created.

#349 is the proof. Fifteen checks green, zero failures, one entry in the checks
list named literally `acceptance: ${{ matrix.suite }}`, and all twelve required
`acceptance: acceptance-*` contexts reported MISSING. A documentation-only pull
request -- the exact case #351 was built for -- could not be merged at all.

The #351 PR body named this trap ("if a required check is skipped by a path
filter, GitHub can leave a PR unmergeable forever") and then walked into it,
because the mitigation it chose was the same bug in a different costume.

The matrix now always expands and every leg reports. What is conditional is the
work: each step carries the gate, a documentation-only run costs a runner
allocation instead of a suite, and an explicit notice step says why a green
check did nothing rather than leaving a reader to guess.

Claude-Session: https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX

* ci: the coverage guard could not fire before the ceiling above it, and today it hung for 24 minutes

`go build, vet, test` on #357 sat 24 minutes in "internal/api coverage
measures the tests, not the preflight" against a step measured at 98-114s
across the last five runs on main, until the job's own timeout-minutes: 25
cancelled it and named nothing.

TWO GAPS, AND THE SECOND IS BROKEN ARITHMETIC WITH OR WITHOUT A HANG. The
probes carried no -timeout, so Go's default of TEN MINUTES PER PROBE applied
to three of them: 30 minutes of worst case behind a job ceiling of 25. The
guard could never have reported; it could only ever have been cancelled. This
is the error the acceptance job already documents about its watchdog, where
the arithmetic "quietly assumes the suite starts when the job does".

And the step was the one exception to a discipline this file states out loud
at :340 -- "THE FOUR SCRIPT STEPS IN THIS JOB CARRY STEP TIMEOUTS ... a job
timeout names nothing, a step timeout names the step". Four got one. The
fifth is the one that hung.

WHY GO'S TIMEOUT CANNOT BE THE BACKSTOP HERE, though it is still worth having.
The guard captures each probe with `out="$(go test ...)"`, and command
substitution blocks until the pipe has no writers left rather than until go
test exits. A test leaking a child that holds inherited stdout hangs the
capture after the binary is gone and after Go's timeout has already fired --
the mechanism the note at :360 already gives for the watchdog, one step below
the step this happened to.

So: -timeout 4m per probe (~3x the slowest real one, 12m worst case) and
timeout-minutes: 14 on the step, above that 12 so Go still wins the race and
prints the goroutine dump naming the test, and below the job's 25 either way.

Verified: passes in 103s locally, matching CI's 98-114s. Mutating the flag to
1ms makes the first probe fail with `panic: test timed out after 1ms` naming
TestLedgerPreflight, so the bound is wired to the probes rather than decorative.

Claude-Session: https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX
@sonarqubecloud

Copy link
Copy Markdown

@rainmanjam
rainmanjam merged commit f62fb74 into main Aug 14, 2026
28 checks passed
@rainmanjam
rainmanjam deleted the docs/coverage-census branch August 14, 2026 21:54
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.

2 participants