Repository navigation
Add author and review skills for new bundle resources - #7360
Draft
janniklasrose wants to merge 2 commits into
Draft
janniklasrose wants to merge 2 commits into
janniklasrose wants to merge 2 commits into
Conversation
Two agent skills under .agents/skills: author-new-resource walks an API readiness gate, the wiring, generators, tests and PR for a new resource type with a verification command per phase; review-new-resource checks such a PR from design down to tests in fixed passes. The dresources README and adapter comments now state the KeyedSlices field requirement, the no-update and partial-update rules, the fake server scope and the invariant matrix step. Co-authored-by: Isaac <no-reply@databricks.com>
Collaborator
Integration test reportCommit: 8e41b44
Top 3 slowest tests (at least 2 minutes):
|
Move the adapter tests to the generate phase, since they need the generated rules; correct the state-fixture row (the state tests assert full coverage); note the bind test's per-type id, the refschema first-run report, the PyDABs codegen failure mode, and where identity, create-only markers and declared error codes live in a proto. Co-authored-by: Isaac <no-reply@databricks.com>
This branch has not been deployed
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
Two agent skills under
.agents/skills/, plus the documentation they rely on:author-new-resourceguides an engineer who owns an API but does not know this codebase through adding a bundle resource type. It starts with an API readiness gate (identity, keyed repeated fields, round-tripping inputs, annotations, update semantics, long-running operations, delete and read-after-create behaviour, launch stage) whose failing checks are sent back to the API team with the AIP reference, then walks the wiring, generators, tests and PR in phases that each end in a verification command. References carry the wiring checklist with the test that enforces each row, the testing playbook with proof-it-ran commands, and a pitfalls list.review-new-resourcereviews such a PR in fixed passes, design first: scope, API shape and plan semantics, adapter contract, wiring completeness, fake-server scope, tests and whether they ran, generated files, user-facing text. Same print-only output format as thepr-reviewskill.bundle/direct/dresources/README.mdnow states theKeyedSlicesfield requirement, the no-update and partial-update rules, the fake-server scope, the invariant matrix step, and the actual contract for the id returned fromDoCreate. Three interface comments inadapter.gosay the same in one sentence each.AGENTS.mdand the dresources rule point at the skills.Both skills cite code paths and patterns only.
Why
Adding a resource should be one PR with little pushback. The resource README stopped where the hard decisions start, and review attention tended to reach design questions late. The gate moves API-shape problems to the API, where they are cheapest to fix, and the checklists make the forgettable steps mechanical.
Tests
Backtested outside the repo against eleven past resource additions and one open PR, each at the commit that received the most review, with the reviewer agent given read-only git and no access to the PR. Human review comments were split into a tuning set (read while writing the skill) and a held-out set (sealed until an LLM judge scored them). On the held-out set the review skill reproduced about half of the substantive human asks by weighted recall (Opus 0.57, Sonnet 0.54 over the 28 asks still applicable at the reviewed commits), and most of its additional findings were judged plausible and valuable; the judges found one contradicted claim for Opus and seven for Sonnet. The author skill's dry-runs at the pre-PR commits returned STOP on the one API whose shape later needed workarounds, and no STOP with the expected asks on the others. One full authoring run from a pre-PR commit, blind to later history, produced a resource within the expected size with
./task test,./task lintand./task generate-checkpassing, and the same design decisions as the merged implementation except two deliberate, documented deviations.Docs-only change in this PR;
./task wsandgo build ./...pass.This pull request and its description were written by Isaac.