Repository navigation
pr-review skill: ask whether API changes were run on a real workspace - #7386
Merged
Merged
Conversation
Pass 9 of the pr-review skill reviews tests but never asks whether a change that sends new values to a Databricks API was verified against a real workspace. The fake server in libs/testserver only returns what its handlers were taught, so a test that records the request shows what the CLI sends, not what the API accepts. Add a bullet that asks for that check when no covering test is Cloud = true and the description does not mention a real-workspace run. Co-authored-by: Isaac <no-reply@databricks.com>
Collaborator
✅ Integration test reportCommit: f4a0a8d
|
pietern
approved these changes
Nov 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Adds one bullet to pass 9 (Tests) of
.agents/skills/pr-review/SKILL.md, placed after the bullet on whether each new or changed test shows the effect it claims. When a change sends a new field, value format or API call, and no covering test isCloud = trueand the description does not mention a real-workspace run, the review asks whether it was verified against one. Nothing else in the skill changes. No.nextchanges/fragment is added, matching #7344, which added the skill.Why
The fake server in
libs/testserveronly returns what its handlers were taught. A test that records the request, or a handler that echoes a field back, shows what the CLI sends, not what the API accepts. Pass 9 never asked whether such a change had been checked against a real workspace, and maintainers ask that question often.The gap showed up in a review of #7184 ("Support file selectors in pipelines run"). The change started sending new
refresh_selection_by_pathvalues (workspace paths, notebook extension stripped) to the pipelines StartUpdate API. The only coverage was the fake server echoing them back. The test was notCloud = true, and the PR description said no live-workspace run was done. The first review question was whether it had been run against a real workspace, and the skill as written would not have raised it.The bullet asks a question rather than requiring a cloud run. Every acceptance test still runs locally, and
Cloud = trueonly adds a run against a real workspace.Tests
./task wsruns, without--fix: 10787 checked, 0 failed..agents/rules/testing.md:Cloud = trueadds a cloud run and never removes the local one,Cloudis inherited (soout.test.tomlholds the effective value), and the fake server lives inlibs/testserver.This pull request and its description were written by Isaac.