Skip to content

Fixes AB#3789817 [ATS Perf] [android-complete] Update PR review skills to prevent performance regressions - #475

Open
Siddhi (siddhijain) wants to merge 2 commits into
masterfrom
siddhijain/3789817-memory-storage-review-attribution
Open

Siddhi (siddhijain) wants to merge 2 commits into
masterfrom
siddhijain/3789817-memory-storage-review-attribution

Conversation

@siddhijain

@siddhijain Siddhi (siddhijain) commented Oct 21, 2026 •

Copy link
Copy Markdown

Fixes AB#3789817

Summary

Add attribution tracking to the Copilot review-analysis tooling for the memory-storage/v1 review rule. This is the first PR in the rollout; the Common and Broker review-guidance changes will be reviewed separately.

  • Parse explicit rule, screen-completion, and guidance-revision markers while ignoring quoted or fenced examples.
  • Preserve rule and review IDs in collected and classified comments.
  • Generate rule_attribution.json with per-repository/PR comment counts, outcome buckets, screen confirmation, and guidance revisions.
  • Retain confirmed zero-finding reviews without treating missing markers as evidence that the screen ran.
  • Collect all review-summary pages and handle empty datasets without invalid JSON or division-by-zero errors.

Counts are deduplicated by repository and comment ID. They measure rule-associated comments, not unique defects or proof that the guidance caused an incremental discovery. Existing outcome-classification and force-push verification requirements remain unchanged.

Validation

& .\.github\skills\copilot-review-analyst\tests\rule-attribution.tests.ps1

Passed locally. The fixtures cover marker parsing, quoted/fenced examples, all outcome buckets, duplicate comment IDs, cross-repository PR numbers, zero-finding and legacy datasets, review-summary pagination, and metadata beyond the summary excerpt. GitHub transport is mocked; the tests do not require network access or credentials.

Scope

Only the five review-analysis guidance, script, and test files are included. Common/Broker module changes and unrelated workspace configuration are excluded.

Add explicit attribution metadata and per-PR outcome reporting, including confirmed zero-finding reviews and regression fixtures.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 61a54db1-5bab-4bf4-9f9a-9e478fa4a1dc
@siddhijain
Siddhi (siddhijain) requested a review from a team as a code owner October 21, 2026 16:05
@github-actions

Copy link
Copy Markdown

❌ Invalid work item number: AB#3789817

##. Work item number must be a valid integer.

Click here to learn more.

@siddhijain
Siddhi (siddhijain) requested a balanced review from Copilot October 21, 2026 16:08
@siddhijain Siddhi (siddhijain) changed the title Fixes AB#3789817: [ATS Perf] [android-complete] Update PR review skills to prevent performance regressions Fixes AB#3789817 [ATS Perf] [android-complete] Update PR review skills to prevent performance regressions Oct 21, 2026
@github-actions

Copy link
Copy Markdown

✅ Work item link check complete. Description contains link AB#3789817 to an Azure Boards work item.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Quoted Markdown continuation lines can be mistaken for attribution markers, producing false screening evidence and inflated counts.

1 open issue
What changed in this PR

Adds memory-storage/v1 attribution tracking to the review-analysis tooling; Common and Broker guidance changes remain separate.

Changes:

  • Parses markers and preserves rule, review, and guidance metadata.
  • Generates deduplicated attribution counts, including confirmed zero-finding reviews.
  • Adds summary pagination, empty-dataset handling, and mocked regression fixtures.
File Description
.github/​skills/​copilot-review-analyst/​tests/​rule-attribution.tests.ps1 Adds parsing, classification, and collector fixtures.
.github/​skills/​copilot-review-analyst/​SKILL.md Documents attribution reporting and limitations.
.github/​skills/​copilot-review-analyst/​scripts/​rule-attribution.ps1 Adds metadata-marker parsing.
.github/​skills/​copilot-review-analyst/​scripts/​final-classification.ps1 Exports attribution counts and handles empty results.
.github/​skills/​copilot-review-analyst/​scripts/​analyze.ps1 Collects metadata and paginates review summaries.

Review effort: Balanced

Comment thread .github/skills/copilot-review-analyst/scripts/rule-attribution.ps1
Track quoted paragraphs and code fences so quoted examples cannot supply rule, screen, or guidance metadata. Cover all three markers and valid top-level markers after quoted content.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 61a54db1-5bab-4bf4-9f9a-9e478fa4a1dc
Siddhi (siddhijain) added a commit to AzureAD/microsoft-authentication-library-common-for-android that referenced this pull request Oct 22, 2026
…nt performance regressions (#3321)

Fixes
[AB#3797297](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3797297)

## Summary

Update Common's actual Copilot code-review instructions and
repository-local review skill to check memory and storage performance
across every production-code PR, including new features outside
authentication and cache code.

- Review whole-data reads, repeated decryption/parsing, allocation and
copy amplification, collection complexity, cache reuse,
retention/invalidation, and expensive work on request/UI paths or under
locks.
- Trace affected callers and callees, including unchanged helpers newly
invoked or called more often, without auditing unrelated legacy code.
- Use the ATS improvements as concrete examples: reuse in-memory
account/credential records and avoid full token-cache decryption during
routine token saves.
- Require actionable evidence and preserve encryption, persistence,
authorization boundaries, synchronization, freshness, ordering, and
defensive-copy guarantees.
- Tag matching findings with `Review rule: memory-storage/v1` and record
screen completion/guidance revision in review summaries, including
zero-finding reviews.

## Attribution

The markers integrate with the measurement tooling in
[AzureAD/android-complete#475](AzureAD/android-complete#475),
which reports rule-associated comments and outcomes. This PR changes the
review guidance itself, not the analysis tooling or authentication
runtime. The performance screening is useful independently of the
reporting rollout.

## Scope and validation

Documentation-only changes to three files:

- `.github/copilot-instructions.md`
- `.github/skills/code-review/SKILL.md`
-
`.github/skills/code-review/references/performance-resources-dependencies.md`

No production code, dependencies, or cache behavior changes. Existing
false-positive suppression and comment-quality policies remain in place.
No runtime build or tests are required for these instruction changes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 61a54db1-5bab-4bf4-9f9a-9e478fa4a1dc

This branch has not been deployed

No deployments
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