Skip to content

ci: add copilot code-review instructions - #4093

Merged
juan-malbeclabs merged 3 commits into
mainfrom
ci/copilot-review-instructions
Jul 28, 2026
Merged

ci: add copilot code-review instructions#4093
juan-malbeclabs merged 3 commits into
mainfrom
ci/copilot-review-instructions

Conversation

@juan-malbeclabs

Copy link
Copy Markdown
Contributor

Summary of Changes

  • Adds .github/copilot-instructions.md (repo-wide review posture + cross-cutting rules) and eight path-scoped .github/instructions/*.instructions.md files, so Copilot's automated PR review applies the standards we already enforce by hand instead of the generic ones.
  • Scoped by applyTo glob so each review only loads what the diff touches: onchain programs, Rust SDK/builders/CLI, Go, E2E/QA, release workflows, the language SDKs and their golden fixtures, tests, and the changelog.
  • No effect on builds or runtime — these files are only read by Copilot.

Where the rules come from

  1. Our own reviews. I pulled every inline review comment on PRs merged in the last month — 421 comments across PRs controller: write config agent version info to ClickHouse #3578serviceability: address review comments on dzf_locked set-flags #4089 — and grouped the recurring findings. The threshold for a rule was that the same pattern appeared in two or more distinct PRs, or is already codified in an RFC or docs/cli-standard.md. Both sides of each thread were read, so suggestions that were deliberately declined (per-environment payload formats, the grace-period bound, hoisting at three copies) did not become rules.
  2. Published RFCs. RFC-1 contributes semver classification and the requirement to state deploy-ordering prerequisites; RFC-10 contributes the compatibility-window schema-evolution invariant (read both formats, keep writing the old one until min_compatible_version clears every reader); RFC-20 gates new CLI verbs.
  3. The code itself. Every factual claim in the rules — symbol locations, SDK_ATTACHES_PERMISSION_ACCOUNTS, the absent overflow-checks profile, split_trailing_permission semantics, the goccy/go-json decode path — was verified against the source rather than taken from the review text.

Rules were then pruned to those where a miss ships a real defect: silent feature disable, broadened authorization, a wire break, an unreclaimable reference count, a release tagged against the wrong commit, an alert that never fires. Style, naming, redundancy-removal and log-clarity nits were dropped, since noisy automated review is worse than none.

Notable gap this closes: nothing previously told an automated reviewer how to treat a pre-existing defect the diff widens — a framing that appears in six of the PRs sampled and shapes most human comments in the sample.

Testing Verification

Follow-up

Path-specific instructions are honored by Copilot code review only when custom instructions are enabled for the repo/org. Worth confirming on this PR's own Copilot review — it touches CHANGELOG.md, so the changelog rules should visibly apply. If they do not, only the repo-wide file is in effect.

@juan-malbeclabs
juan-malbeclabs force-pushed the ci/copilot-review-instructions branch from 9faee9d to cd64a76 Compare July 27, 2026 22:06
Adds a repo-wide .github/copilot-instructions.md and eight path-scoped
.github/instructions/*.instructions.md files so Copilot's automated PR
review applies the standards the team already enforces by hand.

Rules were extracted from the inline review comments on PRs merged over
the previous month, kept only where the pattern recurred across PRs or is
already codified, and verified against the code they describe. RFC-1
(semver classification, deploy-ordering prerequisites), RFC-10
(compatibility-window schema evolution), and RFC-20 contribute the
cross-cutting standards.

Also corrects the project overview in CLAUDE.md, which was the source of
three errors the new files initially inherited: DoubleZero is not "built
on Solana" (the Ledger is its own Solana-based cluster; Solana L1 is a
separate network carrying 2Z and the shred-subscription program), the
client side is users of all kinds rather than validators, and it is
service contributors, not fiber contributors. The smartcontract/ row of
the architecture table is corrected too: the programs there are
serviceability, telemetry, geolocation and record.

No effect on builds or runtime.
@juan-malbeclabs
juan-malbeclabs force-pushed the ci/copilot-review-instructions branch from cd64a76 to abd21e2 Compare July 27, 2026 22:09
Comment thread .github/instructions/solana-programs.instructions.md Outdated
Comment thread .github/copilot-instructions.md Outdated
Comment thread CHANGELOG.md Outdated
Comment thread .github/instructions/go.instructions.md Outdated
- Correct the Ledger program list to serviceability, telemetry, geolocation,
  record (there is no internet-latency program; InternetLatencySamples is an
  account type inside the telemetry program), in both the copilot instructions
  and the root guidance doc.
- Cite the defining file paths for build_with_permission and
  SDK_ATTACHES_PERMISSION_ACCOUNTS, which live outside the
  smartcontract/programs/** scope of that instructions file.
- Cover the shared config package: config/**/*.go in the Go scope and
  config/src/** in the Rust SDK/CLI scope; neither matched any scope before.
- Reference the PR number on the changelog entry.
@juan-malbeclabs
juan-malbeclabs enabled auto-merge (squash) July 28, 2026 20:07
@juan-malbeclabs
juan-malbeclabs merged commit fd25e89 into main Jul 28, 2026
37 checks passed
@juan-malbeclabs
juan-malbeclabs deleted the ci/copilot-review-instructions branch July 28, 2026 20:19
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.

2 participants