Add reviewbot.py #4129

shreyas-goenka · 2025-12-10T18:05:24Z

Changes

Adds a Python-based review bot tool (tools/reviewbot.py) that uses Claude Code to assist with PR reviews.

Features:

Lists PRs with pending review requests for the current user
Interactive selection of which PR to review
Checks out the PR branch, runs Claude with review guidelines, then restores the original branch
Review guidelines defined in prompts/review.md covering:
- Code quality and style
- Test coverage with emphasis on acceptance tests
- Edge case analysis and runtime verification
- Documentation requirements
- Dependency security (pinned actions, trusted sources)

Usage

# Interactive mode - lists PRs and prompts for selection
python tools/reviewbot.py

# Direct mode - review a specific PR
python tools/reviewbot.py 4125

Why

An experimental tool to help out with initial PR reviews. Used it on: #4126

eng-dev-ecosystem-bot · 2025-12-10T18:26:30Z

Commit: e180b82

Run: 20237922373

	Env	🟨KNOWN	🔄flaky	💚RECOVERED	🙈SKIP	✅pass	🙈skip	Time
🟨	aws linux	7			1	380	646	25:40
🟨	aws windows	7			1	382	644	19:31
🟨	aws-ucws linux	3		4	1	523	531	23:04
🟨	aws-ucws windows	3	2	4	1	523	529	25:28
🔄	azure linux		2	1	3	379	644	31:41
💚	azure windows			1	3	383	642	18:18
💚	azure-ucws linux			1	3	520	529	29:01
💚	azure-ucws windows			1	3	522	527	29:02
💚	gcp linux			1	3	370	650	18:44
💚	gcp windows			1	3	372	648	15:31

12 interesting tests: 7 KNOWN, 4 flaky, 1 SKIP

	Test Name	aws linux	aws windows	aws-ucws linux	aws-ucws windows	azure linux	azure windows	azure-ucws linux	azure-ucws windows	gcp linux	gcp windows
🟨	TestAccept	🟨K	🟨K	🟨K	🟨K	💚R	💚R	💚R	💚R	💚R	💚R
🙈	TestAccept/bundle/resources/permissions	🙈S	🙈S	🙈S	🙈S	🙈S	🙈S	🙈S	🙈S	🙈S	🙈S
🟨	TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions	🟨K	🟨K	🟨K	🟨K	🙈S	🙈S	🙈S	🙈S	🙈S	🙈S
🟨	TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions/DATABRICKS_BUNDLE_ENGINE=direct	🟨K	🟨K	🟨K	🟨K
🟨	TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions/DATABRICKS_BUNDLE_ENGINE=terraform	🟨K	🟨K	💚R	💚R
🟨	TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/without_permissions	🟨K	🟨K	💚R	💚R	🙈S	🙈S	🙈S	🙈S	🙈S	🙈S
🟨	TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/without_permissions/DATABRICKS_BUNDLE_ENGINE=direct	🟨K	🟨K	💚R	💚R
🟨	TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/without_permissions/DATABRICKS_BUNDLE_ENGINE=terraform	🟨K	🟨K	💚R	💚R
🔄	TestAccept/bundle/resources/secret_scopes/permissions	✅p	✅p	✅p	🔄f	✅p	✅p	✅p	✅p	🙈s	🙈s
🔄	TestAccept/bundle/resources/secret_scopes/permissions/DATABRICKS_BUNDLE_ENGINE=terraform	✅p	✅p	✅p	🔄f	✅p	✅p	✅p	✅p
🔄	TestFilerWorkspaceNotebook	✅p	✅p	✅p	✅p	🔄f	✅p	✅p	✅p	✅p	✅p
🔄	TestFilerWorkspaceNotebook/pyNb.py	✅p	✅p	✅p	✅p	🔄f	✅p	✅p	✅p	✅p	✅p

Top 25 slowest tests (at least 2 minutes):

duration	env	testname
12:27	azure linux	TestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=direct
12:06	azure linux	TestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=terraform
11:10	aws linux	TestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=terraform
6:56	aws-ucws windows	TestAccept/bundle/resources/synced_database_tables/basic
6:27	azure-ucws windows	TestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=terraform
6:16	aws-ucws windows	TestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=direct
6:13	aws-ucws linux	TestAccept/bundle/resources/synced_database_tables/basic
5:40	gcp windows	TestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=direct
5:34	aws-ucws windows	TestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=terraform
5:27	gcp linux	TestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=direct
5:25	aws linux	TestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=direct
5:24	gcp linux	TestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=terraform
5:14	azure-ucws linux	TestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=terraform
5:13	gcp windows	TestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=terraform
5:03	aws windows	TestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=direct
4:54	azure-ucws linux	TestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=direct
4:16	azure-ucws windows	TestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=direct
3:02	azure-ucws windows	TestAccept/bundle/resources/synced_database_tables/basic
2:53	azure-ucws linux	TestAccept/bundle/resources/synced_database_tables/basic
2:33	aws-ucws linux	TestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=terraform
2:24	aws windows	TestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=terraform
2:17	azure-ucws linux	TestAccept
2:16	gcp linux	TestAccept
2:15	aws-ucws linux	TestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=direct
2:09	azure linux	TestAccept

denik · 2025-12-11T09:45:25Z

prompts/review.md

+## Review Checklist
+
+### Code Quality
+- [ ] Code follows the project's style guide and conventions


Which are? Should it have a link?

I figured claude would figure out the style guide by osmosis. We can add a dedicated style guide later one. Things like acceptance tests could use style consolidation.

prompts/review.md

denik · 2025-12-11T10:39:26Z

Good bot. Besides posting comments PRs, which is good for reviewing external contributions, can we just have a mode where it prints the review locally?

shreyas-goenka · 2025-12-11T13:52:43Z

@denik Printing the review locally is the default mode. You can at the end choose not to publish the review in the prompt. We can always add a flag later if you think that would make for a clearer interface.

shreyas-goenka · 2025-12-11T13:57:20Z

Here's a sample review today:

================================================================================
REVIEW TO BE PUBLISHED
================================================================================

Verdict: COMMENT
PR: #4130 - [DASH] Enable dashboard dataset_catalog/schema in direct deployment mode

--- Overall Comment ---
## Issues Found

1. **Missing edge case test coverage**: The acceptance test only covers the happy path with both fields set to non-empty values. Consider adding tests for:
   - Empty strings for dataset_catalog/dataset_schema
   - Only one field set (catalog without schema, or vice versa)
   - Changing these fields during an update operation
   - Not setting these fields at all (to ensure backward compatibility)

2. **No unit tests for direct deployment dashboard resource**: The file `bundle/direct/dresources/dashboard.go` lacks unit tests. Consider adding tests to verify:
   - DoCreate properly passes dataset_catalog/dataset_schema to the API
   - DoUpdate properly passes dataset_catalog/dataset_schema to the API
   - DoRead correctly clears these fields (sets to empty string)
   - RemapState properly handles these fields

3. **FieldTriggers clarity**: Consider explicitly adding `dataset_catalog` and `dataset_schema` to the FieldTriggers map with `ActionTypeSkip` for both local and remote diffs. This would make the behavior more explicit and clear that these are request-only fields that should not trigger drift detection.

## Suggestions

1. The implementation correctly treats dataset_catalog and dataset_schema as request-only parameters by clearing them in DoRead and responseToState, which prevents false drift detection.

2. The acceptance test properly verifies that these fields are sent to the API via the captured POST request.

3. Consider adding a comment explaining why dataset_catalog and dataset_schema are always set to empty strings in DoRead and responseToState (i.e., they are request-only parameters not returned by the GET API).

--- Inline Comments (2) ---

[1] bundle/direct/dresources/dashboard.go:126
    Consider adding a comment here explaining why dataset_catalog and dataset_schema are always set to empty strings. These are request-only parameters and are not returned by the GET dashboard API.

[2] bundle/direct/dresources/dashboard.go:323
    Consider explicitly adding dataset_catalog and dataset_schema to the triggers map with ActionTypeSkip for clarity:

```go
triggers["dataset_catalog"] = deployplan.ActionTypeSkip
triggers["dataset_schema"] = deployplan.ActionTypeSkip

This makes it explicit that these are request-only fields that should not affect drift detection.

================================================================================
Full review payload saved to: /Users/shreyas.goenka/repos/cli/cli-reviewbot/.reviews/pr-4130.json

Publish this review to GitHub? [y/N]:

Add reviewbot.py

a358e30

shreyas-goenka temporarily deployed to test-trigger-is December 10, 2025 18:05 — with GitHub Actions Inactive

=

7f46261

shreyas-goenka temporarily deployed to test-trigger-is December 10, 2025 19:16 — with GitHub Actions Inactive

commit publish_review.py

bc0d58e

shreyas-goenka temporarily deployed to test-trigger-is December 10, 2025 19:18 — with GitHub Actions Inactive

only write the review once

07c7853

shreyas-goenka temporarily deployed to test-trigger-is December 10, 2025 19:27 — with GitHub Actions Inactive

another iteration

4b94e15

shreyas-goenka temporarily deployed to test-trigger-is December 10, 2025 19:50 — with GitHub Actions Inactive

always publish review

1c0a461

shreyas-goenka temporarily deployed to test-trigger-is December 10, 2025 19:53 — with GitHub Actions Inactive

shreyas-goenka marked this pull request as ready for review December 10, 2025 19:54

shreyas-goenka requested review from andrewnester, anton-107, denik and pietern as code owners December 10, 2025 19:54

format

5f791d9

shreyas-goenka temporarily deployed to test-trigger-is December 10, 2025 20:00 — with GitHub Actions Inactive

denik reviewed Dec 11, 2025

View reviewed changes

fix typo

d1cdb06

shreyas-goenka temporarily deployed to test-trigger-is December 11, 2025 13:49 — with GitHub Actions Inactive

shreyas-goenka requested a review from denik December 11, 2025 13:53

add review and edit option

e180b82

shreyas-goenka temporarily deployed to test-trigger-is December 15, 2025 15:33 — with GitHub Actions Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add reviewbot.py #4129

Add reviewbot.py #4129

shreyas-goenka commented Dec 10, 2025 •

edited

Loading

Uh oh!

eng-dev-ecosystem-bot commented Dec 10, 2025 •

edited

Loading

Uh oh!

denik Dec 11, 2025

Uh oh!

shreyas-goenka Dec 11, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

denik commented Dec 11, 2025

Uh oh!

shreyas-goenka commented Dec 11, 2025

Uh oh!

shreyas-goenka commented Dec 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Add reviewbot.py #4129

Are you sure you want to change the base?

Add reviewbot.py #4129

Conversation

shreyas-goenka commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Usage

Why

Uh oh!

eng-dev-ecosystem-bot commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

denik Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

shreyas-goenka Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

denik commented Dec 11, 2025

Uh oh!

shreyas-goenka commented Dec 11, 2025

Uh oh!

shreyas-goenka commented Dec 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

shreyas-goenka commented Dec 10, 2025 •

edited

Loading

eng-dev-ecosystem-bot commented Dec 10, 2025 •

edited

Loading

shreyas-goenka Dec 11, 2025 •

edited

Loading