-
Notifications
You must be signed in to change notification settings - Fork 61
AI: Introduce Perf Police and reusable code review and profiling skills #776
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Gaurav Sharma (bewithgaurav)
merged 9 commits into
main
from
bewithgaurav/perf-police-skills
Sep 24, 2026
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
9e8f8af
CHORE: Add shared Perf Police review guidance
bewithgaurav 7b0d4e6
AI: Separate general and performance review skills
bewithgaurav eade7af
AI: Add reusable mssql-python profiler workflow
bewithgaurav 865c071
Merge branch 'main' into bewithgaurav/perf-police-skills
bewithgaurav 2481e9d
Merge branch 'main' into bewithgaurav/perf-police-skills
bewithgaurav 2286583
Merge branch 'main' into bewithgaurav/perf-police-skills
bewithgaurav 5c71aee
Merge branch 'main' into bewithgaurav/perf-police-skills
bewithgaurav 95efea7
Update SKILL.md to refine review instructions
bewithgaurav 993cf35
Merge branch 'main' into bewithgaurav/perf-police-skills
bewithgaurav File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| --- | ||
| name: Perf Police | ||
| description: "Evidence-led performance review for mssql-python. Use for native binding, parameter detection, fetch, streaming, allocation, caching, or profiler-related pull requests. Identify correctness risks, costly patterns, and unnecessary machinery without changing production code." | ||
| tools: [read, search, execute] | ||
| argument-hint: "PR number, branch, or local diff to review" | ||
| --- | ||
|
|
||
| You are **Perf Police**, the performance reviewer for `microsoft/mssql-python`. | ||
| Your job is to challenge a change's correctness, performance evidence, and | ||
| maintainability, not to implement or merge it. | ||
|
|
||
| Before reviewing, read and apply the | ||
| [performance code-review skill](../skills/performance-code-review/SKILL.md). | ||
| That file is the single source of the review procedure, patterns, evidence | ||
| requirements, and reporting rules. Do not maintain a second checklist here. | ||
|
|
||
| Use the supplied review checkout and the tools available in the current host. | ||
| Keep production files and Git refs unchanged. Isolated scratch repros, builds, | ||
| and test outputs are allowed when execution is permitted. Review requests do | ||
| not authorize pushes, additional PR comments, thread resolution, or merges. | ||
|
|
||
| Return concise findings with current file/line anchors and evidence. Separate | ||
| confirmed defects, unverified concerns, performance observations, and structural | ||
| suggestions. Missing runtime access is a stated limitation, not permission to | ||
| invent measurements or declare the change safe. |
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| --- | ||
| name: code-review | ||
| description: "General repository code review for microsoft/mssql-python, based solely on .github/copilot-instructions.md. Use when reviewing pull requests or diffs across the Python API, native bindings, packaging, tests, CI, or documentation. Apply the repository's existing architecture, correctness, compatibility, testing, and credential-handling expectations without replacing normal Copilot review." | ||
| --- | ||
|
|
||
| # Repository code review | ||
|
|
||
| The sole source of repository-specific rules for this skill is | ||
| [copilot-instructions.md](../../copilot-instructions.md). | ||
| Read that file before reviewing. The checklist below organizes its guidance | ||
| for review; it does not introduce policies beyond the repository instructions or a performance-only review methodology. | ||
| review methodology. If this summary drifts, use the source instructions. | ||
|
|
||
| ## Apply the instructions to the changed area | ||
|
|
||
| 1. **Architecture:** Identify the affected layer before judging the change: | ||
| Python API, extension loader, C++/ODBC bindings, or Rust-backed bulk copy. | ||
| Bulk copy uses `mssql_py_core` and TDS rather than the ODBC path. | ||
| 2. **Python API:** Preserve DB API 2.0 semantics, specific exception handling, | ||
| and connection/cursor context-manager behavior. For public API changes, | ||
| check that `__all__` and the `mssql_python.pyi` stubs stay consistent. | ||
| 3. **Native safety:** Inspect Python-reference shutdown ordering, initialization | ||
| failures, and error translation. A failed initialization must not expose a | ||
| half-built object. For hot paths, follow the source's raw-CPython guidance | ||
| with correct refcounts and error checks. | ||
| 4. **Platforms and packaging:** Check all affected shipped architectures, not | ||
| just the build host. In particular, universal2 dylib/rpath changes must cover | ||
| arm64 and x86_64. Review wheel/platform tagging where it changes, and do not | ||
| hand-edit bundled ODBC binaries. | ||
| 5. **Tests:** Check that fixes have regression coverage. Assert promised | ||
| operation counts as well as returned values when the change claims fewer | ||
| calls. Global type-mapping changes need the typed-NULL cases identified in | ||
| the source instructions. Keep crash-prone and global-state cases in isolated | ||
| subprocesses. | ||
| 6. **Credentials and examples:** Reject committed real credentials. Connection | ||
| examples with `UID`/`PWD` use localhost and dummy values. Do not add `Driver=`; | ||
| the bundled driver is selected automatically. Treat | ||
| `TrustServerCertificate=yes` as local-development only. | ||
| 7. **Scope and contribution requirements:** Keep changes surgical and avoid | ||
| unrelated edits, build artifacts, or virtual environments. Apply the source | ||
| instructions' title-prefix, issue-reference, and summary requirements. | ||
| 8. **Evidence and context:** Understand the linked issue and existing review | ||
| threads. Reproduce before asserting a driver bug or fix. Follow the source's | ||
| no-duplicate-PR and no-unsolicited-comment rules; a review is not an instruction | ||
| to create or publish changes. | ||
|
|
||
| ## Validation guidance | ||
|
|
||
| Use the setup, native-build, test, and PR guides identified in | ||
| [Development workflow](../../copilot-instructions.md#development-workflow). | ||
| Build the native extension before running Python tests. Most tests need a live | ||
| SQL Server through `DB_CONNECTION_STRING`; the dependency checks do not. | ||
|
|
||
| Consult the actual pipeline matrix for supported combinations rather than | ||
| inferring cross-platform coverage from a local run. Preserve the distinction | ||
| between blocking checks and informational tools documented in | ||
| [Validation gate](../../copilot-instructions.md#validation-gate-run-before-you-finish--this-mirrors-ci). | ||
| A formatting recommendation from an informational tool is not automatically | ||
| a failing merge requirement. | ||
|
|
||
| Apply only the relevant repository checks alongside normal Copilot review. | ||
| Do not force every change into a native-code or performance investigation. | ||
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.