Skip to content

perf(discovery): check restatement, flag and tier-mix rules at write time and scale verification to evidence use - #12553

Closed
melodic-automation-lanes[bot] wants to merge 5 commits into
mainfrom
work-resolve-issue/issue-12517-41540452222-1
Closed

melodic-automation-lanes[bot] wants to merge 5 commits into
mainfrom
work-resolve-issue/issue-12517-41540452222-1

Conversation

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

Refs: #12517

Summary

Opened by the work-resolve-issue lane. The run's own summary:

Research verification looped on format and calibration rules a script can check. The restatement check now also fails on sweep words (every, all, none, only) on index lines that cite a claim ID, and it fails closed. A new check-claim-eligibility.py requires claim_kind and source_owner on every claim and checks single-source flag eligibility (row 4) and the tier mix behind HIGH (row 7). The researcher runs both until they exit 0, and the parent re-runs them as acceptance-gate step 5. Under Evidence use: internal the verifier grades rows 4, 7 and 12 only for the claims a stated decision cites (a new Claims: line), with no rework round; publish keeps the full loop. A slice-root synthesis that adds no claims takes no verifier. Design choices: (1) The sweep-word rule reads only the RESEARCH.md index, because sidecar bodies are where a claim is stated in full and a fail-closed rule there would force rewording real claims. (2) Row 4 eligibility is decided by comparing the primary source's pool with source_owner, which separates a first-party page from a third-party report. (3) The HIGH mix rule is the discipline file's own definition: 3+ independent Tier 0/1 sources, counted as distinct current pools. (4) A claim-free synthesis records the existing verification: skipped (cost) value, so the verification: vocabulary is unchanged. (5) The new script is invoked through python3, because this lane cannot set its executable bit.

Remaining work

  • Comment item 1: intent-trace sources tagged author_kind owner-quote | agent-relay at write time, with a script that rejects a Direct claim whose only source is an agent relay
  • Comment item 2: the fan-out envelope names the .research-dispatch baseline path, so researchers stop reporting a missing sub-slice baseline
  • Comment item 4: research and explore record each dispatch's duration, tokens and verifier verdict to lane telemetry
  • Issue Verification: re-run one topic with these fixes and compare verify share and pass rate with the issue's table

Not verified

  • The test suites (check-restatement-labels.test.sh, check-claim-eligibility.test.sh, contract.test.sh) were not executed: this lane runs only the named wrappers, so the cases were traced by reading instead. Left to the PR's CI.
  • check-claim-eligibility.py and its .test.sh are new files with mode 100644. This lane cannot chmod, so every doc invokes the script as python3 "<path>" and the allow rules match that form. A maintainer may set +x and move it to the direct-path form its siblings use.
  • Tidy was skipped: the change touches 17 files, over the 10-file tidy cutoff.
  • The issue's Verification step was not run: re-running one deep-tier topic and comparing verify share and pass rate with the table needs a host research run.

Fix

  • plugins/discovery/scripts/check-claim-eligibility.py: new script; requires claim_kind and source_owner, checks HIGH (single source) eligibility, the HIGH tier mix and the subject_pool cap; exits 0/1/2 like its siblings
  • plugins/discovery/scripts/check-claim-eligibility.test.sh: new black-box test with pass and fail cases for rows 4 and 7, the two declared fields, and the ungradeable paths
  • plugins/discovery/scripts/check-restatement-labels.py: flags any declared claim ID beside a sweep word on an index line; no longer advisory; label rule and output format unchanged
  • plugins/discovery/scripts/check-restatement-labels.test.sh: sweep-word cases (a failing sweep line, a passing labeled pointer, a plain-HIGH ID, two IDs on one line, word-boundary, no-ID and sidecar-body cases)
  • plugins/discovery/scripts/contract.test.sh: section 25 asserts each new rule; retires the advisory and Restatements: assertions; fixes the Rows: line regex
  • plugins/discovery/skills/research/context/artifact-shape.md: handoff lines are ID pointers with labels only, plus the sweep-word rule; fail-closed checks; claim_kind and source_owner in the claim header and their rules
  • plugins/discovery/skills/research/context/dispatch.md: 'The restatement and eligibility checks' replaces the advisory section; the verifier brief scales to evidence use with a Claims: line; the claim-free synthesis exemption
  • plugins/discovery/skills/research/SKILL.md: gate step 5 runs both checks; the verifier block carries Claims:; no rework round under internal; publish keeps one; the synthesis exception
  • plugins/discovery/skills/research/context/discipline.md: Confidence calibration names the eligibility check
  • plugins/discovery/skills/research/context/deep-tier.md: the engine path records claim_kind and source_owner and runs gate step 5; the synthesis exception
  • plugins/discovery/skills/research/evals/evals.json: case 52 (internal-tier verification of decision claims, no rework), case 53 (claim-free synthesis skips the verifier); case 45 now states Evidence use: publish so its one-rework expectation still holds
  • plugins/discovery/agents/researcher.md: both checks must exit 0 before hand-back; a non-zero exit means fix and re-run; declares claim_kind and source_owner
  • plugins/discovery/agents/research-verifier.md: an optional Claims: field replaces Restatements:; a claims_graded return field; row 4 and row 12 text reflect the pre-run scripts
  • plugins/discovery/reference/parent-contract.md: no rework under evidence_use: internal; allow rules for the eligibility checker
  • plugins/discovery/skills/setup/SKILL.md: ten gate allow rules, adding the eligibility checker
  • plugins/discovery/skills/setup/evals/evals.json: allow-rule eval expects the five gate scripts
  • .changes/discovery/work-resolve-issue-issue-12517-41540452222-1-f2a7a1a8.md: minor changelog fragment

Verification

The runner ran each check from the base copy against f2fb283:

  • scripts/check-changelog-fragments.sh --check: passed
  • scripts/check-changed-skills.sh --merge-base: passed

The run reports:

  • check-changelog-fragments: all 95 fragments valid
  • check-changed-skills: research and setup PASS, 0 errors (warnings were already present: description length, body size, fresh-eyes hints, artifact-shape TOC)
  • check-affected: 13 passed, 5 skipped (typos not installed; checks that run code from the checked tree), over 17 changed files
  • check-evals-quality on research and setup evals.json: PASS; its 3 warnings are on cases this change did not touch (ids 11 and 17 in research, id 5 in setup)
  • cant-fail-scan --file plugins/discovery/scripts/check-claim-eligibility.test.sh --check: PASS, no gating findings
  • fresh-context review subagent over the whole diff: found 3 problems, all confirmed and fixed (a stale Rows: regex at contract.test.sh:1235, eval 45 relying on the internal default for its rework expectation, and the synthesis exemption missing from SKILL.md:42 and deep-tier.md:173). By reading, it t
  • The lane run: https://github.com/melodic-software/claude-code-plugins/actions/runs/41540452222

Left out: the items under Not verified.

Related

#12517

Lane-Activity: work-resolve-issue/resolve-issue
@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed eb9f6d35c91bee884bd7bbb0ffa716315bc64a75: read 1 of 1 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #12553, incremental (91f491bc → eb9f6d35)

I found no security issues in this change.

What I read. I read the whole change in the one file listed in .claude-lane/incremental-files.txt: plugins/discovery/skills/research/SKILL.md, two hunks in .claude-lane/incremental.diff. It doesn't match any trusted-path glob. No plugin manifest, workflow, action, token, kill-switch or merge-hold file changed, so no manifest <field> lines apply.

What changed. The push only rewords two sentences in the research skill:

  • It drops a parenthetical about how the verifier grades under Evidence use: internal. The parenthetical said row 14 is graded in full, and that an absent or empty Claims: line means every accepted claim is graded.
  • It changes "On the cost path (you skip the verifier for cost)" to "Skipping the verifier for cost". The verification: skipped (cost) value it tells the parent to write stays the same.

Removed guidance. This push removes standing guidance, so I judged it by what breaks without it. I did not open the instruction exception register in this run. The removed text covers research-quality grading only: which verifier rows apply and what an empty claims list means. It doesn't govern tool use, credentials, destructive or outward-facing actions, trust decisions, or how untrusted input is handled. Its absence gives no request a path to a protected action. It is not a security finding. Whether row 14 or the empty-line rule is still stated elsewhere (for example in dispatch.md) is a correctness question for the code-review lane.

Spend. This push changes nothing about cost. It only rewords guidance about when the verifier runs.

Prior findings. This lane's only earlier post was a summary with no findings. Rounds with findings before this review: 0, against a cap of 3, so the cap does not apply.

PR context. It contains no text aimed at this lane.

critical-findings: 0

@melodic-automation-lanes

melodic-automation-lanes Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor Author

Unattended merge: native auto-merge leaves this pull request unarmed at head eb9f6d35c91bee884bd7bbb0ffa716315bc64a75. The lanes clear this reason on their own; the next scheduled run judges it again.

pr-merge's gate refuses its verifier verdict (verify:earlier-fail)

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-review: 1 inline finding(s) on f2fb28359a94500da29bb2fe9108f786a7164515; the summary is in this lane's pull request comment.

claude-lane: claude-review

@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed eb9f6d35c91bee884bd7bbb0ffa716315bc64a75: read 1 of 1 changed file(s); 1 finding(s), 0 as inline comments.

Code review: PR #12553, incremental (91f491b → eb9f6d3)

Scope. One file changed since my last review: plugins/discovery/skills/research/SKILL.md. I read its whole change, two hunks in one paragraph pair (L66 and L68). I had no shell, so I ran nothing.

What changed. The "Fix failing CI checks" commit shortens two sentences in the verifier section:

  • L66. It drops the parenthetical "(row 14 in full; an absent or empty line grades every accepted claim)".
  • L68. It rewords "On the cost path (you skip the verifier for cost)" to "Skipping the verifier for cost".

Checked, no new findings:

  • Nothing is lost from the parenthetical. Both points still hold in the files this paragraph points to:
    • dispatch.md#L100 says "Row 14 is still graded in full. An absent or empty Claims: line grades every accepted claim."
    • research-verifier.md#L38-L40 says the same.
    • The verifier, which is the reader that acts on this, gets the rule from its own agent file.
  • The contract assertions still match.
    • The SKILL.md anchor at contract.test.sh#L1380 only matches up to "a stated decision cites", and the new L66 still has that prefix.
    • The skipped (cost) assertion at L974 still matches L68.
    • No assertion in plugins/discovery greps for the removed phrases "On the cost path" or "row 14 in full".

Prior findings.

Not verified. The author reports that the test suites (check-restatement-labels.test.sh, check-claim-eligibility.test.sh, contract.test.sh) were not run, and I could not run them either. That this commit fixes the failing CI check is the commit message's claim, not something I confirmed.

Closing issues. None. The PR only references #12517.

Manifests. No plugin.json or marketplace.json changes in this increment.

Lane-Activity: pr-unblock/address-feedback
Lane-Thread: PRRT_kwDOTCGFQM6y6TXB
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

pr-unblock / address-feedback finished round 1 of max 4 on b160744.

Lane-Activity: pr-unblock/address-feedback
Lane-Thread: PRRT_kwDOTCGFQM6y7MOx
Lane-Thread: PRRT_kwDOTCGFQM6y7MPZ
Lane-Thread: PRRT_kwDOTCGFQM6y7MQe
Lane-Thread: PRRT_kwDOTCGFQM6y7MRC
Lane-Thread: PRRT_kwDOTCGFQM6y7MRw
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

pr-unblock / address-feedback finished round 2 of max 4 on 91f491b.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-review: 1 inline finding(s) on 91f491bcc0f94f7783bc9947c03ecb98d551e4e1; the summary is in this lane's pull request comment.

claude-lane: claude-review

Comment on lines +277 to +281
Any other non-zero exit sends the synthesis back for rewriting, like any failure above. A root that
adds one claim, a cross-topic conclusion included, takes the verifier on every verifier-owned row
too, and takes the same `--sub-slice` run as its step 5 (the plain run when its sub-slices share
an ID), so its own claims and its pointers to sub-slice claims are both checked before the
verifier is dispatched.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A root that adds a claim and whose sub-slices share an ID: nothing checks its pointers, and the verifier isn't told.

This sentence falls back to the plain restatement run "when its sub-slices share an ID". That run reads only the root's own sidecars, so the root's pointers to sub-slice claims (dropped HIGH (single source) labels, sweep words) go unchecked by script. Only the claim-free branch (L274-276) tells the parent to say so in the brief.

agents/research-verifier.md L121-126 now tells the verifier the parent's --sub-slice run "covered the root's pointers to sub-slice claims as well, unless two sub-slices declare one ID: then the brief says so". On this path the parent was never told to write that note. The verifier reads no note, assumes the script covered the pointers, and checks no pointer itself.

How it fails:

  1. Two topic sub-slices each declare C1. IDs are unique only within one slice (artifact-shape.md:135), so independent topic researchers can easily collide.
  2. The root adds a cross-topic conclusion.
  3. A Settled line reads C3 holds for every version.
  4. The plain run passes, since the root's sidecar declares no C3.
  5. The full-row verifier, briefed without the note, skips the pointer.

Fix: extend the brief rule to this case. For example:

…(the plain run when its sub-slices share an ID, and then the brief says the restatement check could not grade its pointers to sub-slice claims), so…

The same gap applies when the root's own claim ID collides with a sub-slice ID: --sub-slice exits 2, and this paragraph doesn't say what that root does.

claude-lane: claude-review

@melodic-automation-lanes melodic-automation-lanes Bot added the needs-session A pr-unblock stop a session builder takes: a change the lane cannot make. Not an owner stop. label Nov 3, 2026
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

The pr-unblock lane stopped address-feedback on this pull request: the pull request has an unresolved review thread the lane does not take (a reviewer it does not act on, a review of an older head, or a thread it already answered), so no lane resolves it and a session builder takes it. The lane labels it needs-session for a session builder, who makes the change this lane cannot, and the run ends in a failing pr-unblock/address-feedback status on the head the lane gated.

When a retry on this head can change the outcome, the lane runs one on its own, up to its per-head cap, and edits this comment if it stops again; a run that does not stop clears the status. When no retry can (a review thread the lane does not take or settle, or a failing check after the retries are spent), a person resolves the threads or gets the checks green, then dispatches the workflow below: that run finds the cause gone and clears the status. On a pull request the lanes App opened, the dispatchers start that run once per stop; on any other, a person does. A new commit to the branch starts clean.

To run it again on this head now, dispatch the pr-unblock workflow:

gh workflow run pr-unblock.yml -R melodic-software/claude-code-plugins -f pr-number=12553 -f activity=address-feedback

@kyle-sexton

Copy link
Copy Markdown
Member

Hold: replaced by #12603 (owner-login replacement after pr-unblock escalated). Releases when: #12603 merges, then this PR closes.

@kyle-sexton

Copy link
Copy Markdown
Member

Superseded by #12603, which merged with this branch's changes and the review fixes.

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

Labels

do-not-merge Hard merge gate: do not merge while applied. needs-session A pr-unblock stop a session builder takes: a change the lane cannot make. Not an owner stop.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant