Skip to content

Make slop gate the verdict, not just carry a label - #461

Closed
not-stbenjam wants to merge 1 commit into
stbenjam:mainfrom
not-stbenjam:slop-gates-the-verdict
Closed

Make slop gate the verdict, not just carry a label#461
not-stbenjam wants to merge 1 commit into
stbenjam:mainfrom
not-stbenjam:slop-gates-the-verdict

Conversation

@not-stbenjam

Copy link
Copy Markdown
Collaborator

Follow-up to #460. This commit was pushed about a minute after #460 was
squash-merged, so it missed the merge.

The gap

#460 gave the Slopinator a BLOCKING severity, but blocking in name only.
Three things on main today let a slop finding ship anyway:

  1. SKILL.md still says "Keep the panel advisory. It does not gate merge."
    with no exception for slop.
  2. APPROVE is "Set when no unresolved BLOCKING findings remain" — nothing
    prevents the arbiter from resolving a slop finding by filtering it as a
    style nit in the noise-filter step.
  3. Part A residue inherits the prose clustering rule ("prefer clustering to
    isolated hits"), so a single review-history comment can read as below the
    reporting threshold.

Changes

  • Carve slop out of the advisory stance. The panel stays advisory on
    everything else; review-history residue and generated prose must not merge.
    An unresolved Slopinator BLOCKING rules out APPROVE on its own and holds
    the verdict at REQUEST_CHANGES.
  • Add an arbiter bias against softening slop. It is not filtered as a style
    nit in Step 3, and not downgraded because the rest of the change is good.
  • Block on every Part A instance. Residue is binary — a comment either
    describes the shipped code or the conversation that produced it — so the
    clustering rule now explicitly governs Part B prose only. One review-history
    comment is enough.
  • Split the reporting-volume guidance out of "What NOT to flag", which the
    added sentence had pushed past the 500-token content-section-length limit.

The precision guardrails are unchanged: "What NOT to flag" still protects long
"why" comments, house style, domain vocabulary, version-scoped documents, and
comments a reviewer explicitly asked for.

Scope note

The panel posts its verdict with gh pr comment, so REQUEST_CHANGES is a
disposition in a comment rather than a GitHub review state — this makes slop
gate the verdict, not the merge button. A hard gate would mean posting a real
review via gh pr review --request-changes plus branch protection, which is a
larger change to Step 6 and is deliberately not in this PR.

Verification

  • make test — 2965 passed, unchanged from main
  • make lint — clean
  • .venv/bin/skillsaw lint . — 0 errors, 0 warnings, grade A+, 6 info-level
    violations (the main baseline; the regenerated report card shows no drift)
  • All three skill copies byte-identical

🤖 Generated with Claude Code

Slop was blocking in name only: the panel-wide rule said it does not
gate merge, so a BLOCKING finding could still land under an APPROVE.

- Carve slop out of the panel's advisory stance. An unresolved
  Slopinator BLOCKING rules out APPROVE on its own.
- Add an arbiter bias against softening slop, so it is not filtered as
  noise or downgraded because the rest of the change is good.
- Report every Part A instance. Residue is binary, so the clustering
  rule that governs prose tells does not apply to it; one
  review-history comment is enough to block.
@not-stbenjam
not-stbenjam requested a review from stbenjam as a code owner July 26, 2026 21:50
@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@not-stbenjam, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 30 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 0e096ed1-45fb-4b80-b9d7-dd860dfcbd58

📥 Commits

Reviewing files that changed from the base of the PR and between 43654b2 and 2d52e78.

⛔ Files ignored due to path filters (5)
  • .agents/skills/skillsaw-review-panel/SKILL.md is excluded by !.agents/**
  • .agents/skills/skillsaw-review-panel/references/slopinator.md is excluded by !.agents/**
  • .claude/skills/skillsaw-review-panel/SKILL.md is excluded by !.claude/**
  • .claude/skills/skillsaw-review-panel/references/slopinator.md is excluded by !.claude/**
  • .skillsaw-card.svg is excluded by !**/*.svg
📒 Files selected for processing (2)
  • .apm/skills/skillsaw-review-panel/SKILL.md
  • .apm/skills/skillsaw-review-panel/references/slopinator.md
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@stbenjam

Copy link
Copy Markdown
Owner

Meh, the current version is fine.

@stbenjam stbenjam closed this Jul 26, 2026
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Make Slopinator BLOCKING gate the panel verdict

✨ Enhancement 📝 Documentation 🕐 10-20 Minutes

Grey Divider

AI Description

• Make Slopinator BLOCKING force REQUEST_CHANGES; panel remains advisory otherwise.
• Prevent arbiter from filtering slop as noise or downgrading it for “good” changes.
• Report every Part A residue instance; clustering guidance applies only to Part B prose.
Diagram

graph TD
P["Skillsaw panel"] --> S["Slopinator"] --> D{"Slop BLOCKING?"} --> RC["Verdict: REQUEST_CHANGES"]
D -->|"No"| O["Verdict: APPROVE / other"]
K[/"SKILL.md"/] --> P --> A["Arbiter"]
R[/"slopinator.md"/] --> S
subgraph Legend
  direction LR
  _proc["Process"] ~~~ _doc[/"Doc"/] ~~~ _dec{"Decision"}
end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Hard gate via GitHub review + branch protection
  • ➕ Enforces slop gating at the merge button (true technical gate).
  • ➕ Clear signal in GitHub UI; integrates with required reviews/status checks.
  • ➖ Larger workflow change (requires gh pr review --request-changes and protection rules).
  • ➖ More operational overhead; harder to adopt incrementally.
2. Convert slop to a status check (CI) that must pass
  • ➕ Merge-gating without relying on comment verdict conventions.
  • ➕ Works with existing branch protection and required checks.
  • ➖ Requires wiring Slopinator into CI and defining pass/fail criteria rigorously.
  • ➖ Harder to handle nuanced false-positive resolution workflows than panel findings.
3. Keep advisory but introduce explicit “cannot resolve as noise” rule only
  • ➕ Minimal behavior change; preserves arbiter flexibility for all cases.
  • ➕ Lower risk of over-blocking from edge cases.
  • ➖ Still allows slop findings to be effectively ignored in practice.
  • ➖ Does not close the gap where slop ships under an APPROVE verdict.

Recommendation: The chosen approach (documented verdict gating for Slopinator BLOCKING, plus explicit arbiter non-softening rules) is the best incremental fix: it closes the practical loopholes without expanding scope into CI/branch protection and GitHub review-state plumbing. Consider the hard gate option later if you want “cannot merge” enforcement rather than “verdict is REQUEST_CHANGES” signaling.

Files changed (7) +127 / -67

Documentation (7) +127 / -67
SKILL.mdMake slop an explicit exception to the panel’s advisory stance +13/-2

Make slop an explicit exception to the panel’s advisory stance

• Updates the panel rules to keep findings advisory except for slop. Clarifies that unresolved Slopinator 'BLOCKING' prevents 'APPROVE', forces 'REQUEST_CHANGES', and must not be softened as noise or downgraded.

.agents/skills/skillsaw-review-panel/SKILL.md

slopinator.mdTighten slop rules: always-block Part A, cluster only Part B +29/-20

Tighten slop rules: always-block Part A, cluster only Part B

• Strengthens the statement that slop does not merge and the arbiter does not downgrade it. Separates reporting-volume guidance into its own section and makes Part A residue always reported (binary), while clustering rules govern Part B prose only.

.agents/skills/skillsaw-review-panel/references/slopinator.md

SKILL.mdSync panel slop gating guidance (APM copy) +13/-2

Sync panel slop gating guidance (APM copy)

• Mirrors the SKILL.md updates that make slop the sole non-advisory exception, block 'APPROVE' on unresolved Slopinator 'BLOCKING', and instruct the arbiter not to soften slop.

.apm/skills/skillsaw-review-panel/SKILL.md

slopinator.mdSync Slopinator rules (APM copy) +29/-20

Sync Slopinator rules (APM copy)

• Mirrors the Slopinator reference updates: slop holds the verdict at 'REQUEST_CHANGES', Part A residue blocks per-instance, and clustering applies to Part B only.

.apm/skills/skillsaw-review-panel/references/slopinator.md

SKILL.mdSync panel slop gating guidance (Claude copy) +13/-2

Sync panel slop gating guidance (Claude copy)

• Mirrors the SKILL.md updates clarifying that slop is not advisory and cannot be filtered as noise or downgraded; unresolved slop blocks 'APPROVE' and yields 'REQUEST_CHANGES'.

.claude/skills/skillsaw-review-panel/SKILL.md

slopinator.mdSync Slopinator rules (Claude copy) +29/-20

Sync Slopinator rules (Claude copy)

• Mirrors the Slopinator reference changes: explicit non-softening stance, reporting-volume section, and severity calibration where Part A always blocks and clustering is Part B-only.

.claude/skills/skillsaw-review-panel/references/slopinator.md

.skillsaw-card.svgRegenerate Skillsaw report card token count +1/-1

Regenerate Skillsaw report card token count

• Updates the rendered report card to reflect the new content token total after documentation changes.

.skillsaw-card.svg

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 140 rules
✅ Skills: 5 invoked
  skillsaw-pr-review
  skillsaw-issue-solver
  skillsaw-pr-followup
  skillsaw-create-plugin
  skillsaw-review-panel

Grey Divider


Remediation recommended

1. Wrong step number 🐞 Bug ≡ Correctness
Description
The arbiter bias says slop findings must not be “filtered as noise in Step 3”, but Step 3 is
dispatching specialists; the noise-filtering step is in Step 5 during arbiter synthesis. This
incorrect reference reduces instruction clarity and can misdirect where the slop exception is
enforced.
Code

.agents/skills/skillsaw-review-panel/SKILL.md[R224-226]

+- Do not soften slop. A Slopinator `BLOCKING` finding is not a style nit to be
+  filtered as noise in Step 3, and it is not downgraded because the rest of the
+  change is good.
Relevance

⭐⭐⭐ High

Team has accepted prior SKILL.md instruction-clarity/correctness fixes; wrong step reference is a
straightforward doc correction.

PR-#394
PR-#376

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
In the panel workflow, Step 3 is explicitly “Dispatch Specialists”, while noise filtering is
explicitly performed in Step 5 (“Run Panel Arbiter Synthesis”), item 3. The new arbiter bias line
introduced by this PR points to “Step 3” for noise filtering, which is therefore incorrect; the same
incorrect line exists in all mirrored skill copies.

.agents/skills/skillsaw-review-panel/SKILL.md[99-104]
.agents/skills/skillsaw-review-panel/SKILL.md[186-193]
.agents/skills/skillsaw-review-panel/SKILL.md[224-226]
.apm/skills/skillsaw-review-panel/SKILL.md[224-226]
.claude/skills/skillsaw-review-panel/SKILL.md[224-226]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The slop exception guidance incorrectly references “Step 3” as the noise-filtering step, but Step 3 is the specialist dispatch step. Noise filtering is performed in Step 5 during arbiter synthesis, so the step reference should be corrected (or rewritten to avoid step-number coupling).

## Issue Context
This text is mirrored across the `.agents/`, `.apm/`, and `.claude/` copies of `skillsaw-review-panel/SKILL.md`, so the correction should be applied consistently in all three.

## Fix Focus Areas
- .agents/skills/skillsaw-review-panel/SKILL.md[217-226]
- .apm/skills/skillsaw-review-panel/SKILL.md[217-226]
- .claude/skills/skillsaw-review-panel/SKILL.md[217-226]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment on lines +224 to +226
- Do not soften slop. A Slopinator `BLOCKING` finding is not a style nit to be
filtered as noise in Step 3, and it is not downgraded because the rest of the
change is good.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

1. Wrong step number 🐞 Bug ≡ Correctness

The arbiter bias says slop findings must not be “filtered as noise in Step 3”, but Step 3 is
dispatching specialists; the noise-filtering step is in Step 5 during arbiter synthesis. This
incorrect reference reduces instruction clarity and can misdirect where the slop exception is
enforced.
Agent Prompt
## Issue description
The slop exception guidance incorrectly references “Step 3” as the noise-filtering step, but Step 3 is the specialist dispatch step. Noise filtering is performed in Step 5 during arbiter synthesis, so the step reference should be corrected (or rewritten to avoid step-number coupling).

## Issue Context
This text is mirrored across the `.agents/`, `.apm/`, and `.claude/` copies of `skillsaw-review-panel/SKILL.md`, so the correction should be applied consistently in all three.

## Fix Focus Areas
- .agents/skills/skillsaw-review-panel/SKILL.md[217-226]
- .apm/skills/skillsaw-review-panel/SKILL.md[217-226]
- .claude/skills/skillsaw-review-panel/SKILL.md[217-226]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@codecov

codecov Bot commented Jul 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.54%. Comparing base (43654b2) to head (2d52e78).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #461   +/-   ##
=======================================
  Coverage   93.54%   93.54%           
=======================================
  Files         145      145           
  Lines       11418    11418           
=======================================
  Hits        10681    10681           
  Misses        737      737           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2d52e788bf

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +206 to +207
This is the disposition for unresolved slop, which is always in scope and
always fixable: the finding quotes the replacement text.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve rejection precedence when slop is also present

When a PR has both unresolved slop and an Ecosystem or out-of-domain rejection, this new unconditional REQUEST_CHANGES rule conflicts with the following REJECT — REDIRECT TO PLUGIN and REJECT criteria. The arbiter may therefore hide a terminal rejection behind a request to fix prose, requiring another review cycle before reporting the actual disposition. Define the precedence explicitly so rejection outcomes win, while slop still rules out APPROVE in all other cases.

Useful? React with 👍 / 👎.

Comment on lines +14 to +16
`REQUEST_CHANGES` until they are fixed. What keeps that from being tyranny is
accuracy, not timidity: "What NOT to flag" below draws the line, and inside it
there is nothing to soften.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Replace the passage that triggers its own slop blocker

Under this reference's newly strengthened Part B rubric, the sentence beginning “What keeps that from being tyranny” clusters manufactured antithesis and dramatic metaphors in shipped skill prose, so a compliant Slopinator must classify this commit itself as BLOCKING. Replace it with the direct wording Read "What NOT to flag" before filing anything; its precision guardrails still apply.

Useful? React with 👍 / 👎.

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