docs(agents): CodeRabbit does not auto-review this repo, and its check never resolves - #416
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 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. Comment |
There was a problem hiding this comment.
Pull request overview
Updates contributor/agent guidance to document that CodeRabbit does not auto-review this repository (and that its GitHub status context can remain pending), so reviewers don’t mistakenly wait for “no pending checks” or assume a CodeRabbit review happened without explicitly requesting it.
Changes:
- Document that CodeRabbit auto-review is not enabled for this repo and must be explicitly requested when desired.
- Clarify the operational consequence for merge gating: use
CI successplus completion of other checks rather than waiting for zero pending checks.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
… over-installed (#417) * fix(ci): the apt bounds were calibrated against a claim, and the step over-installed The aarch64 provision failed on three consecutive PRs (#412, #415, #416) in one hour. The log says exactly where, and it is not a mirror outage: attempt 1 update killed at 180s, precisely the timeout attempt 2 update killed at 180s again attempt 3 update succeeded; install killed at 300s MID-DOWNLOAD, the log ending inside `Get:20 gcc-13-aarch64-linux-gnu [21.1 MB]` No apt error appears anywhere in it. The provision was succeeding slowly, and the wrapper converted that into a hard failure three times over -- the shape this release keeps finding, with the sign reversed: a gate that fires on a healthy run rather than one that passes a broken one. It blocked three PRs while reading as infrastructure decay. The bound came from a premise this script asserted and never measured: "a healthy `update` on these runners is a few seconds", which made 180s look like an order of magnitude of headroom. Rewritten to state the arithmetic instead, since that phrasing is precisely what went unchecked. Two independent changes. `apt-get update` now runs ONLY after a direct install has failed. The runner image ships a populated index, so a refresh does not belong on the happy path -- it is the recovery step for the one failure it fixes, an index stale enough that the requested version has moved. Attempt 1 skips it, removing the 180s that killed two of the three attempts before they reached the package at all. Worst case is now 300 + 15 + 600 + 30 + 600 = 1245s, inside the job's 25-minute budget with room for the check. The step also asked for the wrong package, and the comment above it said so without drawing the conclusion: it named `libc6-dev-arm64-cross` as what actually lands the headers, and stated the cross linker is unused because this gate is `cargo check` only. So it installed a whole cross toolchain to obtain a dependency it had already identified -- 20+ packages, one of them 21.1 MB, for a set of headers. bindgen runs the HOST clang against `--sysroot` and never invokes the cross compiler. Both ARM targets now install the header package directly, plus `--no-install-recommends`, which is the same argument as the timeout: every byte downloaded is time spent inside a bound. The sufficiency of the narrower package is VERIFIED BY THIS JOB, not asserted -- if the headers are not where the export step points clang, bindgen fails loudly in `cargo check` on the very run that installs it. The loop's branch structure is proven with a stubbed dry run: attempt 1 issues `install` alone, attempts 2 and 3 issue `update` then `install`. shellcheck and actionlint both clean. * fix(ci): re-calibrate again, this time against a Fetched line This change's own first CI run passed, and the log says why the previous bounds could not have: Fetched 4201 kB in 4min 45s (14.7 kB/s) Fourteen point seven kilobytes per second. The mirror is degraded by roughly three orders of magnitude, which is why every bound derived from "healthy" behaviour was wrong -- and why the old 40 MB package set was hopeless rather than unlucky: at that rate it needed about 45 minutes, past the whole job budget. Even at 4.2 MB it only just passed, and not the way the first draft of this comment assumed. The real sequence, read from the log rather than inferred from the exit code: attempt 1 install downloaded all 4201 kB (285s), then was killed at 300s during dpkg unpack -- the download finished, the install did not attempt 2 update, then install: NO re-download, because the archives were already in /var/cache/apt/archives. Succeeded. So the run was rescued by apt's archive cache persisting across attempts. That is a genuinely useful property -- each attempt makes progress instead of starting over -- but it was undesigned and undocumented, which puts it in the same class as the bound it rescued: behaviour nobody wrote down, working by accident, and indistinguishable from a design until it stops. It is now written down and no longer load-bearing. INSTALL_TIMEOUT is sized so ONE attempt completes at the worst speed actually observed: 285s of download plus dpkg, so 600s is about 2x that. ATTEMPTS drops to 2 to keep the worst case inside the job's 25 minutes -- 600 + 15 + (180 + 600) = 1395s plus ~45s of surrounding steps. The third attempt is no loss: attempt 2 already retries with a refreshed index AND a warm download cache, covering both the stale index and the slow mirror, so a third would only repeat it. * fix(ci): `--` before the package operand Both reviewers raised it independently on #417. `APT_PACKAGE` comes from a workflow `env:` block and never from event data, so this is not closing a live injection path -- but it is one token that makes the guarantee structural rather than dependent on every future caller remembering where the value came from. A value beginning with a hyphen is now an operand, not an option.
a2823e2 to
d8f5cb6
Compare
`dorny/paths-filter`'s `predicate-quantifier` defaults to `some`, which includes a file if it matches ANY pattern in the filter. The `code` filter is `'**'` followed by seven exclusions, so `'**'` matches everything and every `!` line under it has been dead since the filter was written. The action's README states it outright: under `some`, negation syntax is ineffective. Proven from a run, not from the docs. PR #416 changed exactly one file, `AGENTS.md`, and the filter job's log reads: [modified] AGENTS.md Filter code = true Matching files: AGENTS.md [modified] A markdown file matched a filter whose whole purpose is to exclude markdown. Every documentation PR in this repository's history has therefore run the full code matrix -- fmt/clippy/rustdoc, wasm32, no_std and all four ARM cross-compiles -- and today two docs-only PRs (#414, #416) were blocked by a flaky apt provision for jobs that should never have been scheduled at all. Fixed with TWO filter steps rather than one, because `predicate-quantifier` is a step-level input and the two filters need opposite settings. `code` is one positive pattern plus exclusions, so it needs `every`. `accuracy` must keep the default: its eight patterns are ALTERNATIVES -- a file lives in one of those trees, never in all eight -- so `every` would make it unsatisfiable and silently disable the accuracy battery at review time, which is the gate A5 exists to add. Putting both filters in one step is exactly how the bug got here, and one quantifier cannot serve both. Verification is asymmetric and worth stating plainly. That the bug exists is proven by the log above. That the fix works is NOT yet proven: this PR touches `.github/workflows/ci.yml`, so it is correctly a code change and cannot demonstrate a skip. The demonstration is rebasing a docs-only PR onto this and observing `code = false` -- #414 and #416 are both available and both currently show the opposite.
d8f5cb6 to
66c5716
Compare
`dorny/paths-filter`'s `predicate-quantifier` defaults to `some`, which includes a file if it matches ANY pattern in the filter. The `code` filter is `'**'` followed by seven exclusions, so `'**'` matches everything and every `!` line under it has been dead since the filter was written. The action's README states it outright: under `some`, negation syntax is ineffective. Proven from a run, not from the docs. PR #416 changed exactly one file, `AGENTS.md`, and the filter job's log reads: [modified] AGENTS.md Filter code = true Matching files: AGENTS.md [modified] A markdown file matched a filter whose whole purpose is to exclude markdown. Every documentation PR in this repository's history has therefore run the full code matrix -- fmt/clippy/rustdoc, wasm32, no_std and all four ARM cross-compiles -- and today two docs-only PRs (#414, #416) were blocked by a flaky apt provision for jobs that should never have been scheduled at all. Fixed with TWO filter steps rather than one, because `predicate-quantifier` is a step-level input and the two filters need opposite settings. `code` is one positive pattern plus exclusions, so it needs `every`. `accuracy` must keep the default: its eight patterns are ALTERNATIVES -- a file lives in one of those trees, never in all eight -- so `every` would make it unsatisfiable and silently disable the accuracy battery at review time, which is the gate A5 exists to add. Putting both filters in one step is exactly how the bug got here, and one quantifier cannot serve both. Verification is asymmetric and worth stating plainly. That the bug exists is proven by the log above. That the fix works is NOT yet proven: this PR touches `.github/workflows/ci.yml`, so it is correctly a code change and cannot demonstrate a skip. The demonstration is rebasing a docs-only PR onto this and observing `code = false` -- #414 and #416 are both available and both currently show the opposite.
|
Good suggestion, and it sent me to check the actual configuration rather than assume it — which is the right instinct. It turns out not to apply, and both reasons are now in the entry so the next reader does not re-propose it. There is nothing to remove from branch protection. So the pending Disabling its status reporting would cost the one case where it matters. CodeRabbit can be triggered manually, and when it is, the check is the signal that a review ran. Suppressing the context to make idle PRs look tidier would remove the indicator on precisely the PRs where a review was requested. I have also added the thing this session found the hard way: CodeRabbit rate-limits manual triggers per developer. I posted |
|
Tip For best results, initiate chat on the files or code changes.
The ✏️ Learnings added
You are interacting with an AI system. |
c53781b to
a50447d
Compare
`dorny/paths-filter`'s `predicate-quantifier` defaults to `some`, which includes a file if it matches ANY pattern in the filter. The `code` filter is `'**'` followed by seven exclusions, so `'**'` matches everything and every `!` line under it has been dead since the filter was written. The action's README states it outright: under `some`, negation syntax is ineffective. Proven from a run, not from the docs. PR #416 changed exactly one file, `AGENTS.md`, and the filter job's log reads: [modified] AGENTS.md Filter code = true Matching files: AGENTS.md [modified] A markdown file matched a filter whose whole purpose is to exclude markdown. Every documentation PR in this repository's history has therefore run the full code matrix -- fmt/clippy/rustdoc, wasm32, no_std and all four ARM cross-compiles -- and today two docs-only PRs (#414, #416) were blocked by a flaky apt provision for jobs that should never have been scheduled at all. Fixed with TWO filter steps rather than one, because `predicate-quantifier` is a step-level input and the two filters need opposite settings. `code` is one positive pattern plus exclusions, so it needs `every`. `accuracy` must keep the default: its eight patterns are ALTERNATIVES -- a file lives in one of those trees, never in all eight -- so `every` would make it unsatisfiable and silently disable the accuracy battery at review time, which is the gate A5 exists to add. Putting both filters in one step is exactly how the bug got here, and one quantifier cannot serve both. Verification is asymmetric and worth stating plainly. That the bug exists is proven by the log above. That the fix works is NOT yet proven: this PR touches `.github/workflows/ci.yml`, so it is correctly a code change and cannot demonstrate a skip. The demonstration is rebasing a docs-only PR onto this and observing `code = false` -- #414 and #416 are both available and both currently show the opposite.
#418) `dorny/paths-filter`'s `predicate-quantifier` defaults to `some`, which includes a file if it matches ANY pattern in the filter. The `code` filter is `'**'` followed by seven exclusions, so `'**'` matches everything and every `!` line under it has been dead since the filter was written. The action's README states it outright: under `some`, negation syntax is ineffective. Proven from a run, not from the docs. PR #416 changed exactly one file, `AGENTS.md`, and the filter job's log reads: [modified] AGENTS.md Filter code = true Matching files: AGENTS.md [modified] A markdown file matched a filter whose whole purpose is to exclude markdown. Every documentation PR in this repository's history has therefore run the full code matrix -- fmt/clippy/rustdoc, wasm32, no_std and all four ARM cross-compiles -- and today two docs-only PRs (#414, #416) were blocked by a flaky apt provision for jobs that should never have been scheduled at all. Fixed with TWO filter steps rather than one, because `predicate-quantifier` is a step-level input and the two filters need opposite settings. `code` is one positive pattern plus exclusions, so it needs `every`. `accuracy` must keep the default: its eight patterns are ALTERNATIVES -- a file lives in one of those trees, never in all eight -- so `every` would make it unsatisfiable and silently disable the accuracy battery at review time, which is the gate A5 exists to add. Putting both filters in one step is exactly how the bug got here, and one quantifier cannot serve both. Verification is asymmetric and worth stating plainly. That the bug exists is proven by the log above. That the fix works is NOT yet proven: this PR touches `.github/workflows/ci.yml`, so it is correctly a code change and cannot demonstrate a skip. The demonstration is rebasing a docs-only PR onto this and observing `code = false` -- #414 and #416 are both available and both currently show the opposite.
…k never resolves Found while working #411. CodeRabbit's own comment gives the reason: this repository does not receive automatic reviews because it has fewer than 10 stars. The configuration from #316 is loaded and correct -- it reports the .coderabbit.yaml path, the ASSERTIVE profile and the Pro Plus plan -- so only the automatic trigger is missing. Two consequences, both of which have been operated wrongly since #316. The bot ceremony silently covers two bots rather than three. AGENTS.md credits CodeRabbit with catching a critical fast-forward defect (#358) and a use-after-free in the v2.3.5 libretro controller tables. None of that arrives unless a review is explicitly requested, so a PR reviewed by "all three bots" has in fact been reviewed by Copilot and Antigravity. `@coderabbitai review` requests one. And waiting for zero pending checks never terminates. The CodeRabbit context sits with a null status and a null conclusion indefinitely, so a healthy PR here reads as roughly 27 checks with exactly one permanently pending. The merge criterion is `CI success` = SUCCESS with every OTHER check complete -- never "nothing pending", and never a bare check count either, since a CONFLICTING PR also shows a short list because CI cannot run on it at all.
…the check is not the fix Two review findings, both correct in what they point at. Copilot: the parenthetical pointed at a "memory index" that does not exist in this repository -- it referenced a private note a contributor cannot see, which makes the sentence unactionable exactly where it is meant to be actionable. The trap is now stated inline: a CONFLICTING PR shows a short check list because GitHub cannot build a merge ref so CI never runs, and that is indistinguishable from a healthy PR whose jobs are still queuing. The merge criterion is spelled out as three conditions rather than as a cross-reference. Antigravity proposed removing the CodeRabbit check from branch protection instead of documenting it. Checked before answering: `main`'s ruleset requires exactly ONE context, `CI success` (`gh api repos/OWNER/REPO/rules/branches/main`), so the pending CodeRabbit context is not required and blocks nothing -- there is nothing to remove. Suppressing its status reporting would also lose the check on the PRs where a review IS triggered, which is the one occasion it carries information. Both facts are now in the entry so the next reader does not re-propose it. Also records what this session found the hard way: CodeRabbit rate-limits manual triggers per developer, so a batch of `@coderabbitai review` comments can produce no reviews at all while looking like it worked.
a50447d to
0aba85b
Compare
…o lines later Review caught something this PR did to itself. The bullet directly above the new one has said, since #316, that CodeRabbit is a "3rd AUTOMATED PR review bot" whose ceremony "applies before any merge". The new bullet says it does not auto-review and that the ceremony covers two bots. Both were left standing, adjacent, so a reader arriving at the first one gets the false claim and the correction only if they keep going. That is the same defect class this release keeps finding -- prose asserting a behaviour the system does not have -- reproduced while documenting an instance of it. Corrected at the source: the #316 bullet now says CodeRabbit is CONFIGURED as a third bot, that the ceremony applies to its threads, and that it does not review automatically, pointing at the bullet with the detail. It also records what the old wording cost: "automated" plus "applies before any merge" read as a promise that its findings were arriving, and they were not.
|
Good catch, and it is this PR doing to itself the thing it documents. Fixed at the source rather than by merging the two bullets. The bullet directly above has said since #316 that CodeRabbit is a "3rd automated PR review bot" whose ceremony "applies before any merge". The new bullet says it does not auto-review and that the ceremony covers two bots. Left adjacent, a reader arriving at the first gets the false claim and the correction only if they keep going — prose asserting a behaviour the system does not have, reproduced while documenting an instance of it. The #316 bullet now says CodeRabbit is configured as a third bot, that the ceremony applies to its threads, and that it does not review automatically, pointing forward for the detail. It also records what the old wording cost: "automated" plus "applies before any merge" read as a promise that its findings were arriving, and they were not. Kept as two bullets rather than one because they answer different questions — what is configured and what actually happens — and the second is long enough that folding it in would bury the trigger instruction. The contradiction is gone, which was the real problem. On the verbosity nitpick: same answer as the other PRs — |
Antigravity review (Gemini via Ultra)This PR updates the agent documentation to clarify that CodeRabbit PR reviews must be triggered manually and provides instructions for handling its permanently pending CI check. Blocking issuesNone found. Suggestions
Nitpicks
Automated first-pass review by |
Found while working #411, and it has been true since #316 landed.
CodeRabbit does not auto-review this repository
Its own comment says why:
The configuration is loaded and correct — the same comment reports
Path: .coderabbit.yaml,Review profile: ASSERTIVE,Plan: Pro Plus. Nothing about #316's config work is wrong. Only the automatic trigger is absent.Two consequences, both operated wrongly until now
The ceremony silently covers two bots, not three.
AGENTS.mdcredits CodeRabbit with catching a critical fast-forward defect (#358) and a use-after-free in the v2.3.5 libretro controller tables — and none of that arrives unless a review is requested. A PR described as "reviewed by all three bots" has in fact been reviewed by Copilot and Antigravity.@coderabbitai reviewrequests one; I have posted it on all five currently-open PRs.Waiting for zero pending checks never terminates. The
CodeRabbitcontext sits with a null status and a null conclusion indefinitely. A healthy PR here reads as ~27 checks with exactly one permanently pending, which is indistinguishable from a build still running if you gate on "nothing pending".The merge criterion is therefore
CI success= SUCCESS with every other check complete. Not "nothing pending", and not a bare check count either — a CONFLICTING PR also shows a short list, because GitHub cannot build a merge ref and CI never runs at all.Verification
Docs-only, one line added to
AGENTS.md. The quoted reason, the profile, and the plan are read from the bot's comment on #411; the null status is read from the check rollup on #410, a PR that has already merged.