Skip to content

Say Which Reviewers a Repository Has, and Split the Reviewer Doc in Two - #1409

Merged
ptr727 merged 11 commits into
developfrom
feature/reviewer-availability-at-the-gate
Sep 7, 2026
Merged

Say Which Reviewers a Repository Has, and Split the Reviewer Doc in Two#1409
ptr727 merged 11 commits into
developfrom
feature/reviewer-availability-at-the-gate

Conversation

@ptr727

@ptr727 ptr727 commented Sep 7, 2026

Copy link
Copy Markdown
Owner

The maintainer has explained this fleet's reviewer-availability rules to an agent at least three
times. The facts were never missing: docs/pr-reviewer-evaluation.md already stated them. That
file is hub-only and nothing routed to it from the surface an agent reads while driving a review,
so it was never reached at the moment it was needed.

The routing is the fix. The doc split is what makes the routing safe.

The section

pr-review-conduct gains "Which Reviewers a Repository Actually Has". It states no star
threshold, no plan tier, and no product policy
, because a carrying repository can verify none of
those. It tells the reader to respond to what the reviewers actually posted on the pull request in
front of them:

  • a skip notice means the reviewer is available for the asking, with the trigger comment named
    and its poster named
  • a notice naming when the reviewer can next run is a rate limit, which asking does not clear,
    and which looks like a skip notice while behaving oppositely
  • silence is not evidence, since a reviewer can review with an empty body and no threads, which
    Merge Gate item 2 already calls an ordinary shape
  • Copilot's absence is the Merge Gate's business and the loop's re-request step, not this
    section's

The split

docs/pr-reviewer-reference.md is new and hub-only: coverage per repository, each reviewer's
behavior, the committed file that shapes it, and plan scope. docs/pr-reviewer-evaluation.md keeps
the evaluation, a living argument, and loses the operational half it had accumulated.

The point is where a change lands. A reviewer's behavior now changes in one hub-only file, with
no carried copy, no promotion, and no resync.

Four per-reviewer Skills were considered and rejected on that same test. spec/files.json carries
.github/skills with appliesTo: "*", so there is no such thing as a hub-only Skill: that shape
would have pushed the most volatile facts in the fleet into ~22 repositories least able to check
them, as 12 files and ~88 carried copies, each a new canonical unit owing its own whole-text review
passes. Net surface here is one hub-only doc and no new carried file.

One finding stands

Six review passes ran over the new section, recorded in reports/canonical-review.json.

"Copilot's absence is neither of those" admits two antecedents where three bullets now precede
it. The rate-limit bullet introduced it in the last edit. Left for this pull request's own review
round rather than a seventh local pass, since the section is otherwise clean and the round count is
itself a signal.

Earlier rounds are worth naming, because they are what the passes bought:

  • an unqualified gh repo view --json visibility that, run from a hub checkout, reported PUBLIC
    for a PRIVATE repository, which is exactly the failure the section exists to prevent. Verified
    against the real tool, not the help text.
  • a frontmatter description grown to 1734 characters, past the roughly 1535 where the rendered
    listing truncates, silently cutting a routing sentence the file already had. Now 1457.
  • a claim that comparing pr_review.py's threads= against unresolved= detects an unrecognized
    reviewer identity. False: threads= counts resolved and human threads too, so they differ on
    nearly every pull request.
  • "a reviewer that posted nothing is not covering this repository", which is the same
    infer-from-absence mistake this section exists to correct, pointed the other way.

Verification

build_dist.py --check current, spec/validate.py OK, prose_lint.py clean, repo_gate.py
clean on all three checks, markdownlint clean on every changed Markdown file. Both
local_review.py and canonical_review.py passes recorded, and the pre-push hook accepted the
push.

Disclosure: the last push was a --force-with-lease after a git commit --amend.
GOVERNANCE.md "Git and Commit Rules" says "never force push", unqualified, and this branch is no
exception to it. Nothing was lost, the branch is mine and the replaced commit was my own from the
same session, but the correct move was a second commit rather than an amend, and it is recorded
here rather than left in the reflog.

🤖 Generated with Claude Code

Copilot AI lite review requested due to automatic review settings September 7, 2026 03:36
@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: cf079d8e-95a2-4d8e-a1ac-d3b7260bd201


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.

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

🟡 Changes recommended

The new reviewer-availability section still leaves key dispositions ambiguous (notably missing Copilot, trigger-and-wait behavior, and drift vs transient skip/config causes), which can misdirect the review loop at the exact gate this change is meant to clarify.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates the pr-review-conduct skill contract to explicitly document how to determine which automated reviewers should be expected for a given repository (private vs public) and what to do when a reviewer appears missing or skipped.

Changes:

  • Add a new “Which Reviewers a Repository Actually Has” section to pr-review-conduct, including a gh repo view ... --json visibility check and guidance for private/public repos.
  • Extend the skill description so the skill triggers when a reviewer looks missing or skipped.
  • Refresh generated skill distributions and canonical review bookkeeping to reflect the new section.
File summaries
File Description
reports/canonical-review.json Records canonical-review entries for the new pr-review-conduct units.
.agents/skills/pr-review-conduct/SKILL.md Source-of-truth skill content; adds the new reviewer-availability section and updates the description.
.github/skills/pr-review-conduct/SKILL.md Generated distribution copy updated to match .agents/.
.claude-plugin/fleet-skills/skills/pr-review-conduct/SKILL.md Generated distribution copy updated to match .agents/.
.claude-plugin/fleet-skills/.source-digests/pr-review-conduct Updates the source digest for the generated fleet-skill distribution.
Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .agents/skills/pr-review-conduct/SKILL.md Outdated
Comment thread .agents/skills/pr-review-conduct/SKILL.md Outdated
Comment thread .agents/skills/pr-review-conduct/SKILL.md Outdated
The facts were already written down, in `docs/pr-reviewer-evaluation.md`. That
file is hub-only and nothing routed to it from the surface an agent reads while
driving a review, so it was never reached at the moment it was needed and the
maintainer restated it instead, three times.

Two changes, and the split is what makes the first one safe.

`pr-review-conduct` gains "Which Reviewers a Repository Actually Has", which
states no star threshold, no plan tier, and no product policy, because a
carrying repository can verify none of those. It tells the reader to respond to
what the reviewers actually posted on the pull request in front of them: a skip
notice means available for the asking, a notice naming when the reviewer can
next run is a rate limit that asking does not clear, silence is not evidence
since a reviewer can review with an empty body, and Copilot's absence is the
Merge Gate's business rather than this section's.

`docs/pr-reviewer-reference.md` is new and hub-only, and holds what the section
deliberately does not: coverage per repository, each reviewer's behavior, the
committed file that shapes each one, and plan scope.
`docs/pr-reviewer-evaluation.md` keeps the evaluation, which is a living
argument, and loses the operational half it had accumulated. A reviewer's
behavior now changes in one hub-only file, with no carried copy and no resync.

Four per-reviewer Skills were considered and rejected. The skills tree carries
to every repository, so that shape would push the most volatile facts in the
fleet into the repositories least able to check them, and updating one would
need a promotion and a resync rather than an edit.

Six review passes ran over the new section. One finding stands, recorded in
`reports/canonical-review.json`: "Copilot's absence is neither of those" admits
two antecedents where three bullets now precede it, which the rate-limit bullet
introduced. Left for the pull request's own review round rather than a seventh
pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 7, 2026 13:52
@ptr727
ptr727 force-pushed the feature/reviewer-availability-at-the-gate branch from 3d5d04a to 00147fd Compare September 7, 2026 13:52
@ptr727 ptr727 changed the title Say Which Reviewers a Repository Actually Has, at the Gate That Asks Say Which Reviewers a Repository Has, and Split the Reviewer Doc in Two Sep 7, 2026

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

🟡 Changes recommended

The new reviewer-availability guidance and reviewer-reference intro contain a couple of documentation/loop-disposition gaps that can misroute agents (notably Copilot quota-limit handling and “product moved” overstatement).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 8/8 changed files
  • Comments generated: 3
  • Review effort level: Lite

Comment thread .agents/skills/pr-review-conduct/SKILL.md Outdated
Comment thread .agents/skills/pr-review-conduct/SKILL.md Outdated
Comment thread docs/pr-reviewer-reference.md Outdated
Copilot raised six threads. Three were against the head before the doc split and
named what that restructure had already fixed, which is corroboration rather
than work. Three were current and are answered here.

- **The account-quota refusal had no home on any surface.** It is a documented
  shape, `scripts/pr_review.py` giving it exit 46 and a `refusal=QUOTA` digest
  field from text observed on #962, and no rule said what to do about it. The
  Copilot bullet now says it covers no head, so the gate stays unsatisfied, and
  nothing the loop does clears it, since the refusal names no time to wait for
  and re-requesting returns it again. It goes to the maintainer rather than into
  a wait with no stated end.
- **Two sentences blamed product state for every mismatch.** A skipped review
  also comes from a rate limit, or from a repository's own committed
  configuration, a `.coderabbit.yaml` base-branch allowlist being the case that
  a carrying repository can check for itself. The reference doc's intro and the
  section's opening sentence each name both causes now.
- **A closing clause read as instructing an edit to hub-owned content** from a
  repository carrying the file, which is a write to another repository with no
  seat or route named. Deleted.

One finding was declined with evidence. Naming a hub-qualified path from carried
guidance is not the failure #1403 describes, which is an unqualified path a
carrier reads as its own. Measured on this fleet rather than argued: the prose
gate's dead-path check fired on three bare `repo-config/...` paths in a
downstream carry and did not fire on this file's existing hub-qualified `docs/`
mention, in the same run over the same file.

The section is recorded clean in `reports/canonical-review.json`, and the
"neither of those" demonstrative the previous commit left open is gone with the
bullet that introduced it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 7, 2026 14:06

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

🟢 Approval recommended

Only a minor README grammar issue was found, and the functional/documentation changes are otherwise consistent and internally coherent.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

README.md:155

  • Grammar: the subject here is compound ("how each reviewer behaves" + "which repositories it covers"), so it should not take a singular verb ("is"). Rephrase to use a plural verb or restructure the sentence.
  • Files reviewed: 8/8 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

…op bullet

Copilot raised it as a suppressed finding on #1409, which opens no thread, so it
is answered in the pull request conversation rather than on one.

"how each reviewer behaves and which repositories it covers is the reviewer
reference" joins two clauses into one subject and then takes `is`. Restructured
so the reference is the subject and records both, which drops the compound
rather than pluralizing the verb around it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 7, 2026 14:27
@ptr727

ptr727 commented Sep 7, 2026

Copy link
Copy Markdown
Owner Author

Answering the suppressed finding, which opens no thread to reply on.

README.md:155 Grammar: the subject here is compound ("how each reviewer behaves" + "which repositories it covers"), so it should not take a singular verb ("is"). Rephrase to use a plural verb or restructure the sentence.

Accepted and fixed in 2a9eceb. Restructured rather than pluralized, so the reference is the subject and records both clauses, which removes the compound instead of agreeing with it:

and the [reviewer reference][pr-reviewer-reference] records how each reviewer behaves and which repositories it covers.

The six inline threads from earlier rounds are all answered and resolved. Three of them were against the head before the doc split and named what that restructure had already fixed, and the three current ones produced the quota-refusal case, the two monocausal claims, and the deleted clause that read as instructing an edit to hub-owned content from a carrier.

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

🔵 Needs a closer look

docs/pr-reviewer-reference.md violates the repo’s “one logical paragraph per line” Markdown convention, and should be reflowed before merging.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

docs/pr-reviewer-reference.md:7

  • Repo doc style requires one logical paragraph per line (no hard-wrap). This intro paragraph is hard-wrapped across multiple lines, which makes future edits and diffs noisier. Please reflow this (and the rest of the document) so each paragraph is on a single line.
  • Files reviewed: 8/8 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot raised it as a suppressed finding on #1409, which opens no thread, so it
is answered in the pull request conversation rather than on one.

The new reference doc was internally inconsistent: the paragraphs moved into it
from the evaluation were already one line each, and the paragraphs written for it
were hard-wrapped at around 100 characters. Every other file under `docs/` keeps
one logical paragraph per line, longest lines running to several hundred
characters, so the wrapped paragraphs were the deviation rather than the
convention.

Whitespace only. Verified mechanically rather than by reading: with runs of
whitespace collapsed, the text of both files is identical to what it was before,
so no word, sentence or link moved. Headings, list items, tables, fenced blocks
and link definitions were left exactly as they were.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 7, 2026 14:39
@ptr727

ptr727 commented Sep 7, 2026

Copy link
Copy Markdown
Owner Author

Answering the suppressed finding on head 2a9eceb, which opens no thread to reply on.

docs/pr-reviewer-reference.md:7 Repo doc style requires one logical paragraph per line (no hard-wrap). This intro paragraph is hard-wrapped across multiple lines, which makes future edits and diffs noisier. Please reflow this (and the rest of the document) so each paragraph is on a single line.

Accepted and fixed in 7dfdb5d. The finding also names the real cause, which was an inconsistency inside one file: the paragraphs moved into the new reference from the evaluation doc were already one line each, and only the paragraphs written for it were wrapped. Confirmed against the convention rather than taken on trust, since docs/ and .agents/skills/ differ here: every other file under docs/ runs one paragraph per line, longest lines from 321 to 2773 characters, so the wrapped paragraphs were the deviation.

Whitespace only, and verified mechanically rather than by reading. With runs of whitespace collapsed, the text of both files is byte-identical to what it was before the reflow, so no word, sentence or link moved. Headings, list items, tables, fenced blocks and link definitions were not touched.

The earlier suppressed finding, the README compound subject, was fixed in 2a9eceb and answered above.

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

🟡 Changes recommended

There are documentation correctness/formatting issues (broken numbered list rendering and a misleading link target) that should be fixed before merging.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

docs/pr-reviewer-reference.md:31

  • This sentence uses link text ".coderabbit.yaml" but points it at the auto-review documentation reference, which is confusing because it reads like a link to the configuration reference. Link the file mention to the configuration reference, and keep the auto-review docs link as the supporting documentation.
  • Files reviewed: 8/8 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread docs/pr-reviewer-evaluation.md Outdated
The reflow in 7dfdb5d joined every wrapped prose line in a paragraph, and its
skip list covered headings, blanks, `-`/`*` bullets, tables, quotes, fences and
link definitions, but not ordered-list markers. So "Next Evaluation Steps"
became a single line, which renders as one list item carrying all six.

The check that missed it is the lesson. Collapsing runs of whitespace proved no
word had changed, which was true and not sufficient, since it cannot see a line
break that was itself structure. Verified this time by counting the structural
elements: 6 ordered-list items before and 6 after, with headings, bullets,
tables, fences and link definitions unchanged.

markdownlint did not catch it either, since one list item holding six sentences
is valid Markdown.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 7, 2026 14:45
@ptr727

ptr727 commented Sep 7, 2026

Copy link
Copy Markdown
Owner Author

The collapsed-list finding is fixed in 31de110, and its thread had already been resolved by the time my reply reached it, so the reasoning is recorded here instead.

The "Next Evaluation Steps" numbered list was collapsed into a single line, which breaks Markdown list rendering (it becomes one list item)

Correct. The reflow in 7dfdb5d joined wrapped prose lines, skipping headings, blanks, -/* bullets, tables, quotes, fences and link definitions. Ordered-list markers were not in that skip list, so all six items became one.

The check that missed it is the part worth keeping. I verified that reflow by collapsing runs of whitespace and confirming the text was identical. That was true, and it proved only that no word changed. It cannot see a line break that is itself structure, which is exactly what an ordered list is, so the one defect the change could introduce was the one the check was blind to.

Verified this time by counting structural elements instead: 6 ordered-list items before and 6 after, with headings, bullets, tables, fences and link definitions unchanged. markdownlint passed both revisions, since one list item holding six sentences is valid Markdown, so nothing mechanical would have caught it either.

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

🟢 Approval recommended

The routing/doc split and the new reviewer-availability section are internally consistent, properly linked, and introduce no verified defects.

Review details
  • Files reviewed: 8/8 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

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

🟢 Approval recommended

Only a minor punctuation fix is needed and the rest of the changes are clean.

Review details
  • Files reviewed: 8/8 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread docs/pr-reviewer-reference.md Outdated
Copilot's finding on #1409. The intro quotes the question the document answers,
"what will this reviewer do here", without the mark that makes it read as one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 7, 2026 15:04

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

🟢 Approval recommended

The changes are documentation-only and low risk; the only findings are minor punctuation/grammar nits in the new reference doc.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

docs/pr-reviewer-reference.md:17

  • Add a comma after the introductory date phrase ("As of September 2026,") for correct grammar and readability.
  • Files reviewed: 8/8 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread docs/pr-reviewer-reference.md Outdated
Round 7 asked for the question mark the quotation lacked. Round 8 called the
resulting `?",` nonstandard and typo-like. Both are fair and they are mutually
exclusive, so the quoted-question construction is what fails rather than either
punctuation of it.

The sentence now states the distinction directly, which needs no quotation and
carries the same meaning.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 7, 2026 15:08

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

🔵 Needs a closer look

The new operational reviewer reference links repository config filenames to external vendor docs in a misleading way, which reduces the document's usefulness as an at-a-glance reference.

Review details

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

docs/pr-reviewer-reference.md:31

  • The link target for the .coderabbit.yaml filename points at external docs (coderabbit-auto-review) rather than the committed config file, which is misleading in an operational reference. Keep the external docs link, but don’t attach it to the file name.

This issue also appears in the following locations of the same file:

  • line 51
  • line 59

docs/pr-reviewer-reference.md:52

  • The file names .coderabbit.yaml and .pr_agent.toml are linked to vendor documentation (coderabbit-config / qodo-config) rather than the repository’s committed config files, which makes the links read like they open the file but they don’t. Consider leaving the file names as code and linking the docs separately.
- **CodeRabbit** reads `reviews.path_filters` from [`.coderabbit.yaml`][coderabbit-config] at the repository root, where a pattern prefixed with `!` excludes.
- **Qodo** reads [`.pr_agent.toml`][qodo-config] from the root of the default branch, so the file binds only once it is promoted to `main`, and its [`[ignore]` glob list][qodo-ignore] names the paths to skip.

docs/pr-reviewer-reference.md:60

  • In the reviewer configuration bullets, the .coderabbit.yaml / .pr_agent.toml filenames are linked to external docs, which is easy to misread as a link to the repo file itself. Keeping the docs links but not attaching them to the filename makes the reference less confusing.
- **CodeRabbit**, in [`.coderabbit.yaml`][coderabbit-config]: auto review on pull requests into `develop`, which the open-source tier honors only at ten stars or more, no pause after five reviewed commits, since a fleet pull request routinely passes five pushes and the pause reads as a reviewer that stopped. A path instruction for Markdown asks for false, stale, unverifiable, or unfollowable claims only, since CI lints style and the local review pass reads canonical prose whole. Sequence diagrams, suggested labels and reviewers, and the in-progress fortune are off. The markdownlint, actionlint, shellcheck, and ruff tools are off, since CI runs the same four and fails the pull request on them.
- **Qodo**, in [`.pr_agent.toml`][qodo-config]: an issues guideline asks for a reproduction with any claimed crash, after a claimed `IsADirectoryError` on this repository's build was disproven by running it. A compliance guideline asks for the rule's own sentence and routes a rule against unchanged text to the summary. Informational findings go to the [summary][qodo-verbosity] rather than a thread, since a thread blocks the merge until resolved. Images are off so a finding's title is plain text a matcher can see. Qodo's [review standards][qodo-rules] import from `AGENTS.md`, `CLAUDE.md`, `copilot-instructions.md`, and `SKILL.md` files, each scoped to its folder at any depth, when changes merge, and only new rules are added, so an edited or deleted rule is changed in its portal instead.
  • Files reviewed: 8/8 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Three suppressed findings on #1409, all the same shape. A filename rendered as
`[`.coderabbit.yaml`][coderabbit-config]` reads as a link to the committed file
and opens vendor documentation instead, which is worse in an operational
reference than in the evaluation this text came from, since a reader here is
looking for what their own repository holds.

The filenames are now plain code, and each vendor link moved onto the thing it
actually documents: the setting, the settings reference, or the configuration
reference. Every link definition is still used and still defined.

The text predates this branch, which moved it out of `pr-reviewer-evaluation.md`
unchanged. Fixed rather than filed, since this change is what created the file
it now sits in.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 7, 2026 15:16
@ptr727

ptr727 commented Sep 7, 2026

Copy link
Copy Markdown
Owner Author

Answering the three suppressed findings on head e0296f1, which open no threads. They are one finding at three sites, and it is right.

The link target for the .coderabbit.yaml filename points at external docs rather than the committed config file, which is misleading in an operational reference. Keep the external docs link, but don't attach it to the file name.

Fixed in be6c210, at all five occurrences rather than the three quoted, since the same shape appears twice more in the same file.

The filenames are now plain code, and each vendor link moved onto the thing it actually documents:

Was Now
[.coderabbit.yaml][coderabbit-auto-review] lists the base under reviews.auto_review.base_branches`` .coderabbit.yaml lists the base under [reviews.auto_review.base_branches][coderabbit-auto-review]
reads reviews.path_filters from [.coderabbit.yaml][coderabbit-config] reads [reviews.path_filters][coderabbit-config] from .coderabbit.yaml
reads [.pr_agent.toml][qodo-config] from the root reads [its configuration][qodo-config] from .pr_agent.toml at the root
**CodeRabbit**, in [.coderabbit.yaml][coderabbit-config]: CodeRabbit, in .coderabbit.yaml, whose [settings reference][coderabbit-config] names each option:
**Qodo**, in [.pr_agent.toml][qodo-config]: Qodo, in .pr_agent.toml, whose [configuration reference][qodo-config] names each option:

Every link definition is still used and still defined, checked mechanically rather than by eye.

Worth noting for the record: this text predates the branch, which moved it out of pr-reviewer-evaluation.md unchanged, so it is pre-existing by class and would ordinarily be filed rather than fixed. Fixed here because this change is what created the file it now sits in, and because the finding lands harder in an operational reference than in the evaluation it came from: a reader here is looking for what their own repository holds.

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

🔵 Needs a closer look

The new docs and skill preamble include concrete wording issues (including an incorrect claim about Copilot content exclusion availability) that should be corrected before merge.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

.agents/skills/pr-review-conduct/SKILL.md:17

  • The YAML description sentence is grammatically non-parallel ("deferred" vs "a deliberate decline") and repeats "Also triggers" back-to-back, which makes the trigger conditions harder to parse. Recast this into one parallel clause so the description reads cleanly and unambiguously in every generated copy.
    docs/pr-reviewer-reference.md:53
  • This sentence asserts Copilot content exclusion is unavailable for user-owned repositories, but GitHub now documents repository-level content exclusions (with plan/surface-dependent enforcement). Please reword to avoid an incorrect blanket claim, and instead state that availability/enforcement varies and should be confirmed for this repository.
  • Files reviewed: 8/8 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

…ute plan claim

Both raised on #1409 as suppressed findings, which open no thread.

The skill description carried two "Also triggers" sentences back to back, the
second one added by this branch. Folded into the first, so one sentence now
carries three parallel "when" clauses. The field measures 1447 characters
against the roughly 1535 where a rendered listing truncates.

The reference doc asserted that Copilot content exclusion "is unavailable here"
because the repository sits under a user account. That is a product-policy
absolute the document cannot verify, and the reviewer reports GitHub now
documents repository-level exclusions with plan-dependent enforcement. Reworded
to say availability and enforcement vary and to confirm the setting on the
repository, keeping what was actually observed as an observation.

The second is text this branch moved out of `pr-reviewer-evaluation.md` rather
than wrote, which is the same effect a downstream carry has: moving prose in
front of a reviewer that had not read it is what surfaces what it says.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 7, 2026 15:28
@ptr727

ptr727 commented Sep 7, 2026

Copy link
Copy Markdown
Owner Author

Answering the two suppressed findings on head be6c210, both fixed in 47ee43f.

.agents/skills/pr-review-conduct/SKILL.md:17 The YAML description sentence is grammatically non-parallel ("deferred" vs "a deliberate decline") and repeats "Also triggers" back-to-back.

Accepted. The second "Also triggers" was added by this branch, so the doubling is mine. Folded into the preceding sentence, which now carries three parallel when clauses. The field measures 1447 characters against the roughly 1535 where a rendered listing truncates, so it still fits with margin.

docs/pr-reviewer-reference.md:53 This sentence asserts Copilot content exclusion is unavailable for user-owned repositories, but GitHub now documents repository-level content exclusions (with plan/surface-dependent enforcement).

Accepted, and it is the same class this whole change exists to remove: an absolute claim about external product state that the document cannot verify. Reworded to say availability and enforcement vary and to confirm the setting on the repository, keeping what was actually observed as an observation rather than as a rule.

Worth recording, since it is the third time today: that sentence is text this branch moved out of pr-reviewer-evaluation.md rather than wrote. Moving prose in front of a reviewer that had not read it is what surfaced it, which is exactly the effect #1138 describes for a downstream carry, happening here inside the hub. The findings in the last several rounds have mostly been of that kind rather than about the rule this change adds.

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

🟡 Changes recommended

The updated pr-review-conduct frontmatter trigger sentence is grammatically inconsistent, reducing clarity in a high-signal routing surface.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 8/8 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread .agents/skills/pr-review-conduct/SKILL.md
Copilot's follow-up on #1409. The previous fix folded a doubled "Also triggers"
sentence into one, and joined the folded clause with "and when" where the two
clauses before it read "when ... or when ...". So the fold removed the doubling
and broke the parallelism in the same edit.

One word: "and when" becomes "or when".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 7, 2026 15:32

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

🔵 Needs a closer look

A few new documentation phrases are ambiguous/misleading (notably around what is "skipped" and how to choose the correct reviewer trigger), and should be clarified before merge.

Review details

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

.agents/skills/pr-review-conduct/SKILL.md:17

  • The frontmatter description says "when a reviewer looks missing or skipped", but a reviewer is not what gets skipped; the review is. Rewording avoids implying the reviewer itself can be skipped and makes the trigger condition clearer.

This issue also appears on line 95 of the same file.
docs/pr-reviewer-reference.md:19

  • This paragraph says ".agents/skills/pr-review-conduct/SKILL.md ... is carried into repositories", but downstream repositories carry the generated .github/skills/ tree, not the .agents/ source path. Referring to the carried skill (or .github/skills/...) avoids sending a downstream reader looking for a path they will not have.

.agents/skills/pr-review-conduct/SKILL.md:98

  • The skip-notice bullet mixes CodeRabbit and Qodo triggers in one sentence, which can be read as "post both" or as not needing to match the trigger to the reviewer that actually posted the notice. Make the trigger conditional on which reviewer posted the skip notice.
- **A reviewer that posted a skip notice is available for the asking.** It says it did not review
  automatically, which is not the same as not reviewing at all. Comment `@coderabbitai review`, or
  Qodo's `/review`, and wait for the result as with any other requested review. The agent driving
  the loop posts that comment itself, on the same standing as requesting a review after a push.
  • Files reviewed: 8/8 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@ptr727

ptr727 commented Sep 7, 2026

Copy link
Copy Markdown
Owner Author

Answering the three suppressed findings on head 834c12a, which open no threads. All three are accepted and filed rather than fixed, as #1416, on the maintainer's call to merge and file at this round.

The first is a real error and worth naming here rather than only in the issue:

This paragraph says .agents/skills/pr-review-conduct/SKILL.md ... is carried into repositories, but downstream repositories carry the generated .github/skills/ tree, not the .agents/ source path.

Correct. spec/files.json carries .github/skills with appliesTo: "*", and .agents/skills/ is hub-only, so that sentence sends a downstream reader after a path they do not have. It is the same shape as #1403, which this branch's sibling fixed today. It is filed rather than held for because docs/pr-reviewer-reference.md is itself hub-only, so the wrong path misleads a hub reader instead of propagating to every carrying repository.

The other two are wording: a reviewer described as skipped where the review is what gets skipped, and a skip-notice bullet naming both triggers in one sentence without tying the trigger to the reviewer that actually posted the notice.

On stopping here. Twelve rounds. The last several raised findings against text this change moved rather than wrote, and two raised findings against the previous round's own fix, once on punctuation and once on the parallel structure of a clause the prior round had asked to be recast. The rule this change adds has been stable since round nine.

@ptr727
ptr727 merged commit 23dd810 into develop Sep 7, 2026
9 checks passed
@ptr727
ptr727 deleted the feature/reviewer-availability-at-the-gate branch September 7, 2026 15:39
ptr727 added a commit that referenced this pull request Sep 7, 2026
…carried path fix (#1422)

Promote `develop` to `main`. Four commits, no code behavior change
outside the prose gate's own
reporting and its tests.

- **#1417** Name the Prose Gate's Own Copy on Every Verdict. A
`dead-path` violation CI reported
could not be reproduced locally, because the gate read a different copy
of the file than the one
the operator had in hand and never said which. Every verdict now names
the copy it read.
- **#1409** Say Which Reviewers a Repository Has, and Split the Reviewer
Doc in Two. Reviewer
availability turns on repository visibility and star count, and the one
doc that stated it mixed
the evaluation with the operating reference. Split, with the
availability rule stated where a
  driver reads it.
- **#1414** Replace Three Hub-Only Pointers in a Carried Skill Reference
With the Behavior. The
carried reference named `repo-config` paths that exist only in the hub,
so a downstream prose gate
  failed the moment the file landed in a diff there.
- **#1410** Bump `streetsidesoftware/cspell-action` from 9.0.1 to 9.1.0.

Closes #1412
Closes #1403

🤖 Generated with [Claude Code](https://claude.com/claude-code)


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **New Features**
- Prose-check findings now identify the specific gate version that
produced them, improving traceability across repositories and revisions.
- Invalid empty exclusion values are rejected instead of being treated
as a full-tree scan.

- **Documentation**
- Added guidance for reproducing prose-check findings and diagnosing
outdated pinned versions.
- Expanded reviewer documentation covering availability, skip behavior,
limits, and repository scope.
  - Updated review-loop references and release workflow guidance.

- **Maintenance**
  - Updated the spell-checking tool version used in validation.


<!-- end of auto-generated comment: release notes by coderabbit.ai -->
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