Rewrite testbot README with architecture diagrams#901
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughREADME rewritten to present Testbot as a GitHub Actions bot with two workflows: a generate workflow (weekday cron or manual dispatch) that creates timestamped branches and Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer (PR / Commenter)
participant GitHub as GitHub (Repo & Actions)
participant Generate as Generate Workflow
participant Respond as Respond Workflow
participant Model as Claude Code (AI)
participant Bot as testbot[bot] (Committer)
Dev->>GitHub: Create repo change or manual dispatch
GitHub->>Generate: Trigger generate workflow (cron/manual)
Generate->>GitHub: Create timestamped branch + `[testbot]` PR (label `ai-generated`)
Dev->>GitHub: Post inline review comment starting with `/testbot` on `ai-generated` PR
GitHub->>Respond: Trigger respond workflow (inline comment event)
Respond->>Model: Send conversation with guardrails and prompts
Model->>Respond: Return code edits and inline replies
Respond->>Bot: Commit edits as `testbot[bot]` and post inline replies
Bot->>GitHub: Push commit (with GH013 retry handling)
GitHub->>Dev: Show bot commit and inline replies
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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 `@src/scripts/testbot/README.md`:
- Line 5: The README currently says "Generates tests for low-coverage files on a
weekly schedule" but the cron shown ("0 6 * * 1-5") runs weekdays; update the
wording to match the cron (e.g., change the phrase to "runs on weekdays at
06:00" or "weekday schedule (Mon–Fri)") or, if you intend a true weekly run,
replace the cron with a weekly expression (e.g., a single weekday like "0 6 * *
1"). Edit the sentence containing "Generates tests ... weekly" and/or the cron
string "0 6 * * 1-5" so both describe the same schedule.
- Line 85: The README contains inconsistent descriptions of the Auto-approver
authorization; update both occurrences so they state the same rule: that the
actor must be either a member of the NVIDIA/osmo-dev team OR one of the trusted
bots (svc-osmo-ci, github-actions[bot], coderabbitai[bot]). Edit the
Auto-approver authorization paragraph(s) to use identical wording and include
the explicit OR logic and the trusted bot list so both descriptions match
exactly.
- Around line 184-186: Update the README table values to match the actual
defaults used by respond.py: change `--max-turns` from `75` to `50` and
`--timeout` from `900` to `720`; reference the CLI flags in the docs and verify
they match the defaults defined in the respond.py entrypoint (flags
`--max-turns` and `--timeout`) so operators see the correct values.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: de1c300c-1665-4d1c-9f1f-a7ca362f7b1d
📒 Files selected for processing (1)
src/scripts/testbot/README.md
| Testbot is a GitHub Actions bot backed by Claude Code that: | ||
|
|
||
| ## Architecture | ||
| 1. **Generates tests** for low-coverage files on a weekly schedule (opens PRs for review) |
There was a problem hiding this comment.
Resolve schedule wording mismatch (“weekly” vs weekday cron).
Line 5 says generation runs on a weekly schedule, but Line 47 defines a weekday cron (0 6 * * 1-5), which is daily on weekdays. Please align wording to avoid confusion.
Also applies to: 47-47
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/scripts/testbot/README.md` at line 5, The README currently says
"Generates tests for low-coverage files on a weekly schedule" but the cron shown
("0 6 * * 1-5") runs weekdays; update the wording to match the cron (e.g.,
change the phrase to "runs on weekdays at 06:00" or "weekday schedule
(Mon–Fri)") or, if you intend a true weekly run, replace the cron with a weekly
expression (e.g., a single weekday like "0 6 * * 1"). Edit the sentence
containing "Generates tests ... weekly" and/or the cron string "0 6 * * 1-5" so
both describe the same schedule.
|
|
||
| The respond workflow runs under `environment: testbot-respond` (gated by a required reviewer) so it can access the `NVIDIA_NIM_KEY` secret. The auto-approver runs on `main` via `workflow_run` and: | ||
|
|
||
| 1. Checks if the triggering actor is in the `NVIDIA/osmo-dev` team, OR a trusted bot (`svc-osmo-ci`, `github-actions[bot]`, `coderabbitai[bot]`) |
There was a problem hiding this comment.
Auto-approver authorization description is internally inconsistent.
Line 85 says authorization is team member or trusted bot, while Line 100 describes only team membership verification. Please make these statements consistent.
Also applies to: 100-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/scripts/testbot/README.md` at line 85, The README contains inconsistent
descriptions of the Auto-approver authorization; update both occurrences so they
state the same rule: that the actor must be either a member of the
NVIDIA/osmo-dev team OR one of the trusted bots (svc-osmo-ci,
github-actions[bot], coderabbitai[bot]). Edit the Auto-approver authorization
paragraph(s) to use identical wording and include the explicit OR logic and the
trusted bot list so both descriptions match exactly.
| | `--max-turns` | `75` | Claude Code agent turns | | ||
| | `--max-responses` | `10` | Max threads to address per trigger | | ||
| | `--timeout` | `720` | Claude Code CLI timeout in seconds | | ||
| | `--timeout` | `900` | Claude Code CLI timeout in seconds | |
There was a problem hiding this comment.
Fix documented respond defaults to match respond.py.
These defaults are currently incorrect in docs. respond.py uses --max-turns=50 and --timeout=720 (not 75 / 900), so operators will be misled.
Suggested doc fix
-| `--max-turns` | `75` | Claude Code agent turns |
+| `--max-turns` | `50` | Claude Code agent turns |
...
-| `--timeout` | `900` | Claude Code CLI timeout in seconds |
+| `--timeout` | `720` | Claude Code CLI timeout in seconds |📝 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.
| | `--max-turns` | `75` | Claude Code agent turns | | |
| | `--max-responses` | `10` | Max threads to address per trigger | | |
| | `--timeout` | `720` | Claude Code CLI timeout in seconds | | |
| | `--timeout` | `900` | Claude Code CLI timeout in seconds | | |
| | `--max-turns` | `50` | Claude Code agent turns | | |
| | `--max-responses` | `10` | Max threads to address per trigger | | |
| | `--timeout` | `720` | Claude Code CLI timeout in seconds | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/scripts/testbot/README.md` around lines 184 - 186, Update the README
table values to match the actual defaults used by respond.py: change
`--max-turns` from `75` to `50` and `--timeout` from `900` to `720`; reference
the CLI flags in the docs and verify they match the defaults defined in the
respond.py entrypoint (flags `--max-turns` and `--timeout`) so operators see the
correct values.
02103f3 to
dbbbbcb
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/scripts/testbot/README.md (2)
5-5:⚠️ Potential issue | 🟡 MinorAlign schedule wording with the cron expression.
Line 5 says “weekly schedule,” but Line 47 defines a weekday cron (
0 6 * * 1-5). Please make both describe the same cadence.Also applies to: 47-47
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scripts/testbot/README.md` at line 5, The README line "Generates tests for low-coverage files on a weekly schedule" is inconsistent with the cron expression "0 6 * * 1-5"; update the human-readable wording to match the cron by changing the phrase to indicate a weekday cadence (e.g., "weekday schedule" or "runs Monday–Friday at 06:00") so the description and the cron "0 6 * * 1-5" are aligned; edit the sentence containing that phrase to reflect the weekday schedule.
85-85:⚠️ Potential issue | 🟡 MinorMake auto-approver authorization rules consistent across sections.
Line 85 documents team OR trusted bots, while Line 100 documents only org membership. Please use one consistent rule in both places.
Also applies to: 100-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scripts/testbot/README.md` at line 85, Update the auto-approver authorization text so both sections use the same rule: either require membership of the NVIDIA/osmo-dev team OR allow the trusted bots (svc-osmo-ci, github-actions[bot], coderabbitai[bot]); choose the intended policy and edit both occurrences (the sentence referencing "NVIDIA/osmo-dev" and the sentence at Line 100) to match exactly, including the same wording and bot list, so the README consistently states the same authorization criteria.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/scripts/testbot/README.md`:
- Line 5: The README line "Generates tests for low-coverage files on a weekly
schedule" is inconsistent with the cron expression "0 6 * * 1-5"; update the
human-readable wording to match the cron by changing the phrase to indicate a
weekday cadence (e.g., "weekday schedule" or "runs Monday–Friday at 06:00") so
the description and the cron "0 6 * * 1-5" are aligned; edit the sentence
containing that phrase to reflect the weekday schedule.
- Line 85: Update the auto-approver authorization text so both sections use the
same rule: either require membership of the NVIDIA/osmo-dev team OR allow the
trusted bots (svc-osmo-ci, github-actions[bot], coderabbitai[bot]); choose the
intended policy and edit both occurrences (the sentence referencing
"NVIDIA/osmo-dev" and the sentence at Line 100) to match exactly, including the
same wording and bot list, so the README consistently states the same
authorization criteria.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 1cd2621d-f277-46e9-8267-fa9216aaf619
📒 Files selected for processing (1)
src/scripts/testbot/README.md
Add system architecture diagram showing the LLM/harness/external API separation, per-workflow flow diagrams (generate and respond), and a comprehensive guardrails table covering all safety layers (tool allowlist, label gate, fork rejection, author association, required reviewer, org membership, commit sanitization, partial work discard). Also document the auto-approver workflow, the harness responsibilities split, and the file-based prompt structure (TESTBOT_RULES.md shared between generate and respond). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
dbbbbcb to
6b241b9
Compare
Summary
Rewrite
src/scripts/testbot/README.mdto document the full testbotarchitecture with diagrams.
separation (what Claude Code can do vs what the Python harness does)
guardrails → PR) and respond (comment → auto-approve → Claude → push → reply)
membership check
gate, fork rejection, author association, required reviewer, org
membership check, commit sanitization, GH013 detection, partial work
discard, test-file filter)
vs what the Python scripts handle
(
TESTBOT_RULES.mdshared between generate and respond)Docs-only change.
Issue #760
Checklist
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
ai-generatedPRs and tightened/testbottrigger semantics