Skip to content

chore: add copilot instructions files for PR review - #7470

Open
cylewitruk-stacks wants to merge 4 commits into
stacks-network:mainfrom
cylewitruk-stacks:chore/copilot-instructions-file-v1
Open

chore: add copilot instructions files for PR review#7470
cylewitruk-stacks wants to merge 4 commits into
stacks-network:mainfrom
cylewitruk-stacks:chore/copilot-instructions-file-v1

Conversation

@cylewitruk-stacks

Copy link
Copy Markdown
Contributor

This PR adds specific instruction files to be used by Copilot PR reviews.

These were largely generated by Copilot's own "generate instructions files" tooling, but I had Codex look through a few pages of recent PR's and analyze what reviewers react on (both positive and negative), what Copilot comments on and how "humans" react to its various comments, etc.

So, this is intended to be a first version to be iterated upon.

Copilot AI 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.

Pull request overview

Adds repository-level and path-scoped GitHub Copilot PR review instruction files, tailoring review focus for stacks-core’s main languages and CI automation with an emphasis on consensus safety, correctness, and compatibility.

Changes:

  • Introduces repo-wide Copilot PR review guidance in .github/copilot-instructions.md.
  • Adds language/subsystem-specific instruction files under .github/instructions/ for Rust, Clarity, TypeScript tests, Markdown docs, and GitHub automation.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
.github/copilot-instructions.md Establishes global PR review priorities and comment threshold for stacks-core.
.github/instructions/rust.instructions.md Adds Rust/core-node and consensus/chainstate-focused review checklist.
.github/instructions/clarity.instructions.md Adds Clarity smart-contract semantics and compatibility review checklist.
.github/instructions/typescript.instructions.md Adds guidance for reviewing TypeScript-based contract integration/property tests.
.github/instructions/markdown.instructions.md Adds correctness-sensitive documentation review checklist.
.github/instructions/github-automation.instructions.md Adds GitHub Actions/CI review checklist for workflows, actions, and scripts.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 30619669443

Coverage decreased (-0.01%) to 86.546%

Details

  • Coverage decreased (-0.01%) from the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • 148 coverage regressions across 27 files.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

148 previously-covered lines in 27 files lost coverage.

Top 10 Files by Coverage Loss Lines Losing Coverage Coverage
stacks-signer/src/v0/signer.rs 31 87.72%
stacks-node/src/neon_node.rs 18 83.14%
stackslib/src/net/download/nakamoto/download_state_machine.rs 9 89.74%
stackslib/src/net/download/nakamoto/tenure_downloader_unconfirmed.rs 9 74.13%
stackslib/src/net/server.rs 9 88.54%
stackslib/src/net/stackerdb/sync.rs 9 76.47%
stackslib/src/core/mod.rs 6 98.54%
stackslib/src/burnchains/bitcoin/network.rs 5 79.8%
stackslib/src/chainstate/nakamoto/mod.rs 5 84.77%
stacks-signer/src/v0/signer_state.rs 5 92.5%

Coverage Stats

Coverage Status
Relevant Lines: 231467
Covered Lines: 200325
Line Coverage: 86.55%
Coverage Strength: 19403976.59 hits per line

💛 - Coveralls

@dhaney-stacks dhaney-stacks left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not exactly sure how these will perform, but of course we can merge them and find out!

@federico-stacks federico-stacks 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.

I think this is a good starting point.

I've added a few minor comments, and I also wanted to share a couple of thoughts:

  • I wonder whether these Copilot instructions could somehow be picked up directly from the workspace, so developers using Copilot (or potentially any coding agent) would inadvertently use this "lightest" form of code review. You may have already tested this locally, but I just wanted to double-check.
  • Could we also add an instruction about inserting or updating the license header for .rs files?

- Check descriptions of RPC responses, configuration defaults, events, contract return values, fees, and credited amounts against the changed implementation.
- Verify changed commands, filenames, workflow names, flags, code examples, links, anchors, and sample output.
- Treat operator, upgrade, release, mining, signing, and security documentation as correctness-sensitive.
- Verify changelog entries describe the user-visible impact and use the repository's expected category and component.

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.

actually our PR changelog fragments have not .md but we have chagenlog.d/* path and stacks-signer/changelog.d/*

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was more referring to the final rendered CHANGELOG.md, but we could add a separate instructions file for changelog.d/** if we want to add some specific instructions for files there?

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.

maybe we could refer to chagenlog.d/README.md guidelines and probably the same entry Verify changelog entries describe the user-visible impact and use the repository's expected category and component. could apply also in this case?

The idea could be to tackle issue before composing the final CHANGELOG.md.

However, feel free to resolve this, if we want to evaluate improvements for later


# Markdown and repository-documentation review guidance

- Check descriptions of RPC responses, configuration defaults, events, contract return values, fees, and credited amounts against the changed implementation.

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.

Should we extend this to cover RPC yaml files in docs/rpc/**/*.yaml?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'd probably write a separate "RPC docs" instructions file for this that is scoped specifically to the related files and describing explicitly everything that needs to be checked tbh

Comment thread .github/instructions/github-automation.instructions.md
@cylewitruk-stacks

Copy link
Copy Markdown
Contributor Author
  • I wonder whether these Copilot instructions could somehow be picked up directly from the workspace, so developers using Copilot (or potentially any coding agent) would inadvertently use this "lightest" form of code review. You may have already tested this locally, but I just wanted to double-check.

Likely not; these file locations are pretty GH/Copilot-specific. What we'd likely need to make that work is having a root AGENTS.md which links the relevant files, and probably then a CLAUDE.md with an @AGENTS.md-include (since last time I checked, Claude still doesn't respect AGENTS.md).

  • Could we also add an instruction about inserting or updating the license header for .rs files?

Good idea 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants