Configure AI PR reviewers: CodeRabbit primary, Codex secondary, Copilot off#338
Configure AI PR reviewers: CodeRabbit primary, Codex secondary, Copilot off#338brianmhunt wants to merge 4 commits intomainfrom
Conversation
…ot off Prior PRs (notably #337) ran with all three reviewers on defaults, producing duplicate findings with uneven signal. Codify the intended division of labor: - CodeRabbit — primary. New .coderabbit.yaml with: - auto_review on main, drafts skipped - TKO-specific path_instructions so it doesn't flag factory-function-as-constructor, Object.setPrototypeOf, module- level side effects, or pervasive `any` as anti-patterns - spec-path instructions explaining vitest globals + Mocha/Chai/Sinon - chill profile, no poem, review_status on - Codex (chatgpt-codex-connector) — secondary / adversarial. Leave its auto-review installed so convergence between Codex and CodeRabbit signals high-confidence findings. Useful as a second opinion rather than a primary. - Copilot — disabled at the repo level (UI setting; no repo file). Its contributions on #337 were stale (flagged an O(n²) pattern that was already fixed) and misdirected (suggested env-gating that the maintainer explicitly rejected). Adds an "AI PR review" section to AGENTS.md documenting the arrangement so contributors and future agents understand the intent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 52 minutes and 13 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughA new CodeRabbit configuration file ( Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~4 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Pull request overview
This PR codifies the repository’s AI PR review setup by adding a CodeRabbit configuration file and documenting the intended reviewer roles in AGENTS.md, aiming to reduce duplicated/noisy automated feedback.
Changes:
- Add
.coderabbit.yamlto configure CodeRabbit as the primary auto-reviewer on PRs targetingmain, skipping drafts, and applying TKO-specific path instructions. - Document the layered AI reviewer arrangement (CodeRabbit primary, Codex secondary, Copilot disabled) in
AGENTS.md.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
AGENTS.md |
Adds an “AI PR review” section documenting the intended roles and rationale for the three AI reviewers. |
.coderabbit.yaml |
Introduces CodeRabbit configuration (auto-review settings + path-scoped instructions tailored to TKO patterns/tests). |
phillipc
left a comment
phillipc
left a comment
There was a problem hiding this comment.
You can deactivate CoPilot Reviews in the repo settings.
Keep AGENTS.md lean — the rationale and config belong in .coderabbit.yaml. AGENTS.md just needs the breadcrumb. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.coderabbit.yaml:
- Around line 47-52: Add a new CodeRabbit path entry mirroring the existing
packages/**/spec/**/*.ts block but targeting build spec artifacts by adding an
instructions block for the builds/*/spec/**/*.js pattern that contains the same
guidance about Mocha-style describe/it with Chai and Sinon and that Vitest
injects globals (so bare it()/describe() are expected); ensure the new path
pattern uses the same instructions text as the packages/**/spec/**/*.ts entry so
bundled build tests are not flagged for missing global imports.
- Around line 43-45: The comment points to a non-existent docs link in the YAML:
replace the incorrect reference text "tko.io/public/agents/options.md" with the
correct agent guide reference so the runtime option pattern points to the
existing agents guide; locate the line mentioning ko.options.* and defineOption
in the .coderabbit.yaml content and update the referenced file string to the
agent-facing guide (agents/guide.md) so the docs link resolves correctly.
In `@AGENTS.md`:
- Around line 156-159: The document currently states "Copilot disabled" under
the "AI PR review" section which implies the commit enforces this; update the
wording around that phrase to clarify that disabling GitHub Copilot is an
external repository-level setting (a post-merge GitHub UI action) rather than
something this commit enforces—e.g., change the line to indicate "Copilot should
be disabled at the repository level (GitHub setting); this is an external repo
configuration, not enforced by this commit" so readers understand it’s a
required repo setting, not enforced by code changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a982e510-7962-43cf-8218-c9387d486792
📒 Files selected for processing (2)
.coderabbit.yamlAGENTS.md
| - For runtime options (`ko.options.*`), the pattern is | ||
| `defineOption` from @tko/utils inside the owning package; see | ||
| tko.io/public/agents/options.md. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify that the CodeRabbit config references an existing agent-facing options guide.
set -euo pipefail
fd -a '^options\.md$' tko.io/public/agents || true
rg -n -C2 'options\.md|defineOption|ko\.options' tko.io/public/agents AGENTS.md .coderabbit.yamlRepository: knockout/tko
Length of output: 917
🏁 Script executed:
#!/bin/bash
# List all files in tko.io/public/agents/ to confirm what actually exists
echo "=== Files in tko.io/public/agents/ ==="
ls -la tko.io/public/agents/ 2>/dev/null || echo "Directory not found"
echo ""
echo "=== All markdown files in tko.io/public/ ==="
find tko.io/public -name "*.md" 2>/dev/null
echo ""
echo "=== Content of AGENTS.md if it exists ==="
cat AGENTS.md 2>/dev/null || echo "AGENTS.md not found"Repository: knockout/tko
Length of output: 14482
Update reference to existing agent guide.
Line 45 references tko.io/public/agents/options.md, which does not exist. The actual agent-facing documentation files are guide.md, testing.md, glossary.md, and contract.md. Update the reference to point to an existing file—agents/guide.md (API reference) is the most appropriate for documenting the defineOption pattern for runtime options.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.coderabbit.yaml around lines 43 - 45, The comment points to a non-existent
docs link in the YAML: replace the incorrect reference text
"tko.io/public/agents/options.md" with the correct agent guide reference so the
runtime option pattern points to the existing agents guide; locate the line
mentioning ko.options.* and defineOption in the .coderabbit.yaml content and
update the referenced file string to the agent-facing guide (agents/guide.md) so
the docs link resolves correctly.
| - path: "packages/**/spec/**/*.ts" | ||
| instructions: | | ||
| Tests use Mocha-style describe/it with Chai assertions and Sinon | ||
| fake timers/spies. Vitest runs them with globals enabled. Bare | ||
| `it()` and `describe()` are expected; they come from vitest's | ||
| global injection, not a missing import. |
There was a problem hiding this comment.
Apply the test guidance to build specs too.
AGENTS.md Line 68 documents builds/*/spec/**/*.js as test files, but this path instruction only covers packages/**/spec/**/*.ts. Add a build-spec entry so CodeRabbit also avoids false positives on Mocha/Vitest globals in bundled build tests.
Proposed config addition
- path: "packages/**/spec/**/*.ts"
instructions: |
Tests use Mocha-style describe/it with Chai assertions and Sinon
fake timers/spies. Vitest runs them with globals enabled. Bare
`it()` and `describe()` are expected; they come from vitest's
global injection, not a missing import.
+
+ - path: "builds/**/spec/**/*.js"
+ instructions: |
+ Tests use Mocha-style describe/it with Chai assertions and Sinon
+ fake timers/spies. Vitest runs them with globals enabled. Bare
+ `it()` and `describe()` are expected; they come from vitest's
+ global injection, not a missing import.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.coderabbit.yaml around lines 47 - 52, Add a new CodeRabbit path entry
mirroring the existing packages/**/spec/**/*.ts block but targeting build spec
artifacts by adding an instructions block for the builds/*/spec/**/*.js pattern
that contains the same guidance about Mocha-style describe/it with Chai and
Sinon and that Vitest injects globals (so bare it()/describe() are expected);
ensure the new path pattern uses the same instructions text as the
packages/**/spec/**/*.ts entry so bundled build tests are not flagged for
missing global imports.
| ## AI PR review | ||
|
|
||
| CodeRabbit (primary) + Codex (secondary). Copilot disabled. Config and | ||
| rationale in `.coderabbit.yaml`. |
There was a problem hiding this comment.
Clarify that Copilot disabling is an external repo setting.
Line 158 says Copilot is disabled, but the PR notes that this is a post-merge GitHub UI action. Consider wording this as an intended/required repository setting so the docs don’t overstate what this commit enforces.
Proposed wording
## AI PR review
-CodeRabbit (primary) + Codex (secondary). Copilot disabled. Config and
-rationale in `.coderabbit.yaml`.
+CodeRabbit (primary) + Codex (secondary). Copilot auto-review should remain
+disabled via repository settings. Config and rationale live in `.coderabbit.yaml`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## AI PR review | |
| CodeRabbit (primary) + Codex (secondary). Copilot disabled. Config and | |
| rationale in `.coderabbit.yaml`. | |
| ## AI PR review | |
| CodeRabbit (primary) + Codex (secondary). Copilot auto-review should remain | |
| disabled via repository settings. Config and rationale live in `.coderabbit.yaml`. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AGENTS.md` around lines 156 - 159, The document currently states "Copilot
disabled" under the "AI PR review" section which implies the commit enforces
this; update the wording around that phrase to clarify that disabling GitHub
Copilot is an external repository-level setting (a post-merge GitHub UI action)
rather than something this commit enforces—e.g., change the line to indicate
"Copilot should be disabled at the repository level (GitHub setting); this is an
external repo configuration, not enforced by this commit" so readers understand
it’s a required repo setting, not enforced by code changes.
- Remove every field that just restates a CodeRabbit default (language, early_access, profile: chill, request_changes_workflow, high_level_summary, review_status, collapse_walkthrough, auto_review sub-fields, chat.auto_reply). Keep only `poem: false` (default is true). - Drop the "TKO-specific context" comment block — the instructions below are self-explanatory. - Fix glob overlap: `packages/**/*.ts` matched spec files too, so CodeRabbit would attach BOTH instruction sets to every spec review. Narrow to `packages/*/src/**/*.ts` so src and spec patterns don't overlap. Net: 55 lines → 35 lines, same behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `do NOT flag` list was gatekeeping masquerading as context. CodeRabbit is better off with a clear view of the repo than with a pre-filtered opinion of what it's allowed to notice. If it produces false positives on TKO's intentional patterns (factory-function-as- constructor, module-level side effects, etc.), we add narrow guidance reactively. Test-style instructions also gone — defensive against findings that probably weren't coming anyway. Config is now `poem: false` plus CodeRabbit defaults. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Closing — scope didn't earn its keep. Reviewer arrangement (CodeRabbit primary, Codex secondary, Copilot off) is being enacted via repo UI toggles instead. AGENTS.md note can be added back in a tiny drive-by PR if wanted, but not worth its own PR for 3 lines. |
Summary
Codify the AI PR review arrangement to eliminate duplicate findings and uneven signal across three reviewers. Previous PRs (e.g. #337) ran all three on defaults and got near-identical findings on the same bug, with Copilot contributing the most noise.
Changes
.coderabbit.yaml(new, 50 lines) — pins CodeRabbit as primary with:Not in this PR (UI-only toggles)
Two settings live in GitHub UI and can't be committed:
I'll do these two UI steps after this PR merges.
Why this arrangement
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit