-
Notifications
You must be signed in to change notification settings - Fork 0
docs: codify AI reviewer safety policy and evaluation #41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 30 commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
5ae2622
docs: capture AI reviewer army design
absolutepraya b74163f
docs: add AI reviewer army implementation plan
absolutepraya a51ab4d
chore: configure Qodo review-only mode
absolutepraya d7f1c50
docs: add AI code review policy
absolutepraya d850731
docs: document multi-reviewer workflow
absolutepraya ff03433
docs: add AI review agent guardrails
absolutepraya c0b6632
docs: clarify reviewer free-tier rollout
absolutepraya 26571c7
docs: add AI reviewer live rollout plan
absolutepraya 6687226
chore: make Qodo draft policy explicit
absolutepraya 8c208f9
chore: remove Qodo reviewer configuration
absolutepraya eeb8fb8
docs: revise AI review policy without Qodo
absolutepraya 4da70f9
docs: revise AI reviewer army design
absolutepraya 55766d3
docs: revise AI reviewer implementation plan
absolutepraya a8168f7
docs: revise AI reviewer live rollout
absolutepraya b1d34a7
docs: update reviewer guidance without Qodo
absolutepraya 405085f
docs: update agent AI review policy
absolutepraya 480f1cf
docs: harden reviewer rollout checks
absolutepraya 2fe9a05
docs: complete reviewer validation plan
absolutepraya 2f262ec
docs: preserve current agent guidance
absolutepraya ba489bc
Merge main into chore/40-ai-reviewer-army
absolutepraya 7221af0
docs: tighten AI review merge policy
absolutepraya c593b3e
docs: record reviewer app installation
absolutepraya 57fc384
docs: record AI reviewer safety policy and rejected candidates
absolutepraya a7da0e3
docs: record AI reviewer evaluation outcome
absolutepraya 0a8e536
docs: convert reviewer rollout into safety and research plan
absolutepraya 40313d9
docs: record failed hosted reviewer rollout
absolutepraya 72f0a4b
docs: align contributor AI review guidance with accepted reviewer
absolutepraya cfcf43c
docs: align agent AI review guidance with accepted reviewer
absolutepraya 3f72f70
docs: clarify hosted reviewer cleanup state
absolutepraya 959e302
docs: record reviewer app removal
absolutepraya 7666737
docs: mark rejected reviewer cleanup complete
absolutepraya f9aa8bd
ci: temporarily validate docs for PR 41
absolutepraya 7c42e77
ci: remove temporary PR 41 docs validation
absolutepraya File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,205 @@ | ||
| # AI code review policy | ||
|
|
||
| AI-assisted pull request review is advisory engineering signal. It supplements, but never replaces, GitHub Actions, tests, issue/spec intent, maintainer judgment, or understanding of the existing codebase. | ||
|
|
||
| ## Current accepted reviewer | ||
|
|
||
| CodeRabbit is currently the only accepted AI pull-request reviewer for this repository. | ||
|
|
||
| It remains the broad, repository-aware baseline configured by `.coderabbit.yaml`. Its existing configuration should not be changed merely to accommodate another reviewer. | ||
|
|
||
| No other AI reviewer should be treated as enabled or approved until it passes the acceptance criteria in this document. | ||
|
|
||
| ## Review authority | ||
|
|
||
| An AI review comment is evidence of a possible problem. It is not an instruction to change the code. | ||
|
|
||
| Before acting on a substantive AI finding, verify it against the strongest available sources of truth: | ||
|
|
||
| 1. the originating issue, approved spec, or explicit maintainer decision; | ||
| 2. surrounding code and established behavior; | ||
| 3. tests that encode externally observable behavior or invariants; | ||
| 4. repository documentation and agent instructions; | ||
| 5. actual runtime, API, data-flow, authorization, database, and deployment semantics where relevant. | ||
|
|
||
| Agreement between multiple reviewers increases investigation priority, but it does not prove a finding is correct. | ||
|
|
||
| ### Behavior-changing suggestions | ||
|
|
||
| Do not implement a reviewer suggestion automatically if it would materially change any of these areas unless the issue/spec clearly requires the change: | ||
|
|
||
| - user-visible functionality or UX flow; | ||
| - authentication, authorization, viewer/editor/owner, sharing, or other permission behavior; | ||
| - deletion, retention, ownership, persistence, or lifecycle semantics; | ||
| - API contracts; | ||
| - database constraints, migrations, or data interpretation; | ||
| - compatibility behavior or upstream-sync assumptions; | ||
| - deployment/runtime architecture or operator behavior; | ||
| - intentionally chosen edge-case behavior. | ||
|
|
||
| If issue/spec/code/tests/docs do not resolve the intended behavior, ask the maintainer instead of changing the code merely to satisfy a reviewer. | ||
|
|
||
| ## Hard review-only rule | ||
|
|
||
| The reviewer setup must never automatically modify repository code. | ||
|
|
||
| Do not enable: | ||
|
|
||
| - automatic commits; | ||
| - automatic pushes; | ||
| - automatic application of code suggestions; | ||
| - reviewer-driven coding/fixing agents; | ||
| - workflows that turn review comments into patches automatically; | ||
| - AI auto-approval as merge authority; | ||
| - reviewer-driven branch mutations. | ||
|
|
||
| Inline GitHub suggestion blocks are allowed because a human or coding agent must still choose to apply them deliberately. | ||
|
|
||
| ## Merge and CI policy | ||
|
|
||
| AI review remains advisory. | ||
|
|
||
| - Do not make an AI reviewer a required merge check. | ||
| - Do not require a minimum number of AI approvals. | ||
| - Do not require every AI thread to be mechanically resolved before merge. | ||
| - Do not let a reviewer quota or outage become a merge dependency. | ||
| - Do not replace deterministic CI with AI judgment. | ||
|
|
||
| GitHub Actions remains authoritative for deterministic validation such as formatting, linting, typechecking, tests, generated-artifact checks, Knip, React Doctor, and other repository CI checks. | ||
|
|
||
| Serious AI findings should still be investigated. Advisory means the reviewer cannot become authority by itself, not that review feedback should be ignored. | ||
|
|
||
| ## Noise control | ||
|
|
||
| AI review should focus on substantive engineering problems: | ||
|
|
||
| - correctness and logic bugs; | ||
| - regressions and edge cases; | ||
| - authorization, privacy, and security issues; | ||
| - data-loss or lifecycle risks; | ||
| - meaningful test gaps; | ||
| - maintainability/design issues with real impact; | ||
| - performance problems; | ||
| - unimplemented issue/spec requirements. | ||
|
|
||
| Avoid duplicating formatter/linter/typechecker output or producing generic style advice without demonstrated impact. | ||
|
|
||
| The current CodeRabbit generated-artifact exclusions are: | ||
|
|
||
| - `packages/open-api/karakeep-openapi-spec.json` | ||
| - `packages/sdk/src/karakeep-api.d.ts` | ||
| - `packages/db/drizzle/meta/**` | ||
|
|
||
| Future reviewers should receive equivalent exclusions where their free tier supports them. | ||
|
|
||
| ## Acceptance criteria for any additional reviewer | ||
|
|
||
| A future reviewer must satisfy all of the following before it is approved for ongoing use. | ||
|
|
||
| ### Cost and friction | ||
|
|
||
| - zero paid subscription cost for the intended repository usage; | ||
| - no silent paid upgrade if pricing, eligibility, or quotas change; | ||
| - no recurring administrative approval/application process merely to preserve the free tier unless the maintainer explicitly chooses to accept that trade-off. | ||
|
|
||
| ### Repository scope | ||
|
|
||
| - install only for `absolutepraya/karakeep`, not all repositories on the account; | ||
| - review non-draft pull requests targeting `main` where the service supports scope controls; | ||
| - avoid wasting review quota on generated output and pure dependency-bot PRs where controls exist. | ||
|
|
||
| ### Permission ceiling | ||
|
|
||
| Reasonable permissions include: | ||
|
|
||
| - repository contents: read; | ||
| - pull requests/reviews/comments: write when required to publish review feedback; | ||
| - issues/comments: write only when required for review interaction; | ||
| - checks/statuses: read/write when required to publish review state; | ||
| - metadata: read. | ||
|
|
||
| Reject the reviewer if its GitHub App requires any of these capabilities for installation or normal review operation: | ||
|
|
||
| - repository contents: write; | ||
| - GitHub Actions/workflows: write; | ||
| - repository administration; | ||
| - secrets or environments; | ||
| - equivalent broad code-mutation privileges. | ||
|
|
||
| A vendor promise that review mode will not use an excessive permission does not override this rule. Capability matters as well as intended behavior. | ||
|
|
||
| ### Behavior | ||
|
|
||
| - review/comments only; | ||
| - no automatic commits, pushes, fix application, branch mutation, or autonomous coding agents; | ||
| - no required merge status; | ||
| - deterministic CI remains independent and authoritative. | ||
|
|
||
| ### Quality | ||
|
|
||
| New reviewers are probationary. Findings should be classified as: | ||
|
|
||
| 1. **Confirmed defect**: evidence shows the problem is real and within intended scope. Fix it. | ||
| 2. **Valid but optional**: technically reasonable but outside the requested scope. Normally leave it out of the PR. | ||
| 3. **Intentional behavior**: the reviewer misunderstood a deliberate choice. Reject it. | ||
| 4. **False positive**: the factual claim does not hold after inspection. Reject it. | ||
| 5. **Ambiguous or behavior-changing**: the proposed change could alter intended semantics and available sources do not resolve intent. Ask the maintainer. | ||
|
|
||
| Two or more reviewers independently flagging the same concern increases investigation priority but does not change these classification rules. | ||
|
|
||
| ## Evaluated candidates | ||
|
|
||
| The following services were evaluated during issue #40 and PR #41. They are not approved active reviewers. | ||
|
|
||
| | Reviewer | Result | Reason | | ||
| | --- | --- | --- | | ||
| | Qodo | Rejected | Its permanent zero-cost path requires open-source qualification/application, which did not fit the low-friction free-plan requirement for this rollout. | | ||
| | Sourcery | Rejected | Live installation exposed GitHub App permissions including repository contents write plus Actions/workflows write. This exceeds the approved least-privilege ceiling. Its first review also produced one intentional/optional documentation suggestion and one clear false claim about the dated spec files being future-dated. | | ||
| | Graphite Agent | Rejected | Graphite's GitHub App permission model includes read/write access to repository contents and workflows/Actions as part of its broader PR product. This exceeds the approved review-only permission ceiling even if AI Reviews themselves are configured not to mutate code. | | ||
|
|
||
| These rejections are not claims that the vendors are malicious. They mean the integrations do not fit this repository's chosen risk model. | ||
|
|
||
| Do not re-enable one of these candidates without a new explicit maintainer decision based on materially changed pricing, permissions, or product behavior. | ||
|
|
||
| ## Current research target | ||
|
|
||
| Issue #40 remains open as the search for an additional reviewer that is: | ||
|
|
||
| > free + useful + review-only + least privilege | ||
|
|
||
| A smaller safe reviewer set is preferable to an "AI reviewer army" that requires broad write access. | ||
|
|
||
| ## Manual reviewer interaction | ||
|
|
||
| Use manual re-review only when it adds value after meaningful changes. | ||
|
|
||
| Current accepted example: | ||
|
|
||
| ```text | ||
| @coderabbitai review | ||
| ``` | ||
|
|
||
| ## Maintainer evaluation checklist | ||
|
|
||
| For any future candidate: | ||
|
|
||
| 1. Confirm current official pricing/free-tier terms. | ||
| 2. Confirm the App can be scoped only to `absolutepraya/karakeep`. | ||
| 3. Inspect the exact GitHub App permissions before treating the integration as accepted. | ||
| 4. Reject contents-write, Actions/workflows-write, administration, secrets/environment, or equivalent mutation capability. | ||
| 5. Enable review-only behavior only. | ||
| 6. Use a real non-draft PR as a smoke test. | ||
| 7. Confirm the reviewer comments but does not commit, push, apply changes, or mutate the branch. | ||
| 8. Confirm no reviewer check is required by branch protection/rulesets. | ||
| 9. Inspect the first reviews for false positives, generic advice, duplication, and behavior-changing suggestions. | ||
| 10. Record the evaluation result in the issue/spec before adopting the reviewer. | ||
|
|
||
| ## Design and implementation history | ||
|
|
||
| The complete decision history and evaluation are preserved in: | ||
|
|
||
| - `docs/superpowers/specs/2026-08-15-ai-reviewer-army-design.md` | ||
| - `docs/superpowers/plans/2026-08-15-ai-reviewer-army.md` | ||
| - `docs/superpowers/plans/2026-08-15-ai-reviewer-army-live-rollout.md` | ||
|
|
||
| Vendor behavior changes over time. Re-check official vendor documentation before reconsidering a rejected integration. |
150 changes: 150 additions & 0 deletions
150
docs/superpowers/plans/2026-08-15-ai-reviewer-army-live-rollout.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,150 @@ | ||
| # AI Reviewer Hosted Evaluation Record | ||
|
|
||
| This companion record captures the hosted-service portion of issue #40 and PR #41. It supersedes the original plan to keep Sourcery and Graphite enabled alongside CodeRabbit. | ||
|
|
||
| **Related issue:** #40 | ||
|
|
||
| **Canonical policy:** `docs/ai-code-review.md` | ||
|
|
||
| **Design:** `docs/superpowers/specs/2026-08-15-ai-reviewer-army-design.md` | ||
|
|
||
| ## Non-negotiable safety model | ||
|
|
||
| - Reviewers are critics only. | ||
| - No automatic commits, pushes, applied fixes, reviewer-driven coding agents, or branch mutation. | ||
| - AI comments are evidence to verify, never authority over issue/spec intent or established behavior. | ||
| - No AI reviewer is a required merge gate. | ||
| - Install a candidate only for `absolutepraya/karakeep`. | ||
| - Reject repository-content write, Actions/workflow write, administration, secrets/environments, or equivalent broad code-mutation privileges. | ||
| - Use only zero-cost access unless the maintainer explicitly makes a different decision later. | ||
|
|
||
| ## Original candidate set | ||
|
|
||
| The experiment considered: | ||
|
|
||
| - CodeRabbit: existing trusted baseline; | ||
| - Qodo: correctness/spec/test-focused candidate; | ||
| - Sourcery: maintainability/design/performance/security candidate; | ||
| - Graphite Agent: logic/edge-case/regression candidate. | ||
|
|
||
| The final accepted state after evaluation is **CodeRabbit only**. | ||
|
|
||
| ## Qodo evaluation | ||
|
|
||
| **Status: rejected before installation.** | ||
|
|
||
| Qodo's permanent zero-cost path required open-source qualification/application. That did not fit the desired low-friction "install and keep using the free plan" model. | ||
|
|
||
| No Qodo repository configuration remains in the final PR diff. | ||
|
|
||
| ## Sourcery live evaluation | ||
|
|
||
| **Status: rejected after installation and smoke test.** | ||
|
|
||
| What worked: | ||
|
|
||
| - free open-source access was available; | ||
| - the GitHub App was connected to `absolutepraya/karakeep`; | ||
| - a manual `@sourcery-ai review` request successfully triggered a review on PR #41. | ||
|
|
||
| Why it was rejected: | ||
|
|
||
| GitHub reported the installed Sourcery App with permissions including: | ||
|
|
||
| - repository contents: write; | ||
| - Actions: write; | ||
| - workflows: write; | ||
| - checks/statuses/reviews/comments write permissions needed for integration behavior. | ||
|
|
||
| The first three capabilities exceed the approved least-privilege ceiling. The integration is therefore rejected even if Sourcery's PR-review mode is configured not to auto-fix. | ||
|
|
||
| ### First-review calibration | ||
|
|
||
| Sourcery's first high-level review raised two points: | ||
|
|
||
| 1. reduce duplication between the canonical policy, design spec, and plans; | ||
| 2. rename the `2026-08-15-*` files because it claimed they were future-dated. | ||
|
|
||
| Classification: | ||
|
|
||
| - duplication comment: **valid but optional / intentional design trade-off**. The maintainer explicitly requested complete durable design and execution knowledge in the PR, and the repository already uses separate spec and plan files; | ||
| - date comment: **false positive**. The review ran on 2026-08-16, so `2026-08-15` was the previous day, not a future date. | ||
|
|
||
| No repository changes should be made merely to satisfy either finding. | ||
|
|
||
| ## Graphite live evaluation | ||
|
|
||
| **Status: rejected on permission model.** | ||
|
|
||
| What worked: | ||
|
|
||
| - the free Hobby plan was available; | ||
| - Graphite was connected to `absolutepraya/karakeep` with AI Reviews enabled. | ||
|
|
||
| Why it was rejected: | ||
|
|
||
| Graphite's GitHub App permission model includes read/write access to repository contents and Actions/workflows as part of the broader Graphite pull-request product. | ||
|
|
||
| Those capabilities exceed the approved review-only ceiling. Graphite is therefore rejected even if AI Reviews themselves are configured as non-mutating. | ||
|
|
||
| A long-term quality probation is unnecessary after the permission failure. | ||
|
|
||
| ## CodeRabbit regression check | ||
|
|
||
| CodeRabbit remains the accepted reviewer. | ||
|
|
||
| - Keep `.coderabbit.yaml` unchanged unless a real integration conflict is discovered. | ||
| - Keep CodeRabbit advisory rather than authoritative. | ||
| - Apply the same evidence-not-authority rule to CodeRabbit findings even though it has more practical trust in this repository. | ||
|
|
||
| ## Branch protection / ruleset policy | ||
|
|
||
| No AI reviewer status should be a required merge check. | ||
|
|
||
| Reviewer downtime, quota exhaustion, or a negative AI status must never mechanically block merge eligibility. | ||
|
|
||
| If a future candidate automatically introduces or encourages a required AI status, remove that requirement before evaluating the integration further. | ||
|
|
||
| ## Hosted cleanup complete | ||
|
|
||
| The rejected integrations no longer retain repository access. | ||
|
|
||
| On 2026-08-16, the maintainer confirmed that repository access for both rejected hosted integrations was removed: | ||
|
|
||
| - Sourcery access to `absolutepraya/karakeep` removed; | ||
| - Graphite access to `absolutepraya/karakeep` removed. | ||
|
|
||
| Repository-side verification also confirmed that `main` has no required AI-review status checks or branch-protection requirements. CodeRabbit remains the accepted active reviewer. | ||
|
|
||
| If either rejected integration is reconsidered later, treat it as a fresh evaluation and re-check the current permission model before granting access again. | ||
|
|
||
| ## Evaluation result | ||
|
|
||
| The original three-reviewer live rollout is **not successful** under the approved safety model. | ||
|
|
||
| That is an expected valid outcome of the experiment. The design explicitly preferred a smaller safe reviewer set to weakening least-privilege requirements. | ||
|
|
||
| The resulting reviewer state is: | ||
|
|
||
| | Reviewer | State | | ||
| | --- | --- | | ||
| | CodeRabbit | Accepted / active | | ||
| | Qodo | Rejected: OSS qualification/application friction | | ||
| | Sourcery | Rejected and access removed: excessive GitHub App write permissions; first review also demonstrated noise/false-positive risk | | ||
| | Graphite Agent | Rejected and access removed: excessive GitHub App write permissions | | ||
|
|
||
| ## Future research | ||
|
|
||
| Issue #40 remains open to find an additional reviewer satisfying: | ||
|
|
||
| > free + useful + review-only + least privilege | ||
|
|
||
| For future candidates: | ||
|
|
||
| 1. verify current pricing first; | ||
| 2. inspect exact GitHub App permissions before treating installation as accepted; | ||
| 3. reject contents-write, Actions/workflows-write, admin, secrets/environment, or equivalent mutation capability; | ||
| 4. scope installation to this repository only; | ||
| 5. use a real PR as a smoke test; | ||
| 6. inspect actual findings before trusting the reviewer; | ||
| 7. record both permission and quality results in the repository. | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.