Skip to content

Rewrite testbot README with architecture diagrams#901

Open
jiaenren wants to merge 1 commit into
mainfrom
jiaenr/testbot-readme-diagrams
Open

Rewrite testbot README with architecture diagrams#901
jiaenren wants to merge 1 commit into
mainfrom
jiaenr/testbot-readme-diagrams

Conversation

@jiaenren

@jiaenren jiaenren commented Apr 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

Rewrite src/scripts/testbot/README.md to document the full testbot
architecture with diagrams.

  • System architecture diagram showing the LLM / harness / external API
    separation (what Claude Code can do vs what the Python harness does)
  • Per-workflow flow diagrams for both generate (cron → targets → Claude →
    guardrails → PR) and respond (comment → auto-approve → Claude → push → reply)
  • Auto-approver section documenting the environment gate and team
    membership check
  • Guardrails table — 10 distinct safety layers (tool allowlist, label
    gate, fork rejection, author association, required reviewer, org
    membership check, commit sanitization, GH013 detection, partial work
    discard, test-file filter)
  • Harness responsibilities table — explicit split of what the LLM handles
    vs what the Python scripts handle
  • Prompt files section explaining the file-based prompt structure
    (TESTBOT_RULES.md shared between generate and respond)

Docs-only change.

Issue #760

Checklist

  • I am familiar with the Contributing Guidelines
  • New or existing tests cover these changes
  • The documentation is up to date with these changes

Test plan

  • Docs-only change; no code affected

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Clear two‑workflow overview (scheduled/manual generate + inline respond) and examples for inline review threads
    • Documented generation creating labeled ai-generated PRs and tightened /testbot trigger semantics
    • Documented automatic approver behavior limited to trusted org members and approved bots
    • Added guardrails (tool allowlists, test-file filtering, label/fork/author gating, env-based API access, commit sanitization, retry/timeout, partial-work discard)
    • Clarified respond defaults (max-turns 50→75, timeout 720→900)

@jiaenren jiaenren requested a review from a team as a code owner April 24, 2026 23:09
@coderabbitai

coderabbitai Bot commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: ff2cfc98-e70b-4d66-83e5-26fd18cc583b

📥 Commits

Reviewing files that changed from the base of the PR and between dbbbbcb and 6b241b9.

📒 Files selected for processing (1)
  • src/scripts/testbot/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/scripts/testbot/README.md

📝 Walkthrough

Walkthrough

README 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 ai-generated PRs, and a respond workflow that reacts to inline review comments beginning with /testbot from repo members; adds architecture, guardrails, and configuration details.

Changes

Cohort / File(s) Summary
Testbot Documentation
src/scripts/testbot/README.md
Complete rewrite describing a two-workflow model: Generate (weekday cron / manual dispatch → timestamped branch + [testbot] PR labeled ai-generated) and Respond (handles inline review comments whose first text is /testbot on ai-generated PRs, restricted to repo members excluding bots). Adds system diagrams, detailed workflow steps, auto-approver section gated by testbot-respond env and NVIDIA org membership, and extensive guardrails: tool allowlists, test-file-only filtering and reverts, label/fork/author gating, env-based API access, commit message sanitization, push retry with GH013 handling, partial work discard on timeout/max-turns. Updates respond workflow defaults (--max-turns 50→75, --timeout 720→900).

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
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 I hopped through docs and schemed a plot,
Two workflows stitched, a tidy lot,
Guardrails placed where bots should not,
I signed the PR with a thankful trot,
Tests and trees — automation hot!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change—a rewrite of the testbot README with architecture diagrams—which is the primary documentation update in the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jiaenr/testbot-readme-diagrams

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 001d490 and 02103f3.

📒 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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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]`)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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.

Comment on lines +184 to +186
| `--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 |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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.

Suggested change
| `--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.

@jiaenren jiaenren force-pushed the jiaenr/testbot-readme-diagrams branch from 02103f3 to dbbbbcb Compare April 25, 2026 00:08

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (2)
src/scripts/testbot/README.md (2)

5-5: ⚠️ Potential issue | 🟡 Minor

Align 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 | 🟡 Minor

Make 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

📥 Commits

Reviewing files that changed from the base of the PR and between 02103f3 and dbbbbcb.

📒 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>
@jiaenren jiaenren force-pushed the jiaenr/testbot-readme-diagrams branch from dbbbbcb to 6b241b9 Compare April 25, 2026 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant