AI: Preserve original Perf Police flow with additive guidance

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Author: jahnvi480

.github/skills/mssql-profiler/SKILL.md

@@ -15,8 +15,10 @@ Read the [profiler guide](../../../profiler/README.md) and inspect the relevant
 1. Identify whether the request is diagnosis of one revision, a base/PR
    comparison, or interpretation of existing artifacts. Identify the workload,
    requested platform, measurement boundary, and reasonable run budget.
+
    State the suspected cause, the expected observation, and what would refute it.
    Prefer one bounded experiment using existing workloads over a new framework.
+
 2. Use the supplied checkout and its active development interpreter. Record the
    revision and dirty changes. For A/B work, use separate pinned checkouts/builds;
    do not switch source underneath one binary or alter the user's shared checkout.
@@ -203,27 +205,35 @@ enable boundary, or disabled interval can be dropped.
    Verify each process imports its intended package and native binary.
 2. Confirm the workload reaches the changed path. Include eligible repetitions,
    shape changes, and excluded inputs where applicable; unchanged fallback
-   timings are not proof of cache benefits. Read the actual benchmark registry
-   and worker, not just scenario names: ordinary SELECT fetching may never reach
-   a catalog-specific branch. Include unchanged-path controls for shared changes.
+   timings are not proof of cache benefits.
+
+   Read the actual benchmark registry and worker, not just scenario names:
+   ordinary SELECT fetching may never reach a catalog-specific branch. Include
+   unchanged-path controls for shared changes.
+
 3. Define cold setup, warmup, and the measured sequence before running. Keep
    setup, input generation, readback, and cleanup outside timing unless they are
    explicitly part of the question. Do not invalidate a prepared statement with
    housekeeping SQL on the same cursor between measured repetitions.
 4. Run matching profiling-enabled builds for attribution. Record actual call
    counts and changed-value correctness, not only duration. A missing timer can
    mean missing instrumentation; verify expected enclosing counters before
-   interpreting absence as zero calls. For a diagnostic experiment, vary one
-   suspected cause while retaining a matched control and the other conditions.
-   Check the predicted operation/phase change, not just a lower total time.
-   Keep diagnostic probes out of the final release-latency comparison.
+   interpreting absence as zero calls.
+
+   For a diagnostic experiment, vary one suspected cause while retaining a
+   matched control and the other conditions. Check the predicted operation/phase
+   change, not just a lower total time. Keep diagnostic probes out of the final
+   release-latency comparison.
+
 5. Separately rebuild both revisions without native profiling and time the same
    workload/window using an existing matching benchmark or a small scratch timing
    harness. `Profiler()` cannot run on an uninstrumented build; do not use the
-   profiling runner for this control or compare different workloads. Share setup,
-   values, timing, validation, and cleanup where possible; adapt only the
-   instrumentation. Report unavailable counters as unavailable, not zero or
-   results manufactured by a fake profiler.
+   profiling runner for this control or compare different workloads.
+
+   Share setup, values, timing, validation, and cleanup where possible; adapt
+   only the instrumentation. Report unavailable counters as unavailable, not
+   zero or results manufactured by a fake profiler.
+
 6. Counterbalance base/PR order over repeated rounds. Preserve individual
    observations and note contention. Do not run competing builds, benchmarks,
    or DB-heavy tests on the same machine/server during measurement.
@@ -295,13 +305,19 @@ Return a concise report with these fields, marking unavailable fields explicitly
 
 | Field | Required detail |
 | --- | --- |
-| Provenance | Base/head SHA and CI merge/run/job/attempt when applicable, dirty changes, interpreter/dependencies, native binary and ODBC provider identity, effective build/profiling flags, OS/architecture, SQL Server version |
+| Provenance | Base/head SHA, dirty changes, interpreter, native binary identity, effective build/profiling flags, OS/architecture, SQL Server version |
 | Workload | Scenario/script, rows/columns/parameters, value/type distribution, expected path and observed operation counts |
-| Window | Setup/warmup policy, measured operations, commit/fetch/cleanup inclusion, repetitions, process reuse, predecessor workloads and within-worker order |
+| Window | Setup/warmup policy, measured operations, commit/fetch/cleanup inclusion, repetitions and run order |
 | Results | Raw artifact paths, per-round observations, aggregate units, timings, deltas and variability |
 | Interpretation | What was observed, what remains uncertain, failures/skips, and platform/execution limits |
 
+Extend provenance with dependencies, actual ODBC provider identity, and any CI
+merge/run/job/attempt. Include the process reuse, predecessor workloads, and
+within-worker order described above in the window.
+
 Do not include credentials or private connection details in that report.
-Preserve raw evidence and clean up only run-owned temporary resources. Verify
-cleanup actually completed; a timeout or stop request alone does not prove it.
+Preserve raw evidence and clean up only run-owned temporary resources.
 Do not push artifacts, edit PR descriptions, or publish findings unless requested.
+
+Verify cleanup actually completed; a timeout or stop request alone does not
+prove it.

.github/skills/performance-code-review/SKILL.md

@@ -33,49 +33,74 @@ it is not a replacement for unrelated repository checks.
 Use this workflow to assess a change or plan a permitted scratch experiment;
 it does not authorize production edits or additional runtime activity.
 
-1. **Define the problem and the boundaries.** State the observable failure or
-   slow operation, affected inputs, expected behavior, and what would count as
-   improvement. Identify contracts and neighboring paths that must not change.
-2. **Establish a trustworthy baseline.** Reproduce through the supported public
-   API using the smallest relevant existing harness and the same input on the
-   candidate and its actual base. Reduce the workload without removing the
-   conditions that trigger the problem. Separate pre-existing behavior and
-   environment/setup failures from a change-specific effect.
-3. **Trace what actually runs.** Read changed units, callers, deleted checks,
-   and cleanup paths. Follow dispatch rather than assuming similarly named
-   operations share an implementation. For parameter work, distinguish
-   `execute`, `executemany`, array binding, overrides, and DAE. Establish loop
-   frequency, data flow, ownership, and timer boundaries before naming a cost.
-4. **Separate observations from explanations.** Record the raw symptom, likely
-   causes, and the smallest check that would distinguish them. For each proposed
-   cause, predict what should change and what should stay unchanged. A costly
-   pattern or a correlated timing change is a hypothesis, not a root cause.
-5. **Test one explanation at a time.** Keep a pinned control and vary one
-   relevant factor in an isolated experiment when execution is permitted.
-   Hold unrelated code, workload, settings, and timing boundaries fixed.
-   Retain adverse and inconclusive results. If a check cannot distinguish the
-   causes, refine the hypothesis rather than enlarge the patch or discard data.
-6. **Assess the smallest causal fix.** Connect the proposed change to the
-   demonstrated mechanism, not just a better aggregate score. Prefer a local
-   removal of redundant work, a valid invariant moved out of a loop, or a
-   narrowly eligible fast path over a broad rewrite or new cache. Treat an
-   untested alternative as a suggestion, not an equivalent fix.
-7. **Trace the full impact before removing work.** Identify every caller of
-   shared helpers and every correctness responsibility of the old path.
-   Preserve diagnostics, error propagation, ownership, invalidation, fallback,
-   and cleanup. Move a responsibility to a verified replacement before
-   removing its old implementation; do not trade correctness for fewer calls.
-8. **Check both the mechanism and unaffected behavior.** Inspect existing tests
-   and diff coverage. Verify the intended behavior change and unchanged
-   contracts against the baseline, with eligible, excluded, boundary, failure,
-   and recovery cases. Assert promised operation reductions as well as values.
-   Retain representative neighboring-path controls; a passing target workload
-   or coverage percentage does not establish that shared behavior is safe.
-9. **Decide from the evidence.** Check whether attribution supports the proposed
-   cause and whether matched release measurements justify the added complexity.
-   Drop disproved findings and explain corrections. Separate established
-   mechanisms, measured effects, unresolved alternatives, and untested paths;
-   do not force an optimization when the evidence remains inconclusive.
+1. Read changed units and their surrounding callers, including deleted safety
+   checks, tests, public Python entry points, and native cleanup paths.
+
+   For a failure or slow operation, state the affected inputs, expected behavior,
+   what improvement means, and neighboring contracts that must not change.
+
+2. Trace the current call graph. For parameter work, distinguish `execute`,
+   `executemany`, array binding, overrides, and DAE rather than assuming that
+   similarly named operations share a binder.
+
+   Follow actual dispatch, data flow, and timer boundaries before attributing
+   a cost.
+
+3. Locate each changed loop and its input-dependent frequency. Apply the cost
+   model and pattern checklist below; record concrete growth before zone labels.
+
+   Separate the observed cost from its explanation: a costly pattern or
+   correlated timing change is not a demonstrated root cause.
+
+4. Trace ownership, Python contracts, error returns, and invalidation end to end.
+   Treat initial findings as hypotheses, not established defects.
+
+   For each suspected cause, identify the smallest check that could refute it
+   and predict what should change and what should stay unchanged. Trace every
+   shared-helper caller; move diagnostics, error propagation, ownership,
+   invalidation, fallback, and cleanup to a verified replacement before
+   removing old work.
+
+5. Reproduce candidate defects with the smallest relevant existing harness.
+   Compare the same input on the PR and its actual base. Check public API paths,
+   not only direct calls into internal wrappers.
+
+   In a permitted isolated experiment, keep a pinned control and vary one
+   relevant factor; hold unrelated code, workload, settings, and timing
+   boundaries fixed. Reduce the workload without losing the trigger, and
+   distinguish setup, fixture, and harness failures from driver behavior.
+
+6. Inspect available diff coverage and existing regression tests. Prioritize
+   uncovered failure, fallback, boundary, and teardown paths. Coverage percentage
+   alone neither proves safety nor establishes a defect.
+
+   Assert promised operation reductions as well as values. Retain representative
+   neighboring-path controls and eligible, excluded, boundary, and recovery cases;
+   a passing target workload alone does not establish shared-path safety.
+
+7. Evaluate performance evidence and simplifications using the rules below.
+   Do not ask for a migration or dependency PR without a demonstrated call-path
+   dependency; an exceptional DAE path does not justify rewriting all batching.
+
+   Connect the smallest proposed fix to the demonstrated mechanism, not only
+   an aggregate score. Prefer removing redundant work, moving a valid invariant
+   out of a loop, or a narrowly eligible fast path over a rewrite or new cache.
+   Use matched release measurements to judge added complexity; an untested
+   alternative is a suggestion, not an equivalent fix.
+
+8. Challenge each finding: what evidence would disprove it, is it pre-existing,
+   is the caller supported, and does the suggested fix preserve every invariant?
+   Drop disproved claims and explain material corrections plainly.
+
+   Retain adverse and inconclusive results. If a check cannot distinguish causes,
+   refine the hypothesis rather than enlarge the patch or discard data. Do not
+   force an optimization without supporting evidence.
+
+9. Report only actionable findings. Separate correctness, measured performance,
+   unverified concerns, and code-organization suggestions.
+
+   Scope claims to established mechanisms and measured effects; identify
+   unresolved alternatives and untested paths.
 
 Before concluding, answer these questions from the evidence: does the change
 address the ask; is its scope justified; does it regress other supported paths;
@@ -159,9 +184,11 @@ dispatch path. Never flag it solely because its type name appears in a loop.
   preserve the expected exception behavior.
 - Keep Python code-point counts, UTF-16 code units, and encoded bytes distinct.
   Astral characters require two UTF-16 units. Size the final normalized/formatted
-  value, including the chosen encoding and any required terminator. Use the same
-  discipline in SQL fixtures: verify that boundary values fit the declared type
-  and reach the intended driver path before interpreting a failed assertion.
+  value, including the chosen encoding and any required terminator.
+
+  Use the same discipline in SQL fixtures: verify that boundary values fit the
+  declared type and reach the intended driver path before interpreting a failed
+  assertion.
 - Inspect actual consumers of new metadata. Populating an unused length field
   may be a correctness prerequisite, but it is not evidence of a current
   customer-visible crash fix or a delivered speedup.
@@ -230,9 +257,11 @@ concurrent-lifecycle design.
   adding another framework; more validation machinery is not performance evidence.
 - Require evidence for a performance optimization's payoff. Safety ownership,
   protocol validation, and error handling do **not** need a throughput gain to
-  justify their existence. Classify added code by responsibility before proposing
-  cuts; neither fewer lines nor fewer calls proves lower latency. Reject a
-  measured slowdown rather than preserving machinery because it looks cheaper.
+  justify their existence.
+
+  Classify added code by responsibility before proposing cuts; neither fewer
+  lines nor fewer calls proves lower latency. Reject a measured slowdown rather
+  than preserving machinery because it looks cheaper.
 - Before calling a guard redundant, prove the state unreachable through all
   supported callers. Distinguish a genuinely fixed bound from user-supplied data.
 - A proposed cut must preserve diagnostics, lifetime, fallback, and public API
@@ -288,9 +317,11 @@ do not start an unrelated benchmark merely because a review is running.
   loader can legitimately load the `.so`/`.pyd`; its own path is not sufficient
   provenance. Discard mislabeled or logging-contaminated runs.
 - Use Release/`-DNDEBUG` for comparisons. Debug-only assertion costs are not
-  release regressions. Verify effective configuration and compiler flags as
-  described in the [profiler skill](../mssql-profiler/SKILL.md#build-and-verify-instrumentation),
-  not a build's label. Do not copy another PR's numbers onto a later change.
+  release regressions. Do not copy another PR's numbers onto a later change.
+
+  Verify effective configuration and compiler flags as described in the
+  [profiler skill](../mssql-profiler/SKILL.md#build-and-verify-instrumentation),
+  not a build's label.
 - Counterbalance base/PR order, retain raw per-round observations, and avoid
   competing benchmarks or DB-heavy tests on the same machine/server. Alternation
   reduces order bias; it does not remove all contention or server-state effects.