docs: add REVIEW.md for KiloCode automated review guidance#983
Conversation
Built by mining 172 fix commits (18mo) and ~90 human PR review comments from this repo. Top focus areas: Effect values awaited without being run (shipped a real permission bypass), session/worker race conditions, altimate_change marker/fork-merge hygiene, V2-vs-shipped server surface confusion, and dbt/warehouse connector correctness. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
📝 WalkthroughWalkthroughA new documentation file, REVIEW.md, is added to the repository defining a code review policy: scope boundaries, evidence standards, severity calibration, targeted bug classes, repo invariants, exclusions, and comment formatting rules. ChangesReview policy documentation
Estimated code review effort: 1 (Trivial) | ~3 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Reviewed by glm-5.2 · Input: 24.1K · Output: 2.2K · Cached: 85.4K |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
REVIEW.md (2)
12-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid claiming compile-time enforcement.
Marker checks are enforced in CI, not at compile time. That wording overstates the guarantee and may mislead reviewers about when marker failures surface.
🛠 Suggested wording
-- **`Marker Guard`** — presence/format of `altimate_change` marker comments is CI-enforced at compile time (see `#872`, `#904`). Don't flag a *missing* marker as a guess; CI will fail the PR. Do still flag *misuse* (see Focus Areas below) since Marker Guard checks presence, not correctness. +- **`Marker Guard`** — presence/format of `altimate_change` marker comments is CI-enforced (see `#872`, `#904`). Don't flag a *missing* marker as a guess; CI will fail the PR. Do still flag *misuse* (see Focus Areas below) since Marker Guard checks presence, not correctness.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@REVIEW.md` around lines 12 - 13, The “Marker Guard” note overstates the guarantee by saying marker presence/format is enforced at compile time; update the wording in REVIEW.md to say these checks are enforced in CI instead, and keep the rest of the guidance about not flagging missing markers and not treating anti-slop output as a merge gate. Use the existing “Marker Guard” and “anti-slop.yml” bullets as the anchors when revising the phrasing.
16-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExclude
REVIEW.mdfrom the docs-only skip.This blanket exception would make future edits to the review policy itself skip review. Narrow it to generated docs/CHANGELOGs, or explicitly carve out governance docs like
REVIEW.md.🛠 Suggested wording
-- If a PR only touches `.claude/` skills, docs, or trivial config, skip deep review — see Comment style. +- If a PR only touches `.claude/` skills, generated docs, or trivial config, skip deep review — see Comment style. `REVIEW.md` and other governance docs still need review.Based on learnings, docs-only diffs can skip deep review, but policy files need an exception.
Also applies to: 69-71
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@REVIEW.md` around lines 16 - 17, The docs-only skip rule in REVIEW.md is too broad because it would exempt future edits to the review policy itself from review. Update the policy text near the docs-only exception so it only covers generated docs, CHANGELOGs, or similarly low-risk artifacts, and explicitly excludes governance files like REVIEW.md from the skip. Keep the wording aligned with the existing comment style and make sure the same adjustment is reflected in the related duplicated guidance referenced by the comment.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@REVIEW.md`:
- Around line 12-13: The “Marker Guard” note overstates the guarantee by saying
marker presence/format is enforced at compile time; update the wording in
REVIEW.md to say these checks are enforced in CI instead, and keep the rest of
the guidance about not flagging missing markers and not treating anti-slop
output as a merge gate. Use the existing “Marker Guard” and “anti-slop.yml”
bullets as the anchors when revising the phrasing.
- Around line 16-17: The docs-only skip rule in REVIEW.md is too broad because
it would exempt future edits to the review policy itself from review. Update the
policy text near the docs-only exception so it only covers generated docs,
CHANGELOGs, or similarly low-risk artifacts, and explicitly excludes governance
files like REVIEW.md from the skip. Keep the wording aligned with the existing
comment style and make sure the same adjustment is reflected in the related
duplicated guidance referenced by the comment.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
🤖 Code Review — OpenCodeReview (Gemini) — No Issues FoundNo supported files changed. |
2 participants
What this is
Adds a root
REVIEW.mdthat the KiloCode automated code reviewer reads from the PR base branch as repo-specific review guidance. It activates automatically once this merges tomain— Kilo just needs the "Use REVIEW.md" setting enabled.How it was built
Mined from this repo's own history: 172 fix commits (~18 months) and ~90 human PR review comments, condensed into a single ~9,500-character digest evidence base. Every focus area below cites the specific fix commits/PRs that motivated it — no generic advice.
Top focus areas
await ctx.ask(...)-style calls only awaited theEffectwrapper object without executing it, shipping a real permission-check bypass. Now called out as a Critical pattern to catch.cancel()/idle-state races, worker leaks on invalid--session, stale-file races, DuckDB concurrent-access retries.altimate_changemarker discipline (100% coverage bar) and guidance on when Marker Guard false-positives on deep edits inside already-marked blocks vs. genuine upstream-only API leakage (LanguageModelV3*vs the shippedV2surface).packages/server) alongside the real shipped server (packages/opencode/src/server); several past review findings were false positives against the dead surface.🤖 Generated with Claude Code
Summary by cubic
Adds a root REVIEW.md to guide KiloCode’s automated reviews with repo-specific rules. This reduces false positives (e.g., V2 vs shipped surface) and focuses checks on real risks like unrun Effects and session/worker races.
Written for commit a3be9ae. Summary will update on new commits.
Summary by CodeRabbit