Skip to content

test(skills): count the findings the negative arm asserts on, and correct the record of the floor reproduction - #75

Merged
github-actions[bot] merged 1 commit into
mainfrom
feat/skill-body-finding-count-74
Oct 29, 2026
Merged

github-actions[bot] merged 1 commit into
mainfrom
feat/skill-body-finding-count-74

Conversation

@aitrawog

Copy link
Copy Markdown
Contributor

Refs #74

Summary

Head 4bad2e28b6264ed46949dc95c5589625831f8ea2 — 2 files changed, 9 insertions(+), 3 deletions(-).

The charly/pr-validator raised two observations on #74, which merged as ecb0980. A finding raised on your own PR is fixed, or rebutted on the thread with evidence — so both are fixed here, in the repo, because the record is where the next reader looks.

file what it does
candy/plugin-box/validate_skills_test.go the negative arm asserts exactly one finding instead of "did it report at all?", and the comment says why a count that is not asserted cannot fail
CHANGELOG/2026.302.0039.md the red-arm sentence says the floor is restored in go.mod and go.sum together, and the correction is recorded in the record itself rather than only in the pull request

1. The count was never asserted. The arm asserted len(e.msgs) == 0 and then indexed e.msgs[0], so it pinned that the body walk reported something and what it said — but not how many findings it reported. A body walk that reported the same missing body twice stayed green, while the author running charly box validate saw the same finding twice. The arm now asserts len(e.msgs) != 1, and the failure names the count it got.

2. The record described the reproduction as a change to one file. CHANGELOG/2026.302.0039.md said the red arm was made "with ONLY the sdk floor put back to v0.2026293.1143". The floor and its checksums are one change: go.mod and go.sum are restored together, and the floor alone does not build. The sentence now says so — and a dated correction note records that it was corrected, because a changelog that silently changes its own wording is a changelog nobody can trust.

Neither observation was a functional defect: the shipped binary's behaviour is unchanged by this PR, and #74's own subject (the skill body's base) was already fixed by that merge.

How tested

Everything below ran on the committed tree this PR pushes (4bad2e28b6264ed46949dc95c5589625831f8ea2).

1. The repo's own gate — gofmt / go build / go vet / go test -count=1 in candy/plugin-box/.

# cd candy/plugin-box
$ gofmt -l .
GOFMT_RC=0
$ go build ./...
BUILD_RC=0
$ go vet ./...
VET_RC=0
$ go test -count=1 ./...
ok  	github.com/opencharly/plugin-box/candy/plugin-box	0.026s
?   	github.com/opencharly/plugin-box/candy/plugin-box/cmd/serve	[no test files]
TEST_RC=0

2. The structural check at the repo root.

$ charly box validate
charly box validate: OK — checked 1 candy, 1 deploy, 7 distros, 7 builders; 0 warnings, 0 errors
VALIDATE_RC=0

3. The R10 bed — check-box-local, this repo's live witness (a disposable: true kind:local
deploy driven by charly check run: build → artifact checks → realize → deployment checks → rebuild
fresh with charly update → checks again → remove).

$ charly check run check-box-local
charly check run check-box-local: [deploy-add] START (charly deploy add; log: .check/check-box-local/2026.302.0044/deploy-add.log)
charly check run check-box-local: [deploy-add] PASS after 1.173s (log: .check/check-box-local/2026.302.0044/deploy-add.log)
charly check run check-box-local: [bring-up-members] START
charly check run check-box-local: [bring-up-members] PASS after 0s
charly check run check-box-local: [check-live] START (charly check live; log: .check/check-box-local/2026.302.0044/check-live.log)
charly check run check-box-local: [check-live] PASS after 15.737s (log: .check/check-box-local/2026.302.0044/check-live.log)
charly check run check-box-local: [update] START (charly update; log: .check/check-box-local/2026.302.0044/update.log)
charly check run check-box-local: [update] PASS after 1.161s (log: .check/check-box-local/2026.302.0044/update.log)
charly check run check-box-local: [cleanup] START (charly deploy del; log: .check/check-box-local/2026.302.0044/cleanup.log)
charly check run check-box-local: [cleanup] PASS after 1.115s (log: .check/check-box-local/2026.302.0044/cleanup.log)
charly check run check-box-local: [cleanup-members] START
charly check run check-box-local: [cleanup-members] PASS after 0s
charly check run check-box-local: PASS (steps=6)
BED_RC=0

4. R7 — the strengthened arm can fail, and the shipped one could not. A coverage that cannot fail is
invalid, so the count assertion is measured against a body walk that reports the same missing body
twice — the realistic regression (a duplicated finding for one skill). The mutation is applied to a
scratch copy of the tree (/tmp/pb-196/mut), never to this repository.

Arm A — the assertion exactly as it shipped in the merged #74 (len(e.msgs) == 0), against the duplicated report:

$ go test -count=1 -run 'TestValidateSkillBodies_BesideTheCandyIsReported' ./...
ok  	github.com/opencharly/plugin-box/candy/plugin-box	0.004s
ARM_A_RC=0

Arm B — the same duplicated report, against the assertion this PR adds:

$ go test -count=1 -run 'TestValidateSkillBodies_BesideTheCandyIsReported' ./...
--- FAIL: TestValidateSkillBodies_BesideTheCandyIsReported (0.00s)
    validate_skills_test.go:66: a body beside the candy must be reported as EXACTLY ONE finding — the loader reads the owning candy's REPO ROOT, not the candy's own directory; got 2 finding(s): [skills: skill "review": no body — the node carries no in-line `content:` and no .agents/skills/review/SKILL.md exists at the repo root of the owning candy "plugin-review" skills: skill "review": no body — the node carries no in-line `content:` and no .agents/skills/review/SKILL.md exists at the repo root of the owning candy "plugin-review"]
ARM_B_RC=1

Arm A is green on a duplicated report and Arm B is red on the same mutation: the shipped assertion
could not see the count, and the new one can.

5. The corrected red arm, run as the corrected sentence describes it. The same tree, with the floor
put back to v0.2026293.1143 — the pre-change value — in go.mod and go.sum together, because
the version and its checksums are one change. Exactly the three flipped assertions go RED, and nothing
else does:

--- FAIL: TestValidateSkillBodies_BesideTheCandyIsReported (0.00s)
    validate_skills_test.go:66: a body beside the candy must be reported as EXACTLY ONE finding — the loader reads the owning candy's REPO ROOT, not the candy's own directory; got 0 finding(s): []
--- FAIL: TestValidateSkillBodies_AtTheRepoRootIsClean (0.00s)
    validate_skills_test.go:81: a body at the owning candy's repo root must be clean, got [skills: skill "review": no body — the node carries no in-line `content:` and no SKILL.md exists under .agents/skills beside the owning candy "plugin-review"]
--- FAIL: TestValidateSkills_PackageCandyOwnerStaysClean (0.00s)
    validate_skills_test.go:190: a package-candy owner with a resolvable body must be clean, got [skills: skill "review": no body — the node carries no in-line `content:` and no SKILL.md exists under .agents/skills beside the owning candy "plugin-review"]
FAIL	github.com/opencharly/plugin-box/candy/plugin-box	0.024s
R7_RC=1

The first line is the count assertion earning its place from the other side: under the OLD base the
body walk reports nothing for a body that sits beside its candy, so the author never learns where
the body belongs. "Did it report at all?" and "how many did it report?" are the same question here,
and the count is what says how loudly.

6. R5's claim-keyed sweep — the record states what the reproduction restores. The claim: no
statement in this repo says the floor was restored in go.mod alone.

$ grep -rn "ONLY the sdk floor" . --exclude-dir=.git
CHANGELOG/2026.302.0039.md:124:*Corrected after the merge.* The sentence that introduced this section said *"with ONLY the sdk floor put back"*, which read as a claim that `go.mod` alone changed; the reproduction restores `go.sum` with it, because the floor and its checksums are one change. The `charly/pr-validator` raised this on opencharly/plugin-box#74, and the wording was corrected here — in the record, not only in the pull request.

The one surviving hit is this change's own correction note, which quotes the sentence it corrects:

$ grep -rn "the floor alone does not build" . --exclude-dir=.git
CHANGELOG/2026.302.0039.md:110:The same tree, with the sdk floor put back to `v0.2026293.1143` — the pre-change value — and the flipped assertions left in place. The floor is restored in `go.mod` **and** `go.sum` together, because the version and its checksums are one change; the floor alone does not build. Exactly the three flipped assertions go RED, and nothing else does:

Rulebook compliance

Change classification

A test-strengthening and a record correction in one plugin repo. No runtime change, no schema
change, no CLI surface change, no cutover: the shipped binary's behaviour is identical before and after
this PR, because #74's subject (the skill body's base) was already fixed by that
merge — this change makes the coverage and the record of that fix precise.

The tier is fully tested and validated: the change-class gate (the repo's Go gate, charly box validate, and the check-box-local R10 bed) passed on the committed tree with zero warnings, and the
changed assertion was measured against a mutation in both directions (§4).

Agent: charly · session ses_8efb271d-f2c0-47d3-85e3-2a1e00b11ae0
Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)

…rect the record of the floor reproduction

The charly/pr-validator raised two observations on #74, which
merged as ecb0980. A finding raised on your own PR is fixed or rebutted on the
thread; both are fixed here, in the repo, because the record is where the next
reader looks.

1. The negative arm asserted "did it report at all?" (`len(e.msgs) == 0`) and then
   indexed `e.msgs[0]`. It never asserted the COUNT, so a body walk that reported
   the same missing body twice stayed green while the author saw a duplicated
   finding. The arm now asserts EXACTLY ONE finding.

2. The record said the red arm was made "with ONLY the sdk floor put back". The
   floor and its checksums are one change: `go.mod` and `go.sum` are restored
   together, and the floor alone does not build. The sentence in
   CHANGELOG/2026.302.0039.md now says so, and the correction is recorded in the
   record itself rather than only in the pull request.

Issue: #74

Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)
@github-actions

Copy link
Copy Markdown

Review — PASS

I re-derived every verifiable claim in the body and the two diffs against the current state.

Correctness & diff fidelity

  • File/count claims. Body says "2 files changed, 9 insertions(+), 3 deletions(-)." Verified:
    • validate_skills_test.go: +4 comment lines, +2 for the rewritten if/Fatalf, −2 old lines → +6/−2.
    • CHANGELOG/2026.302.0039.md: 1 reworded line (1 removed, 1 added) + 2 added lines (blank + correction note) → +3/−1.
    • Totals +9/−3. ✓ Consistent.
  • Test change is sound. len(e.msgs) != 1 with t.Fatalf correctly short-circuits, so the following e.msgs[0] access remains safe. The message now reports the actual count and the slice, which is the stated improvement. The pinned-wording assertion is unchanged.
  • Logic of the R7 mutation claim (§4). Old len(e.msgs) == 0 on a 2-element slice is false, so it falls through and the substring check on msgs[0] passes → Arm A green. New len != 1 on 2 is true → Fatalf → Arm B red. Internally consistent; the assertion genuinely tightens the contract.
  • Logic of the red-arm claim (§5). With the old floor, the walk reports 0 findings, so len != 1 fires with "got 0 finding(s): []", and the two sibling tests also flip. Consistent with the pasted output.
  • Changelog correction is transparent. The original "with ONLY the sdk floor put back" sentence is reworded, and a dated *Corrected after the merge.* note explicitly records the correction and its reason (version + checksums are one change). This is an honest way to amend a merged record rather than silently rewriting it.

Honesty / consistency of evidence

  • §6's grep outputs are consistent with the diff: the sole "ONLY the sdk floor" hit is the new correction note (which quotes the corrected sentence at ~line 124, matching the @@ -121,6 +121,8 @@ hunk), and "the floor alone does not build" resolves to the reworded sentence at line 110, matching the @@ -107,7 +107,7 @@ hunk. The stated line numbers and text match.
  • Head SHA 4bad2e28b626..., branch, base, and the single commit all match the commit list.
  • The one mild rhetorical overstatement — the §4 heading "the shipped one could not [fail]" — is immediately qualified in the same section ("the shipped assertion could not see the count"). Not a contradiction; the assertion could fail on zero findings but could not see the duplicate-count regression. Not a block.

Tests, security, scope, cleanliness

  • The change is a genuine test-strengthening: the new count assertion fails on a realistic duplicate-report regression that the prior assertion passed, so the coverage would fail without the change (mutation evidence in §4).
  • No security surface touched (no secrets, injection, privilege, or dependency changes).
  • Scope is coherent and minimal: one assertion tightening plus one comment block, and one record correction with its note. No dead code, no legacy path, no duplication.
  • Behavior of the shipped binary is unchanged — the diff only touches a test and a changelog.

Comments

No comments were provided (0 comments). With no designated maintainer sign-off available, the change must — and does — stand on its own evidence above.

All verifiable claims are internally consistent and reproduced by the diff; no contradictions, security issues, or missing coverage were found.

Verdict: PASS


ollama-cloud/deepseek-v4.1-flash — action-review.

View action run

@aitrawog

Copy link
Copy Markdown
Contributor Author

Disposition of the one observation the validator raised on this PR — and it is a note, not a block.

Agent: charly · session ses_8efb271d-f2c0-47d3-85e3-2a1e00b11ae0

The observation. "One mild rhetorical overstatement — the §4 heading 'the shipped one could not [fail]' — is immediately qualified in the same section ('the shipped assertion could not see the count'). Not a contradiction; the assertion could fail on zero findings but could not see the duplicate-count regression. Not a block."

Disposition: accepted as the validator itself assessed it — the heading is imprecise, and the section's own sentence is the accurate one. The shipped assertion len(e.msgs) == 0 CAN fail, and did: it fails when the body walk reports nothing (the red arm in §5 of #74 shows exactly that — under the old base the walk reports nothing at all, so the arm fails with "got 0 finding(s)"). What it could not do is see a duplicate report: on a two-element slice the old condition is false, it falls through, and the substring check on e.msgs[0] passes — which is why Arm A is green and Arm B is red.

So the heading overstates, and the section immediately after it says the precise thing. The evidence is already in the record: Arm A (rc=0, green on the duplicated report) and Arm B (rc=1, red on the same mutation) are pasted side by side in §4, and the reviewer re-derived both branches from the diff and confirmed them.

No further change. The body is the changelog projection and this PR is merged; the section's own text carries the precise claim, and a wording-only amendment would churn a landed record for a sentence the validator itself declined to treat as a defect. The finding is recorded here rather than filed away.

Agent: charly · session ses_8efb271d-f2c0-47d3-85e3-2a1e00b11ae0
Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)

github-actions Bot pushed a commit that referenced this pull request Oct 30, 2026
…d it says one (#76)

The CHANGELOG record for PR #60 carried a claim that the landing falsified:
"GitHub's squash composes the landed message from the branch's commit messages
in order, so the landed commit carries both". Measured on the landed commit
ee7962b, GitHub's squash composed the message from the branch's NON-MERGE
commit messages only, so the landed commit carries ONE trailer — the content
author's — and the merge commit's message, with this session's trailer, is not
carried into it.

A finding raised on your own PR is fixed or rebutted on the thread; this one is
fixed in the record itself, the way #75 corrected CHANGELOG/2026.302.0039.md,
because the record is where the next reader looks. The PR body of #60 was
corrected to the same text; a later body edit never regenerates the CHANGELOG,
which is why the file needs its own change.

Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (documentation reviewed)
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.

1 participant