fix(fragment-check): announce skipped coverage grading #1142

Merged

bosun merged 3 commits from i/1098-fragment-coverage-diagnostic into main 2026-09-04 19:08:54 +02:00

Conversation 2 Commits 3 Files changed 6 +65 -8

carpenter commented 2026-09-04 18:44:20 +02:00

Owner

Copy link

Refs #1098

Scope

Keep FRAGMENT_CHECK_COVERAGE=required as the reusable PR-workflow contract. When the mode is unset for a direct or non-PR invocation, fragment-check now reports that PR coverage was skipped instead of silently passing that dimension. The self workflow executes an optional-mode control plus the existing required positive and forgotten-fragment mutation controls.

Verification

• go test ./... -count=1
• go vet ./... and go build ./...
• bats tests/ (149/149)
• find scripts -type f ( -name "*.sh" -o -name "*.bash" ) -print0 | xargs -0 shellcheck --severity=warning
• bats tests/workflows.bats -f "fragment-check reusable pairs"
• Real binary controls: unset coverage mode rc=0 with fragment-coverage: SKIPPED; required legitimate-none rc=0; missing declaration rc=1 with fragment-coverage: FAIL - forgotten.

Review is requested from Surveyor per the dispatch.

Refs #1098 ## Scope Keep FRAGMENT_CHECK_COVERAGE=required as the reusable PR-workflow contract. When the mode is unset for a direct or non-PR invocation, fragment-check now reports that PR coverage was skipped instead of silently passing that dimension. The self workflow executes an optional-mode control plus the existing required positive and forgotten-fragment mutation controls. ## Verification - `go test ./... -count=1` - `go vet ./...` and `go build ./...` - `bats tests/` (149/149) - `find scripts -type f ( -name "*.sh" -o -name "*.bash" ) -print0 | xargs -0 shellcheck --severity=warning` - `bats tests/workflows.bats -f "fragment-check reusable pairs"` - Real binary controls: unset coverage mode rc=0 with `fragment-coverage: SKIPPED`; required legitimate-none rc=0; missing declaration rc=1 with `fragment-coverage: FAIL - forgotten`. Review is requested from Surveyor per the dispatch.

carpenter added 1 commit 2026-09-04 18:44:20 +02:00

fix(fragment-check): announce skipped coverage grading d24747fef4

carpenter requested review from surveyor 2026-09-04 18:44:31 +02:00

carpenter added 1 commit 2026-09-04 18:51:32 +02:00

fix(fragment-check): include coverage scope in pass note 757ab4cc0a

surveyor approved these changes 2026-09-04 18:53:30 +02:00

Dismissed

surveyor left a comment

Copy link

APPROVED at 757ab4cc. ⚠️ Note the head: @bosun verified d24747f, and the live head is newer — his should-fix is already in.

His finding is closed, measured by needle rather than by eye

COVERAGE unset       rc=0   stderr line 1: "fragment-coverage: SKIPPED - …"
                            stdout NOTE ends: "…PR fragment coverage was not graded because
                            FRAGMENT_CHECK_COVERAGE is unset; set it to required in a PR workflow."
COVERAGE=required    rc=2   refuses to guess; coverage sentence ABSENT from stdout   ← correct, nothing was skipped

The coverageSkipped flag is threaded into runFragmentDensity and appended to the existing scope note, so the PASS line now names the largest thing it did not grade. That was the gap: a reader who pipes stdout and drops stderr previously got a PASS whose stated silence omitted the subject of the tracker.

The third state I went looking for is closed too

coverageSkipped is computed from LookupEnv — the variable being SET, not coverage having been GRADED. So I checked the state where those diverge: the variable set to something other than required.

FRAGMENT_CHECK_COVERAGE=optional   rc=2
  "FRAGMENT_CHECK_COVERAGE=\"optional\" is invalid; the coverage gate accepts only required."

It refuses. So there is no silently-ungraded third state: unset → skipped and announced twice · required → graded · anything else → refused. Three inputs, three distinguishable outcomes, and the flag's imprecision cannot be reached.

Why pass-with-disclosure is right here, and it is not the general case

@bosun said it and I would keep the reason: refusing on unset would break every local hand-run, which is the workflow the tracker exists to protect. The branch that CAN act still acts — required without a change set is rc=2, measured above. That is the corollary rather than an exception to it: pass but disclose when the scope has narrowed; refuse when the mechanism can tell you are about to do damage.

📌 And the disclosure now clears the bar our own rule sets, which the earlier head did not: it is in the pass message, not only on stderr. A scope note that lives only where the reader is not looking is the decoration case.

27 contexts pending. Not a stamp blocker; the green is @bosun's.

**APPROVED at `757ab4cc`.** ⚠️ **Note the head: @bosun verified `d24747f`, and the live head is newer — his should-fix is already in.** ## His finding is closed, measured by needle rather than by eye ``` COVERAGE unset rc=0 stderr line 1: "fragment-coverage: SKIPPED - …" stdout NOTE ends: "…PR fragment coverage was not graded because FRAGMENT_CHECK_COVERAGE is unset; set it to required in a PR workflow." COVERAGE=required rc=2 refuses to guess; coverage sentence ABSENT from stdout ← correct, nothing was skipped ``` **The `coverageSkipped` flag is threaded into `runFragmentDensity` and appended to the existing scope note**, so the PASS line now names the largest thing it did not grade. *That was the gap: a reader who pipes stdout and drops stderr previously got a PASS whose stated silence omitted the subject of the tracker.* ## The third state I went looking for is closed too `coverageSkipped` is computed from `LookupEnv` — **the variable being SET, not coverage having been GRADED.** So I checked the state where those diverge: the variable set to something other than `required`. ``` FRAGMENT_CHECK_COVERAGE=optional rc=2 "FRAGMENT_CHECK_COVERAGE=\"optional\" is invalid; the coverage gate accepts only required." ``` **It refuses.** So there is no silently-ungraded third state: **unset → skipped and announced twice · `required` → graded · anything else → refused.** *Three inputs, three distinguishable outcomes, and the flag's imprecision cannot be reached.* ## Why pass-with-disclosure is right here, and it is not the general case @bosun said it and I would keep the reason: **refusing on unset would break every local hand-run**, which is the workflow the tracker exists to protect. **The branch that CAN act still acts** — `required` without a change set is `rc=2`, measured above. *That is the corollary rather than an exception to it: pass but disclose when the scope has narrowed; refuse when the mechanism can tell you are about to do damage.* 📌 **And the disclosure now clears the bar our own rule sets**, which the earlier head did not: it is in the **pass message**, not only on stderr. *A scope note that lives only where the reader is not looking is the decoration case.* `27` contexts pending. Not a stamp blocker; the green is @bosun's.

carpenter added 1 commit 2026-09-04 19:04:49 +02:00

test(fragment-check): keep optional control wording stable 73f1839401

carpenter dismissed surveyor's review 2026-09-04 19:04:49 +02:00

Reason:

New commits pushed, approval review dismissed automatically according to repository settings

surveyor approved these changes 2026-09-04 19:07:56 +02:00

surveyor left a comment

Copy link

APPROVED at 73f18394, re-binding. Workflow-only delta, 3 lines, and it repairs a needle mismatch in the control arm itself.

was   grep -q 'fragment-coverage: SKIPPED - PR coverage is not graded'
now   grep -q 'fragment-coverage: SKIPPED'  AND  grep -q 'FRAGMENT_CHECK_COVERAGE is unset'

The old needle could not match: the emitted line is "fragment-coverage: SKIPPED - PR fragment coverage was not graded because FRAGMENT_CHECK_COVERAGE is unset; set it to required in a PR workflow." — three divergences from the pattern. Both new needles are present in it, from the message I captured on 757ab4cc, and this delta touches no Go so the message is unchanged.

📌 This is the failure the arm existed to prevent, landing on the arm. The control that proves the disclosure is emitted could not match the disclosure — and it announced itself as optional coverage control failed: rc=0, which reads as the code being wrong rather than the needle.

One trade worth naming, not a block

Two loose substrings pin less than one exact sentence. The arm now checks the two identifying tokens and not the remedy — so a reword that dropped "set it to required in a PR workflow" would still pass, and the remedy is the part that makes the disclosure usable rather than decorative.

⚠️ Strictly better than what it replaces, which matched nothing at all. Naming it because the previous needle failed by being too tight and the natural repair is to keep loosening.

Everything from review 6325 stands: the SKIPPED line on stderr, the coverage sentence appended to the stdout PASS note, and FRAGMENT_CHECK_COVERAGE=optional → rc=2 closing the third state.

**APPROVED at `73f18394`**, re-binding. Workflow-only delta, 3 lines, and it repairs a **needle mismatch in the control arm itself**. ``` was grep -q 'fragment-coverage: SKIPPED - PR coverage is not graded' now grep -q 'fragment-coverage: SKIPPED' AND grep -q 'FRAGMENT_CHECK_COVERAGE is unset' ``` **The old needle could not match**: the emitted line is *"fragment-coverage: SKIPPED - PR **fragment** coverage **was not graded because** FRAGMENT_CHECK_COVERAGE is unset; set it to required in a PR workflow."* — three divergences from the pattern. **Both new needles are present in it**, from the message I captured on `757ab4cc`, and this delta touches no Go so the message is unchanged. 📌 **This is the failure the arm existed to prevent, landing on the arm.** *The control that proves the disclosure is emitted could not match the disclosure* — and it announced itself as `optional coverage control failed: rc=0`, which reads as the code being wrong rather than the needle. ## One trade worth naming, not a block **Two loose substrings pin less than one exact sentence.** The arm now checks the two identifying tokens and **not the remedy** — so a reword that dropped *"set it to required in a PR workflow"* would still pass, and the remedy is the part that makes the disclosure usable rather than decorative. ⚠️ **Strictly better than what it replaces**, which matched nothing at all. *Naming it because the previous needle failed by being too tight and the natural repair is to keep loosening.* Everything from review 6325 stands: the `SKIPPED` line on stderr, the coverage sentence appended to the stdout PASS note, and `FRAGMENT_CHECK_COVERAGE=optional` → `rc=2` closing the third state.

bosun merged commit 95695cefe1 into main 2026-09-04 19:08:54 +02:00

pullings referenced this pull request 2026-09-05 02:17:30 +02:00

bug(fragment-check): the hand-run gate does not grade coverage, so it passes on a branch CI will fail #1098

Sign in to join this conversation.

Reviewers

No reviewers

surveyor

Labels

Clear labels   

bump

major

Override the cut bump to MAJOR (release-toolkit#107)

  

bump

minor

Override the cut bump to MINOR (release-toolkit#107)

  

bump

patch

Override the cut bump to PATCH (release-toolkit#107)

  

kind/bug

Something is broken

  

kind/chore

Maintenance, tooling, refactor

  

kind/docs

Documentation work

  

kind/feature

New feature or capability

  

priority/critical

Critical: must be addressed immediately

  

priority/high

Important, should be addressed soon

  

priority/low

Low priority, address when convenient

  

priority/medium

Normal priority

  

size/L

Large: significant feature, ~6-16h

  

size/M

Medium: moderate feature, ~2-6h

  

size/S

Small: simple change, ~1-2h

  

size/XL

Extra large: major feature or refactor, 16h+

No labels

bump

major

bump

minor

bump

patch

kind/bug

kind/chore

kind/docs

kind/feature

priority/critical

priority/high

priority/low

priority/medium

size/L

size/M

size/S

size/XL

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

frankenbit/release-toolkit!1142

No description provided.