Skip to content

Release: Copilot usage reports, Copilot audit field capture, Teams add-on deprecation, CI enforcement - #283

Open
sambetts wants to merge 46 commits into
mainfrom
dev
Open

Release: Copilot usage reports, Copilot audit field capture, Teams add-on deprecation, CI enforcement#283
sambetts wants to merge 46 commits into
mainfrom
dev

Conversation

@sambetts

@sambetts sambetts commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Release candidate: devmain.

Base main @ 5ff6327 → head dev @ a936caf (merge-base 2ada302). main's extra commits are prior release merges and contribute no tree difference, so two-dot and three-dot diffs are identical.

31 squash-merged PRs, 210 files, +37,032 / −3,531, 44 commits (the remaining commits are older-style branch merges and a few direct pushes). This body is kept current as PRs land; it describes the release as it now stands rather than as it was first opened. Last updated for #314.

Closes #270, closes #271, closes #272, closes #276, closes #278, closes #286, closes #290, closes #295, closes #297, closes #302, closes #285, closes #287, closes #294, closes #296, closes #312.

(The first ten were closed by hand on 2026-08-21 once their fixes reached dev; the keywords are kept so the association is recorded on merge to main. main is the default branch, so these fire here — they would have been inert in the dev-targeted PRs.)


1. What is in this release

PR Change Schema Config schema
#262 Copilot audit import persists parsed-but-dropped fields 5 new tables + columns on 4 existing
#260 Imports the three Graph Microsoft 365 Copilot usage reports 3 new tables 1.9.0 → 1.10.0 (additive)
#279 Deprecates Teams add-on / app-install tracking, removes the import Conditionally drops 3 tables 1.10.0 → 2.0.0 (BREAKING)
#291 Copilot AI interaction history import (opt-in, statistics only) 10 new tables 2.0.0 → 2.1.0 (additive)
#292 Copilot licence adoption tool (find unused seats / unlicensed heavy users)
#263 Stops mailbox-less users flooding App Insights with 404 exceptions
#264 #265 #266 #267 #268 #269 Telemetry service: EasyAuth fix, App Insights instrumentation, dashboard rework, tests + CI/CD, Node 20 removal
#288 Telemetry service integration tests + an unenforced authorisation scope, fixed
#289 Azure configuration drift detection (maintainer CI only)
#280 #282 CI: required checks now actually run on PRs; EF migration snapshot guard
#284 Release review fixes (manual-script prerequisite, O(N²) row filter)
#293 Multi-model review fixes: adoption period fan-out, drift-summary redaction, import cadence gate, back-off, dedup re-key and 7 more
#298 Copilot interaction history: UserGroupsFilter becomes an optional narrowing rather than a precondition; unnarrowed user selection moved into SQL
#299 Tests.FakeDataGen: generates Copilot prompt history and the new audit tables; stronger upgrade-from-stable rehearsal
#301 Reverts the profiling weekly-boundary change out of this release (the underlying bug is tracked in #300, with an [Ignore]d repro test kept in place)
#303 Both Copilot adoption trend series computed in a single pass instead of two identical scans of copilot_chatsaudit_events (#295)
#304 Retention bound added to the three Copilot usage-report tables, which were growing forever, plus per-user purge for the activity log (#286)
#305 GitHub API rate limiting no longer breaks CI or real installs; a 403 is explained instead of looking like a permissions error (#302)
#306 Copilot interaction history: wildcard scope and group-discovery caps no longer weaken the Graph call brake; truncated scope reported as a failure (#297)
#307 RC fixes: Graph paging failures no longer recorded as successful empty imports (#285); accessed-resource de-dup keeps distinct actions instead of collapsing them (#287); interaction-history de-dup bounded to a time window (#294); keyword_id index for the orphan-keyword cleanup (#296) 3 index migrations
#314 Fixes fractional chart axis labels (a 0–1 sentiment axis rendered as "1, 1, 0, 0, 0, 0") and corrects the sentiment scale in the chart description — it is a 0.0–1.0 positive-sentiment confidence, not −1 to +1
#313 Reports: Copilot prompt insights. Adds a word cloud of the most common extracted prompt phrases, a weekly prompt-sentiment line chart and a prompt-language breakdown to the Copilot tab — shown only when cognitive services are configured (#312)
#310 Makes the activity-report phase fail per report rather than all-or-nothing, and stops #309's strict paging aborting install verification. Also closes two persisted paths that still wrote Graph request URLs into the import-log error column, and hardens the de-dup fail-open sentinel against a SqlDateTime overflow
#309 Release-critique follow-ups: the legacy daily usage reports (SharePoint/Teams/OneDrive/Yammer/Exchange) had the same #285 hole and now fail loudly instead of recording an empty day; a fail-closed de-dup guard made fail-open; Graph URLs kept out of the persisted error column; three documentation errors
#308 Engineering notes and PR template: corrects two pieces of guidance disproven by this release's own measurements, reconciles a contradiction in the schema-performance gate, and adds migration/config checklists
Combined O365 + Copilot profiling data generator (Tests.FakeDataGen), pushed directly to dev

Verified against the diff, not the PR text

Claim Verification
CONFIG_VERSION ends at 2.1.0 BaseSolutionInstallConfig.cs on dev reads "2.1.0"
7 migrations, each with a .manual.sql 7 .cs + 7 matching .manual.sql in the main...dev diff (1:1, verified by name)
Create DB.sql needs no change Unchanged in the diff — and it contains no Copilot tables at all, not even copilot_chats from 2024. It is a legacy base schema; every Copilot table has always come from a migration. This is verified, not an omission.
Config-schema delta Added GraphCopilotUsageReports, CopilotInteractionHistory; removed GraphUserApps

The break is the removal of ImportTaskSettings.GraphUserApps. Older configs still load — the property is simply ignored.


2. Migrations

Seven new migrations — four additive (bar the conditional drop in DeprecateTeamsAddons), plus three index migrations from #307. Each has its own .manual.sql.

Note the diff also shows an eighth migration file, 202607231700001_CoverCopilotAccessedResourceDedup.cs, as modified. That migration already shipped in the previous stable release; #309 changed only its XML doc comment (to record that #307 supersedes its de-dup tuple, and that upgrading from before it builds the index twice). Its Up_Sql / Down_Sql are byte-identical, so no customer database is affected and no new manual script is required. Verified: the diff for that file contains no non-comment lines.

Migration Effect Cost
202608190622001_CopilotDroppedAuditFields 5 Copilot tables + 9 nullable columns, and two FK indexes on copilot_event_accessed_resources Tables/columns are metadata-only and instant. The index pair is one of the two heavy steps in the release (the other is the dedup rebuild below, on the same table).
202608190725064_AddCopilotUsageReports 3 usage-report tables Metadata-only; only lock on existing data is a brief schema lock on dbo.users for the FK
202608191533567_DeprecateTeamsAddons Drops the 3 Teams add-on tables only when all three are empty EXISTS test, not COUNT(*), so O(1)-ish even on a billion-row table
202608200600001_AddCopilotInteractionHistory 10 empty interaction-history tables + indexes Purely additive: no table altered, no rewrite, no blocking lock. Seconds at any size
202608210700001_WidenCopilotAccessedResourceDedupIndex Rebuilds IX_copilot_event_accessed_resources_dedup with action_id + list_item_unique_id_id as key columns (#287) Drop + rebuild on an existing, potentially large table. 5.9 s / 83 MB at 2M rows; ~30 s at 10M, ~5 min at 100M. Same table and same window as the CopilotDroppedAuditFields index pair
202608210700002_IndexCopilotInteractionKeywordsByKeyword (keyword_id, interaction_id) on the Copilot keyword link table (#296) Table is new in this release → empty on upgrade → instant
202608210700003_IndexCopilotInteractionsDedupWindow (session_id, created_utc) INCLUDE (graph_interaction_id) (#294) Table is new in this release → empty on upgrade → instant

Gate classification (schema-performance rule)

Stating this explicitly, because an unclassified migration reads as an unmeasured one:

Migration Class Measurement
AddCopilotUsageReports Additive; both indexes are functional upsert keys Not required — supplied anyway (§3)
AddCopilotInteractionHistory Additive; all 19 indexes on new empty tables Not required — retro-measured in round 2
DeprecateTeamsAddons Removal only, conditional on emptiness Not required
CopilotDroppedAuditFields Additive tables/columns + 2 FK indexes on an existing large table Judgement call. Both columns are NULL on 100% of existing rows (no backfill), so there is no "before" query. Build time + storage supplied, plus a no-regression check (insert_junction 328 → 314 ms). Classified not performance-motivated; needs conscious sign-off
WidenCopilotAccessedResourceDedupIndex Correctness-motivated, with a measured performance defence ✅ 500-row batch 1,623 → 1,622 reads / 2 ms; 20,000-row batch 63,889 → 63,883 reads, 85 → 94 ms; seek retained at both. Rejected INCLUDE variant documented (fewer reads, 5.5x slower)
IndexCopilotInteractionKeywordsByKeyword Performance-motivated ✅ per-user purge 11,256 → 137 reads, 155 → 16 ms, Scan → Seek; bulk sweep 797 → 437 ms (reads unchanged, reported honestly)
IndexCopilotInteractionsDedupWindow Performance-motivated ✅ 288,000 → 12,467 rows loaded, 5,484 → 3,050 reads, 45 → 6 ms, seek retained. Bounding without the index measured worse than not bounding (337 ms) — code and migration must ship together

Non-migration SQL in gate scope: common_upsert_copilot_agents.sql carries a performance-motivated de-dup re-key (1.8x fewer reads / 5.7x faster, §8) and, from #307, the full-tuple de-dup correctness fix.

Maintenance window — the only part that can cause an outage

CopilotDroppedAuditFields builds IX_..._action_id and IX_..._list_item_unique_id_id on copilot_event_accessed_resources, one of the larger tables on a Copilot-heavy tenant. Measured at synthetic scale (offline build, buffer pool dropped, medians of 3) on a 3,000,000-row junction table:

Index Build time Size
IX_..._action_id 2.5 s 40.7 MB
IX_..._list_item_unique_id_id 3.1 s 40.7 MB
pair 5.6 s 81 MB

The build attempts ONLINE on Enterprise (EngineEdition 3), Azure SQL DB (5) and Managed Instance (8), falling back to offline elsewhere. On Standard/Express/Web the build is offline and locks the table, so those tenants should upgrade in a maintenance window with the importer stopped.

copilot_event_accessed_resources is now touched twice — the FK index pair above, and the dedup index rebuild in 202608210700001_WidenCopilotAccessedResourceDedupIndex. The other five migrations create new/empty tables and need no window.

Rows in copilot_event_accessed_resources FK pair Dedup rebuild Combined (linear) Plan for
1M ~2 s ~3 s ~5 s ~5 s
10M ~19 s ~30 s ~50 s 1–2 min
100M ~3 min ~5 min ~8 min 15–25 min

The "plan for" column is deliberately more pessimistic than the arithmetic. The per-million figures are linear extrapolations from a 2M-row measurement, and index builds are O(n log n) with a sort that will spill to tempdb well before 100M rows — so budget 2–3x the linear estimate and make sure tempdb has room.

Storage. The FK pair adds ~27 MB per million rows. The widened dedup index is 83 MB per 2M rows gross, but it replaces the existing 6-key index (~69 MB), so the net addition is only ~7 MB per million. At 100M rows: ~3.4 GB net. Two things that figure does not cover, and which an admin must have free anyway:

  • Peak space during the rebuild — old index, new index and sort space coexist. Budget roughly +4 GB transient at 100M rows.
  • Transaction log growth — in FULL recovery a large index build generates multi-GB of log. Check log space and backup cadence before starting.

Apply the database migrations before or with the binaries — never run the new importer against a pre-migration schema. The merge SQL is an embedded resource in the importer and the bounded de-dup read is C#, but both of their supporting indexes arrive via migration. Because the manual-SQL path deliberately allows schema and binaries to be applied separately, "new binaries, old schema" is a reachable state — and by this release's own measurements it is slower than the version being upgraded from: the full-tuple merge against the un-widened index measured 530 ms vs 85 ms (5.6x slower, seek lost), and the bounded de-dup read without its index measured 337 ms vs 45 ms (7.5x slower, seek lost). Both recover completely once the migrations are applied.

2.1 Three snapshot defects found and fixed before merge

#260, #279 and #291 were each authored before the migrations that ended up preceding them, so their .resx model snapshots predated the branch they merged into. That is issue #271 exactly, and it is why #282 exists.

Migration Target before after Entity sets
AddCopilotUsageReports 53,548 55,764 128 → 133
DeprecateTeamsAddons 52,488 54,332 127 → 130
AddCopilotInteractionHistory stale (pre-#260/#279 model) regenerated 130 → 140

Reproduced before fixing, on a scratch merge with the LocalDB test database dropped and the solution rebuilt:

Tree Migration tests
dev alone 27 passed
dev + #260 as authored 18 failed, 8 passed — There is already an object named 'copilot_event_accessed_resource_actions' in the database.
dev + #260 repaired 27 passed

#291's stale snapshot failed differently and more loudly — AutomaticMigrationDataLossException at context construction. Its regenerated snapshot was verified by decompressing both and diffing entity sets: the delta is exactly the 10 interaction-history entities, with nothing removed (no Teams add-on entity resurrected, no usage-report entity dropped).

#291's migration was also re-timestamped from 202608191347242 to 202608200600001, because the original sorted before the already-merged 202608191533567_DeprecateTeamsAddons — i.e. it would have inserted itself into the middle of applied history.

2.2 The manual scripts were stale too — reviewer hotspot

The least obvious part of the release.

.manual.sql scripts stamp dbo.__MigrationHistory with an embedded copy of the model snapshot. Re-scaffolding the .resx left those embedded blobs pointing at the old model, so a DBA upgrading by hand would have stamped an out-of-date model and EF would then have generated an automatic migration at runtime — reintroducing the same failure on hand-upgraded databases only. The installer path would never have shown it.

Script Embedded model Expected Status
CopilotDroppedAuditFields 80,778 hex 80,778 already correct
AddCopilotUsageReports 53,548 base64 55,764 regenerated
DeprecateTeamsAddons 78,730 hex 81,496 regenerated
AddCopilotInteractionHistory stale 44,289 bytes regenerated, verified to round-trip to identical base64

The regenerated blobs were executed against SQL Server and confirmed to assemble to the correct DATALENGTH (41,823 and 40,748 bytes). Two prerequisites were also corrected: AddCopilotUsageReports' (#284) and AddCopilotInteractionHistory' (from IndexReportDateQueries to DeprecateTeamsAddons).


3. Performance KPIs

Required by the repo's "prove every schema change improves performance before it is approved for stable" rule.

Benchmarked the hot path of the new import: the per-batch existence lookup in CopilotUsageUserDetailLoader.SaveBatchAsync, which reads back already-stored rows so unchanged rows are not rewritten. It runs once per 1,000-user batch — 200 times per import at the 200,000-user baseline.

Synthetic scale: 200,000 users × 15 daily report refreshes × 2 periods (D7 + D28) = 6,000,000 rows. Scattered user ids, since report order and users.id order are uncorrelated on a real tenant. Medians of 6 runs, first discarded, OPTION (RECOMPILE). SELECT * because the loader materialises full entities. All data synthetic — no customer database was involved.

Read path — IX_date_user_id_report_period_days

Window Before (no index) After (index)
realistic (1 date, 1 period) 120,447 reads / 628 ms / Clustered Index Scan 6,142 reads / 15 ms / Index Seek 20× fewer reads, 42× faster
narrow (1 date, 2 periods) 120,447 / 2,119 ms / scan 20,618 / 165 ms / seek 5.8× fewer reads
wide (15 dates, 2 periods) 120,447 / 2,790 ms / scan 252,397 / 5,643 ms / seek ⚠️ 2× slower

The realistic row is the one that matters: one Graph report call covers a single period and returns a single reportRefreshDate, so reportDates and periods each hold exactly one value per batch.

Projected per import at 200,000 users (200 batches): ~126 s → ~3 s, and ~24.1M → ~1.2M logical reads.

The wide-window regression is real and deliberately reported. At 30,000 matched rows the key lookups (the index does not cover the ~23 columns EF materialises) cost more than a scan. This loader never issues that shape, but anything that later widens the date range — a backfill importing many refresh dates at once — would hit it and would need a covering index. Recorded here so that is a conscious decision rather than a surprise.

Write path

Variant 20,000 rows rows/sec Projected 200,000
With index 328 ms 60,975 3.3 s
Without index 250 ms 80,000 2.5 s

The index costs ~0.8 s of write time per full import and saves ~123 s of read time. It is also the natural-key uniqueness constraint, so it has to exist regardless.

Index build: 6.3 s on 6M rows, 151.2 MB. The table starts empty on every existing customer, so the build at upgrade time is instant.

These are SQL-side figures; they exclude EF and network round-trip overhead. The delta attributable to the index is what is measured, and that is what the decision rests on.


4. Upgrade and combined verification

Tests.UnitTests/UpgradeFromStableTests.cs rehearses the upgrade an existing customer actually performs, rather than starting from an empty database:

  1. Build a database at main's newest migration (202608131055001_IndexReportDateQueries).
  2. Seed it — including a Greek URL (Καλημέρα κόσμε) to catch Unicode truncation.
  3. Upgrade to the newest migration on dev.
  4. Assert row counts survive, the Greek URL is byte-intact, and the new Copilot tables exist.

Both branches of DeprecateTeamsAddons' conditional drop are covered, which an empty-database test can never reach:

Case Result
Teams add-on tables hold data Tables retained, row counts unchanged ✅
Teams add-on tables empty Tables dropped

#291 and #292 had never been tested against each other, so the merged tree was verified directly at e93630b: full solution builds (Debug + Release), and 94/94 pass across the migration-pipeline, migration-cleanup, interaction-history and adoption suites against a dropped-and-replayed LocalDB — so the whole chain reaches 202608200600001 with no snapshot mismatch. Their only file overlap was Entities.csproj and Tests.UnitTests.csproj; git merged both cleanly and the build confirms it. All dev CI is green at that SHA.


5. CI changes

#280 fixes two independent defects that meant required checks frequently never reported, leaving PRs at BLOCKED and mergeable only by admin bypass:

  • pull_request listened only for ready_for_review, which a PR opened non-draft never emits.
  • A workflow-level paths: filter meant a PR touching only src/TelemetryService/** never started the workflow at all — and a workflow that never starts reports nothing.

The gate is applied per step, not as a job-level if:, because GitHub evaluates a job-level if: before expanding the matrix: a skipped test_dotnet reports as plain test_dotnet and the required test_dotnet (Release) context never appears. Measured on #280 itself.

test_aitracker was also gated on src/AnalyticsEngine/** rather than the tracker's own src/SPO/AITracker/**, so its tests never ran when the tracker changed.

#282 adds the migration snapshot guard that would have caught all three defects in §2.1 on the pull request. #289 adds a weekly read-only az deployment group what-if drift check (maintainer-side only; deploys nothing).


6. New user-visible and privacy surfaces

Two things in this release are not bug fixes and need their own treatment in the customer-facing release notes:

#291 — Copilot AI interaction history (opt-in). This is the first feature that sends customer Copilot prompt text out of the tenant, to Azure AI Language, and only when cognitive services are configured. No prompt or response text is persisted — bodies are reduced to counts and discarded, no column can hold one, response bodies are kept out of logs, and Copilot responses are never scored. But "nothing is stored" is not "nothing leaves the tenant", and the notes must say so plainly.

It is off by default and is capped per cycle — because the endpoint is one HTTP call per user with no tenant-wide or delta form, so an unscoped run at the 200k-user baseline would be 200k Graph calls per cycle. It also needs AiEnterpriseInteraction.Read.All (application, no delegated form), which the installer does not grant: missing admin consent is the most likely reason for this import to silently do nothing.

#292 — Copilot licence adoption tool. New admin API (CopilotAdoptionAPIController) and admin-app pages for finding unused seats and unlicensed heavy users. Read-only and additive: no EF model change, no migration, no config-schema change; it queries existing tables via a context factory.

#288 also fixed a live authorisation defect on the maintainer telemetry service: the Telemetry.Read scope was never enforced, because [RequiredScope] is inert metadata whose handler was never registered, and [Authorize(Roles = …)] replaces the default policy rather than extending it. A token with the correct role but the wrong scope returned 200 before, 403 after. Exposure was limited — the app role is assignment-gated and EasyAuth validates audience and allowedApplications — so this was a missing defence-in-depth layer, not an open door. It affects the maintainer dashboard only, not customer deployments.


7. Risks and reviewer hotspots

  1. The manual-script model blobs (§2.2). Highest-value thing to check. Verify by reconstructing an embedded blob and comparing it to the .resx Target.
  2. The offline index builds on non-Enterprise editions (§2). The only part of the release that can take a visible outage on a large tenant. copilot_event_accessed_resources is rebuilt twice (FK index pair, then the dedup index), and the other five migrations are instant — so it is easy to under-budget the window. See the sizing table and the storage/log notes in §2.
  3. Upgrade ordering (§2). New binaries against a pre-migration schema is measurably slower than the release being upgraded from, because the merge SQL and the bounded de-dup read both assume indexes that arrive by migration. This only bites sites that apply schema by hand, which is exactly the audience the manual scripts serve.
  4. A failing daily usage report now aborts that cycle's activity-report phase (Release critique follow-ups: the same #285 hole in the legacy daily reports, a fail-closed dedup guard, and three doc errors #309). The SharePoint/Teams/OneDrive/Yammer/Exchange daily loaders previously swallowed a 403 or 5xx and recorded an empty day, after which the phase stamped itself complete for 24 hours — a broken import looked healthy and idle, exactly the Graph paging failures are recorded as successful empty imports #285 symptom on an older code path. They now use strict paging, so the failure propagates and the phase is retried next cycle instead of going quiet. This restores the contract the phase is already written around (it deletes its "last imported" timestamp before loading and only re-saves it once every loader completes), but it is a genuine behaviour change on a long-standing path: one failing report type now costs that cycle's other reports, which are picked up on the next run. Admin-visible effect: a misconfigured tenant will now surface real errors in Application Insights where it previously showed a clean, empty import. That is the point, but it should be flagged so a burst of new errors after upgrading is read correctly. Two related admin-visible consequences:
    • While any one usage report is failing, the once-a-day throttle stays disarmed, so the phase re-runs every cycle and re-downloads the full window for all reports — roughly 10x the normal Graph call volume until the underlying error is fixed. No data is lost and no other import is affected; it is efficiency only. Tracked as Activity reports: a single failing usage report disables the finalized-date skip list, causing a full re-download every cycle #311.
    • A tenant with a permanently broken report will stop emitting the "usage reports finished" telemetry event, so any "hasn't finished in X hours" monitoring will start firing. That is the fix working as intended, not a new fault.
  5. The accessed-resource de-dup identity changed (Copilot RC fixes: Graph paging failures, accessed-resource dedup correctness, and two bounded reads (#285, #287, #294, #296) #307). Rows are now keyed on the full seven-column tuple instead of five. Two consequences worth watching: a resource accessed with more than one action now produces one junction row per action rather than one in total, and on re-import the full-tuple NOT EXISTS will back-fill rows that were previously collapsed away — so copilot_event_accessed_resources row counts will step up after the upgrade. That is the fix working, not double-counting, but it should be said out loud in the admin notes. The growth is unquantified: the original justification for collapsing ("Action is Read for every access, so nothing real is lost") was removed rather than tested, and the benchmark holds the resolved-row count constant, so it measures cost per row and not the change in row count.
  6. CONFIG_VERSION 2.1.0, with a breaking step at 2.0.0. Confirm the removal of GraphUserApps degrades gracefully for existing saved configs.
  7. DeprecateTeamsAddons is conditional. A tenant with add-on data keeps the tables and the reporting views; a fresh install loses them. Two shapes in the field from one migration.
  8. The wide-window index regression (§3). Not hit today; would be hit by a multi-date backfill.
  9. Copilot: import AI interaction history from Graph (opt-in, statistics only) #291 is a new egress path for prompt text (§6). Worth an explicit sign-off rather than being read as just another importer.
  10. test_dotnet (Release) now runs on every PR and does nothing (~10 s) when src/AnalyticsEngine/** is untouched. Deliberate: the alternative is unenforceable required checks.

8. Multi-model review outcome

This release has been through two independent multi-model reviews.

Round 1 — the diff as originally opened

Three models (Claude Opus 4.8, GPT-5.6 Sol, Gemini 3.1 Pro) reviewed it independently. Every finding was verified against the code; one did not survive that check.

Fixed and included here (#284):

  • AddCopilotUsageReports.manual.sql named the wrong prerequisite, letting a hand-upgrading DBA skip a migration and still pass the gate.
  • CopilotUsageUserDetailLoader dropped unkeyable rows with an O(N²) RemoveAt loop.

Rejected: a reported .ToLowerInvariant() rule violation in SentEmailImporter — those values are persisted, not just used as set keys, so removing them would change stored data.

Round 2 — #288, #289, #291 and #292

Those four were added after round 1, so they were reviewed separately by four models (Claude Opus 4.8, GPT-5.6 Sol, Gemini 3.1 Pro, Grok 4.6), alongside a synthetic-scale benchmark of the interaction-history dedup path. Two findings did not survive verification and were dropped.

It found four release blockers, all fixed in #293, now merged into dev:

Blocker Effect
Adoption report snapshot selected by date only copilot_usage_user_activity_log is keyed (date, user_id, report_period_days), so every licensed user was duplicated per stored period — adoption figures inflated up to 4x, able to exceed the licensed population
Drift check published raw config values --result-format FullResourcePayloads into a world-readable job summary on a public repo — would have printed the Application Insights connection string
Interaction-history cadence gate stamped on failure Enabling the feature before AiEnterpriseInteraction.Read.All consent silently did nothing for 24h
Transient Graph errors fed the 72h unlicensed back-off Two throttling blips parked the entire active pilot group for three days

Plus seven correctness/scale fixes, including a dedup re-key that fixes a cross-user data drop and measures 1.8x fewer logical reads / 5.7x faster, and $orderby=createdDateTime asc (flagged independently by three of the four models) without which a truncated first-run backfill can permanently lose history.

The benchmark also supplied the measurement #291 shipped without: its (session_id, graph_interaction_id) index takes the dedup from 48,328 logical reads to 1,323 (seek, not scan), so the shipped index shape is proven correct and needs no change.

Four further findings were filed with their measurements rather than fixed in #293. Two have since been fixed and are in this release#295 (adoption trend double scan, fixed in #303) and #297 (pilot-scope call brake, fixed in #306). Two remain deferred: #294 and #296both are now fixed and in this release via #307, each carrying the measured before/after the gate requires (see the gate classification table in §2).

Round 3 — the easy-issue sweep

After round 2, the remaining open issues were triaged for what could be safely completed for this release. Four were genuinely low-risk and are now merged: #286 (#304), #295 (#303), #297 (#306) and #302 (#305).

Deliberately not taken, despite looking easy:


9. Known issues shipping with this release

Tracked, not blocking, but they should appear in the customer-facing notes:


10. Deferred


11. On merge


Added since this PR was last updated

Two further changes merged into dev.

Copilot Adoption: explainability, parity, correctness and scale (#315)

Follow-up to the original tool (#292). The user-visible additions are info tips on every assertion, an agents inventory with Keep/Review/Retire/New verdicts, an unlicensed-usage view, executive visuals and a 12-sheet Excel export with live charts.

The part that matters for a release decision is the scale work. Four hand-written queries asked for several COUNT(DISTINCT …) in one GROUP BY. SQL Server streams a single distinct aggregate cheaply; two or more force a spool. Measured on a synthetic 200,000-user / 12M-interaction tenant, all four exceeded the 90-second command timeout — and the failure mode is a warning on the page rather than an error, so on a large tenant the report was not slow, it was quietly broken, and nothing in production reported it.

Query Elapsed before after
LicensedUsersSql 28d 281 s 73 s
LicensedUsersSql 365d 303 s 135 s
AgentUsageSql 128 s 23 s
UnlicensedUsageRowsSql 131 s 54 s
WeeklyAdoptionTrendSql 315 s 38 s

No schema change, no migration, no new index — every fix is a change to how the query was written. Each rewrite was gated by an old-vs-new row-for-row comparison before performance was considered (33,999 / 200 / 33,999 / 27 rows, zero differing).

Two honest caveats, both carried over from #315 rather than hidden:

  • LicensedUsersSql at a 365-day window is still 135 s at that scale. No longer failing, but not comfortable.
  • Two of the rewrites trade more logical reads for much less elapsed time. Normally the wrong way round; shipped because the alternative returned no report at all.

Also in #315: several correctness fixes (capped-denominator rates that capped a 200k tenant at 25% adoption; an agent KPI inflated ~4×; a "last N days" window that spanned N+1 days; an agent user count inflated by unattributed audit events), a new CopilotAdoptionAnalysis App Insights event with per-step durations as measurements, and a terminology pass from "seat" to "licence" in everything a user reads.

Installer: handle new-tenant Graph Reports readiness (#317)

A newly created tenant's Microsoft 365 reporting backend may not have onboarded yet. In that state every /reports/get* endpoint returns HTTP 404 carrying the nested Graph code UnknownTenantId, while all other Graph endpoints are healthy. The installer now detects that condition and reports it, instead of the admin seeing a successful install whose usage reports silently return nothing.

Related, NOT in this release: the importer-side half of the same problem (the activity-report phase stamping itself complete after recording consecutive empty days) is still outstanding on branch sambetts/graph-report-404-silent-failure. Release notes must not imply the silent-failure path is fully closed.

Migrations in this release

Seven migrations. All are additive or index-only — new tables and nullable columns for the Copilot usage reports, interaction history and dropped audit fields, plus three index changes:

  • CoverCopilotAccessedResourceDedup, WidenCopilotAccessedResourceDedupIndex — measured, documented in their doc comments
  • IndexCopilotInteractionKeywordsByKeyword, IndexCopilotInteractionsDedupWindow — on tables new in this release, so instant

Per the release policy each needs its <migrationid>.manual.sql attached to the stable GitHub release — CI does not upload these, so they must be added by hand, in migration-id order.

sambetts and others added 24 commits August 19, 2026 08:52
The Copilot importer already deserialised a set of CopilotInteractionAuditRecord
fields and then threw them away - they were never written to SQL. Persist them.
No new API, permission or import-cycle cost: the data is in payloads we already
download.

Now stored:
* CopilotEventData.ThreadId, ClientRegion, CopilotLogVersion -> copilot_chats
* ALL Contexts (Id/Type/ContainerId) -> new copilot_event_contexts +
  copilot_event_context_types. Only the first file/meeting context is resolved
  into copilot_event_files/_meetings; the rest of this unordered collection was
  lost. That resolution behaviour is unchanged.
* AISystemPlugin (Id/Name/Version) -> new copilot_ai_system_plugins + junction
* AccessedResources[].Action -> new copilot_event_accessed_resource_actions
  lookup + action_id; .listItemUniqueId -> list_item_unique_id_id, resolved
  against the EXISTING resource-id dimension (the payload repeats Id there)
* Messages[].Size / .isPrompt -> copilot_event_messages. Prompts are no longer
  filtered out: Size only exists on the prompt row and is_prompt would be a
  constant otherwise. Roughly doubles that table.
* ModelTransparencyDetails provider/version -> copilot_ai_models, whose key
  becomes the (name, provider, version) tuple

No backfill is possible - Management Activity API content is retrievable for 7
days - so this applies to newly imported interactions only.

The accessed-resource de-dup tuple is deliberately NOT widened: it is covered
exactly by IX_copilot_event_accessed_resources_dedup, so the batch collapse moved
into the resolve step (GROUP BY the original 5 columns + MIN on the new payload
columns) and insert_junction no longer sorts at all.

Also fixes copilot_event_messages having no de-duplication: an interaction that
stages into two staging tables (Teams chat context + a following file context)
inserted its messages twice.

Measured at synthetic scale (1M chats / 3M junction rows, 1000-event batch, plan
cache cleared, medians of 8 warm runs): logical reads 438,673 -> 564,582 (+29%),
elapsed 1,780 -> 1,602 ms. insert_junction 328 -> 314 ms, link_ai_models
505 -> 125 ms. Junction index builds at 3M rows: 5.6 s / 81 MB for the pair.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Users with no Exchange Online mailbox - unlicensed, on-premises, inactive,
or guest (#EXT#) accounts - return HTTP 404 from Graph on every sent-email
call, permanently. Each such user was producing three App Insights exception
records on every 10-minute import cycle, and was re-checked indefinitely.

Three changes:

1. A 404 is now a typed, non-error outcome. ManualGraphCallClient throws
   GraphResourceNotFoundException (derived from HttpRequestException, so
   existing catch blocks are unaffected) and logs at Debug rather than Error,
   exposing Graph's error code (MailboxNotEnabledForRESTAPI,
   Request_ResourceNotFound) for diagnostics.

2. PageableGraphLoaderExtensions no longer double-logs. It was calling
   LogError twice for an exception ManualGraphCallClient had already logged,
   turning one failed call into three exception records. It now logs only the
   paging consequence, at Warning. Gains an opt-in throwOnNotFound flag
   (default false, so every existing caller keeps its partial-result
   behaviour) which the sent-email loader uses to tell "no mailbox" apart
   from "mailbox with no sent mail" - previously indistinguishable, as the
   404 was swallowed and an empty list returned.

3. SentEmailImporter negatively caches mailbox-less users and skips them,
   re-sweeping the whole directory every SentEmailNoMailboxRetryHours
   (new AppSetting, default 24; 0 disables) so newly-licensed users are
   picked up. Backed by Redis when configured, otherwise in memory, and
   hoisted to process lifetime in Program.cs alongside the existing cadence
   stores. Deliberately a single key holding the whole set, not one key per
   user, so it stays O(1) round trips at the 200k-user target scale.

No behaviour change for other Graph importers, no schema or migration
change, and no CONFIG_VERSION bump (AppConfig is runtime app settings, not
the installer's saved config schema).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The telemetry dashboard failed with "Could not load dashboard data: stats:
HTTP 403" for users holding a completely valid token - correct audience,
correct issuer, and both the Telemetry.Dashboard.Read role and the
Telemetry.Read scope present.

The 403 came from App Service Authentication, not the application. The
authsettingsV2 validation block set allowedAudiences but no
defaultAuthorizationPolicy, so the platform stored
allowed_client_applications as an EMPTY array - which EasyAuth reads as
"permit nothing". It authenticated the caller and then rejected the request
before it ever reached Kestrel.

The symptom is badly misleading and worth recording:

  - anonymous requests SUCCEED (unauthenticatedClientAction is AllowAnonymous),
    so /health and /api/auth/config kept returning 200;
  - a token with an INVALID signature also succeeds, because it fails
    validation and is therefore treated as anonymous and passed through to the
    app, which then answers with its own 401;
  - a VALID token is the only thing that gets a 403.

So every conventional check - app role assignment, admin consent, audience,
issuer, tenant, token expiry, and the ASP.NET Core authorization attributes -
looks correct, because all of them are correct. An isolated reproduction of the
exact production auth pipeline (same .NET 10, same Microsoft.Identity.Web
4.14.2, same AzureAd settings, same [Authorize(Roles)] + [RequiredScope]
attributes) returns 200 for the very token that production rejects.

Fixed by naming the SPA client explicitly in allowedApplications. This is also
tighter than leaving the policy null, which would admit any client application
able to obtain a token for this audience.

EasyAuth remains enabled and non-enforcing, so the MISE key-discovery telemetry
required by SFI ID2.1.1 / ID2.1.2 continues to be emitted and the app
registration stays compliant.

azuredeploy.json is recompiled from the bicep. The local Bicep CLI was upgraded
to 0.46.1 first, because building with the older 0.36.1 silently downgraded the
nested deployment apiVersion from 2025-04-01 to 2022-09-01.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The EasyAuth / MISE runtime caches the auth configuration, so a corrected
allowedApplications value is not honoured until the site restarts. Until then
the config APIs (az webapp auth show, the ARM authsettingsV2 GET) all report the
new value while requests are still being rejected using the old one - so a
correct fix looks like it did not work.

Observed while remediating the empty allowed_client_applications allow-list: the
setting read back correctly from both the v1 and v2 config views, yet the
dashboard kept returning 403 until the App Service was restarted.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Application Insights resource for the telemetry service had received
nothing since the service was first deployed - no requests, traces,
dependencies or exceptions - even for requests that demonstrably reached the
application and returned 200.

The connection string was configured correctly and the ingestion endpoint was
reachable from inside the container (a GET to /v2/track returns 405, i.e. the
network path is fine). Nothing was reading either.

Two gaps:

1. The application referenced no telemetry SDK at all - no
   Microsoft.ApplicationInsights.AspNetCore, no OpenTelemetry - and Program.cs
   never wired one up.
2. The infrastructure set ApplicationInsightsAgent_EXTENSION_VERSION=~3, which
   is the *Windows* App Service codeless-attach setting. This site runs on
   Linux (DOTNETCORE|10.0), where that setting is silently ineffective for
   .NET. It made the site look instrumented while guaranteeing silence.

Fixed by adding Azure.Monitor.OpenTelemetry.AspNetCore and calling
AddOpenTelemetry().UseAzureMonitor(), guarded on the connection string being
present so local development stays quiet. The misleading agent setting is
removed from the bicep, with a comment explaining why it must not come back.

Note this needs an application redeploy to take effect - correcting the app
settings alone will not start telemetry flowing.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The dashboard rendered everything on one page: four headline cards, then the
full aggregated table list, with the clients table stranded below it. On a real
install base the table list is long enough that the clients table is effectively
undiscoverable, and the styling did not match the in-product admin app.

UI
- Rebuilt on Fluent UI v9, matching src/AnalyticsEngine/Web/Scripts/admin-app:
  brand header, TabList navigation, same content width and tokens. The bespoke
  App.css is gone so the two apps cannot drift apart.
- Content is split across Overview / Tables / Clients / Adoption, each
  lazy-loaded as its own chunk (main bundle 785 kB -> 534 kB).
- Tables and Clients are filterable and sortable; Clients shows relative "last
  report" times, highlights stale installs and lists each client's enabled
  imports in a popover.
- Added a Refresh button, proper loading and error states, and a distinct empty
  state for "no telemetry received yet".

New statistics, all derived server-side from telemetry clients already send, so
no client-side change is required to populate them:
- Reporting freshness: clients bucketed by last check-in (24h / 7d / 30d /
  stale), which surfaces installs that have silently stopped reporting.
- Deployment size distribution: median, average and largest client by both rows
  and size, plus average table count. Averages alone hide the shape of the
  install base.
- Build adoption: clients per build version, with last-seen.
- Import feature adoption: per-toggle enabled/disabled counts, parsed from each
  client's ConfiguredImportsEnabledDescription.
- Storage by SQL schema, so application tables can be told apart from profiling
  ones.
- Azure AI usage: total data points and how many clients report them.
- Distinct table count, and average rows per client per table.

Two deliberate choices in the aggregation:
- Table totals are now keyed on schema + table rather than table name alone.
  The same table name can legitimately exist in two schemas, and merging them
  would silently overstate both.
- Feature adoption percentages are of clients reporting that toggle, not of all
  clients. A newly added import is absent from older builds rather than
  reported as off, so the alternative would make new features look unpopular.

Version labels and schema names are grouped under "(unknown)" when a client
predates the field rather than being dropped, so counts still reconcile.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The telemetry service had no tests and no pipeline: it was built and deployed by
hand, and the dashboard aggregation (medians, settings parsing, freshness
bucketing) was entirely unverified.

Tests
- New Tests.Unit (MSTest, net10.0), registered in TelemetryService.slnx: 58
  tests covering dashboard aggregation, the settings-string parser, the upload
  endpoint's signature checks, save/merge behaviour and configuration binding.
  Aggregate now takes an injectable nowUtc so freshness bucketing is
  deterministic instead of depending on when the suite runs.
- Vitest + Testing Library for the client: 22 tests covering the formatting
  helpers and the sortable table. Fluent UI has to be inlined in the Vitest
  config because @fluentui/react-icons ships extensionless ESM chunk imports
  that Vite's node resolver cannot follow.

Two real defects were found by writing the tests:
- Version adoption sorted "(unknown)" ahead of a real build on a tie, so a
  bucket that is not a version could present as the most common one. Unknown is
  now deliberately ordered last.
- TelemetryController guards against an empty TelemetrySecret, but that value is
  a required config binding, so WebAppConfig already refuses to construct
  without it. The guard is unreachable through normal startup; the test now
  documents it as defence-in-depth rather than pretending it is a live path.

Refactoring
- Program.cs is now just an entry point. DI composition moved to
  TelemetryServiceCollectionExtensions so tests can build the real object graph
  without duplicating the wiring, and Program is made partial so integration
  tests can host it with WebApplicationFactory.
- Cosmos container creation moved out of the startup path into a hosted service.
  It previously ran before the host was built, so an unreachable or
  misconfigured Cosmos account stopped the site starting at all - including the
  anonymous /health endpoint, which makes an outage look like a failed
  deployment.
- vite.config.ts generated ASP.NET dev certificates at module scope, so every
  production build - including in CI, where there is no dev certificate and no
  need for one - shelled out to `dotnet dev-certs`. Certificate handling now
  only happens when actually serving.

CI/CD
- New telemetry-service workflow, kept separate from the existing ones because
  those build the .NET Framework solution on Windows and publish GitHub
  releases, whereas this is a .NET 10 Linux web app deployed to App Service.
- Lints and tests both halves on every PR; deploys on pushes to main and dev.
- dotnet publish also builds the Vite client into wwwroot, so one artifact is
  the whole site.
- After deploying it polls /health, so a build that deploys but fails to start
  is reported as a failed deployment rather than a success.
- Azure sign-in uses OIDC federated credentials, so no secret is stored. The
  app name, resource group, tenant and subscription all come from repository
  secrets/variables - no environment identifiers are committed to this public
  repository.
- The deploy job cannot run from a pull request or from a fork.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ci, pr and tests all build the .NET Framework AnalyticsEngine solution, but
triggered on any change under src/**. Now that src/ also holds TelemetryService,
a telemetry-only change kicked off a full Windows release build, an installer
build and the AnalyticsEngine test suite for no reason.

Narrowed all three to src/AnalyticsEngine/** and their own workflow file.
reports/** stays, because the Power BI templates ship as part of that release.

ci also triggered on .github/**, so any change to a workflow, an agent
definition or the Copilot instructions started a full release build. That is now
just .github/workflows/ci.yml, which is the file that actually changes what the
release produces.

tests.yml has three filters, not one: the push paths, the pull_request paths and
the dorny/paths-filter "code" filter that gates every job. All three are updated,
otherwise the workflow would still start and then evaluate the old filter.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
azuredeploy.json conflicted because #264 and this branch each recompiled it from
their own bicep. Resolved by regenerating the template from the merged
resources.bicep rather than hand-merging generated output, so it now carries both
the EasyAuth allowedApplications fix and the removal of the Windows-only
ApplicationInsightsAgent_EXTENSION_VERSION setting.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Program.cs conflicted because #265 added the in-process Application Insights
wiring while this branch slimmed Program.cs down to an entry point. Both are
wanted, so the resolution keeps the App Insights block inside the slim file.
Azure.Identity is no longer imported here - the Cosmos credential moved to
TelemetryServiceCollectionExtensions - so only the OpenTelemetry using remains.

Web.Server.csproj auto-merged and retains both the Azure.Monitor.OpenTelemetry
package reference and the InternalsVisibleTo needed by the test project.

Verified after resolving: 58 server tests and 22 client tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Resolves the workflows README conflict: keeps the AnalyticsEngine scoping note
from this branch and the telemetry-service section added by #267, and reworks the
intro because there are now four workflows and telemetry-service is deliberately
not AnalyticsEngine-scoped.

Also bumps the actions in telemetry-service.yml. The first pipeline run warned
that checkout, setup-dotnet, setup-node and upload-artifact all target Node 20
and were being forced onto Node 24. Moved to the current majors (checkout v7,
setup-dotnet v6, setup-node v7, upload-artifact v7, download-artifact v8), which
also brings the file in line with the versions the other workflows already use.

azure/login and azure/webapps-deploy were not flagged and are left alone.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The previous run's annotation flagged azure/login@v2 as targeting Node 20 and
being force-run on Node 24. Azure publishes v3 specifically for Node 24 support,
with the same inputs, so this is a straight major bump.

azure/webapps-deploy@v3 was not flagged and is already current, so it is left
alone.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Sam Betts <sambetts@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…282)

Co-authored-by: Sam Betts <sambetts@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…260)

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

Copy link
Copy Markdown
Collaborator Author

Multi-model review of this release

Three models reviewed the full main...dev diff independently, with the same prompt and the repo's own rules (200k-user baseline, EF6 automatic-migration risk, Unicode/nvarchar, migration safety, public-repo data hygiene):

Model Verdict
Claude Opus 4.8 Ship
GPT-5.6 Sol Don't ship
Gemini 3.1 Pro Don't ship

I verified every finding against the code rather than relaying it. Two did not survive that check. The table below is my adjudication, not a merge of their opinions.

# Finding Found by Verified? Status
1 Graph failures recorded as successful empty imports GPT ✅ confirmed Open — highest value
2 Accessed-resource actions collapsed by independent MIN() GPT ✅ confirmed Open
3 New usage table missing from retention cleanup GPT ✅ confirmed Open
4 Manual script named the wrong prerequisite Opus + GPT ✅ confirmed ✅ Fixed in #284
5 RemoveAt in reverse loop is O(N²) Gemini + GPT ✅ confirmed ✅ Fixed in #284
6 Unbounded per-user Graph calls under a group filter Gemini + GPT ✅ confirmed Open — pre-existing pattern
7 User-data lookup omits the new workload GPT ✅ confirmed Open
8 .ToLowerInvariant() are redundant allocations Gemini wrong No action

1. Graph failures are recorded as successful empty imports — open

The most valuable finding of the exercise, and no amount of unit testing would have surfaced it.

PageableGraphLoaderExtensions treats any non-gateway-timeout HttpRequestException as end-of-paging:

logger.LogWarning($"Unexpected HTTP error on page {pageCount}: {ex.Message}. " +
    "Will not retry page & returning results upto current page.");
nextUrl = null;

It returns the rows accumulated so far and does not throw. For the Copilot reports that means a 403 from a missing Reports.Read.All grant returns zero rows with no exception. CopilotUsageUserDetailLoader then takes the parsed.Count == 0 branch, logs "Expected on a tenant with no Microsoft 365 Copilot licences", writes an import log with no error, and returns 0. The cadence gate records success, so it will not retry for 24 hours.

The existing shape-change guard does not catch it either, because it is conditioned on reports.Count > 0:

if (reports.Count > 0 && parsed.Count == 0) { throw ... }

So a permissions misconfiguration, a 5xx mid-paging, or an exhausted 429 all present to an admin as "you have no Copilot licences". That is exactly the misleading-symptom class this project's release notes are supposed to call out.

Suggested fix: a strict paging mode that rethrows HTTP failures, used by all three Copilot reports, with the deliberate 404/non-global-cloud case handled separately and recorded in CopilotUsageReportImportLog.Error.

2. Accessed-resource actions are collapsed, and pairings can be fabricated — open

common_upsert_copilot_agents.sql:302-312 groups on the original five-column resource tuple and then takes two independent minima:

MIN(raction.id)    AS action_id,
MIN(rlistitem.id)  AS list_item_unique_id_id

Two consequences:

  1. Two accesses of the same resource with different actions (Read, Write) collapse into one row, keeping only the lower id. That directly undercuts Copilot: persist parsed-but-dropped audit fields #262's purpose, which is to stop dropping these fields.
  2. Because the two MIN()s are evaluated independently, the surviving row can pair an action_id from one source row with a list_item_unique_id_id from another — a combination that never appeared in the payload.

The in-code comment justifies this by keeping the dedup identity unchanged so IX_..._dedup still covers the anti-join. That is a real and well-reasoned constraint. But note the stated 1700-byte index-limit rationale does not apply here: both columns are int foreign keys, so adding them to the key costs ~8 bytes, not customer-text width.

Not a crash and not loss of pre-existing data, so defensible for a test release — but it should not reach stable without either widening the tuple or an explicit decision that the fidelity loss is acceptable.

3. copilot_usage_user_activity_log has no retention bound — open

Clean Old Data Data.sql prunes every other usage-report table (onedrive_user_activity_log, outlook_user_activity_log, sharepoint_user_activity_log, teams_user_activity_log, the Yammer set, and the Teams add-on tables under OBJECT_ID guards). The new table is absent.

At the 200k-user baseline that is up to 200,000 rows per report refresh per period, with nothing ever removing them. The table this release removes (teams_addons_user_installed_log) was deprecated in #279 precisely for becoming the largest table in the product — so it would be unfortunate to introduce another unbounded one in the same release.

Fix: a guarded batched delete on the indexed date column, alongside the others.

4 and 5 — fixed in #284

Both confirmed and already addressed:

  • AddCopilotUsageReports.manual.sql gated on 202608131055001_IndexReportDateQueries rather than its immediate predecessor 202608190622001_CopilotDroppedAuditFields, so a DBA upgrading by hand could skip a migration and still pass the check. Installer/EF path unaffected (it applies in id order).
  • CopilotUsageUserDetailLoader dropped unkeyable rows with RemoveAt in a reverse loop — O(N²) element moves when removals are scattered. Now RemoveAll.

On severity for #5: both models overstated it slightly. The quadratic case needs request.PeriodDays to be null (period ALL) and a mix of keyable and unkeyable rows. On a normal D7/D28 import nothing is removed at all and the old loop was O(N). Fixed anyway — it is free.

6. Unbounded per-user Graph calls under a group filter — open, pre-existing

FilterToUsersInScope awaits one /users/{upn}/memberOf per report row, sequentially. Two models called this a blocker. I would not block on it, for three reasons: it only applies when a group filter is configured, it is the same pattern every other per-user usage-report loader uses, and the code already logs a loud warning naming the call count.

It is still the right thing to fix — resolve the filter groups once and intersect UPNs in memory — but it is a pre-existing architectural pattern, not a regression introduced here.

7. User-data lookup omits the new workload — open

UserDataLookupAPIController does not expose copilot_usage_user_activity_log, and drops the Teams app-install category even though #279 deliberately retains populated teams_addons_user_installed_log tables. Deletion remains correct (the new table cascades on user delete, and cleanup handles retained Teams rows dynamically); it is discovery that is incomplete. Worth fixing before stable given what that endpoint is for.

8. .ToLowerInvariant() — reported, but wrong

Gemini flagged SentEmailImporter.cs:728,743 as violating the "don't .ToLower() into an OrdinalIgnoreCase collection" rule. The collections are OrdinalIgnoreCase, so the report looks right at a glance — but the lowercased values are assigned back and persisted:

toAddr = toAddr.ToLowerInvariant();
recipientAddresses.Add(toAddr);   // -> stored
...
FromAddress = fromAddr,           // -> stored

They normalise stored email addresses; they are not merely set keys. Removing them would change what is written to the database. No action taken. Recording it because a plausible-looking rule violation that turns out to be load-bearing is exactly the kind of "fix" that causes an incident.


Where all three models agreed the code is clean

Worth stating, since it is most of the diff:

  • All three migration .resx snapshots match their manual-script model blobs; entity/snapshot parity is clean.
  • Migration DDL is guarded, idempotent and resumable; suppressTransaction: true used correctly; ONLINE attempts go through sp_executesql with an offline fallback and edition gating.
  • The Teams deprecation uses EXISTS not COUNT(*), retains all three tables if any holds data, and only rewrites dependent views on the drop path.
  • No added varchar anywhere; customer text is bounded nvarchar, including indexed nvarchar(850).
  • The Copilot persistence path follows the repo's scale rules: GetRange batching, Contains chunked to 1,000, OrdinalIgnoreCase in-memory resolution with no SQL LOWER(), bounded EF change tracking.
  • CONFIG_VERSION correctly records the additive 1.10.0 step and the breaking 2.0.0 removal; legacy GraphUserApps tokens are ignored rather than throwing.
  • Public-data scan: only synthetic values (Contoso, zeroed GUIDs, well-known Azure role IDs, public service hostnames). No customer or tenant identifiers.
  • Telemetry service auth, role/scope enforcement, signed upload, Cosmos caps and deploy gating all clean.

My recommendation

Ship as a test release, with findings 1, 2 and 3 tracked as issues and fixed before stable.

Reasoning: nothing here corrupts or loses pre-existing customer data, the upgrade path is verified on a populated database in both of DeprecateTeamsAddons' branches, and the migration chain is sound. Findings 2 and 3 are fidelity and growth concerns that a test tenant will tolerate. Finding 1 is the one to fix soonest — not because it breaks the import, but because it makes a broken import look like a healthy one, which is the hardest kind of problem to diagnose in the field.

If the test tenant shows zero Copilot usage rows after deployment, check Reports.Read.All before believing the "no licences" message.

…eployment (#289)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
sambetts and others added 11 commits August 20, 2026 13:25
… only) (#291)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… users (#292)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Generate O365 audit events plus SharePoint, OneDrive, Outlook, and Teams profiling sources, with a combined option that shares users and an exact date window with Copilot data. Align Copilot app hosts with profiling and include the full Sunday boundary in weekly aggregation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 07839e7d-91ef-4eaf-8fcb-a6100194a4d4
…p, Graph paging order, and 7 more (#293)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ing, not a precondition (#298)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ked in #300) (#301)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… a stronger upgrade rehearsal (#299)

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

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

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…oles in the call brake (#297) (#306)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
sambetts and others added 3 commits August 21, 2026 11:19
…ectness, and two bounded reads (#285, #287, #294, #296) (#307)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…n the PR template (#308)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…eports, a fail-closed dedup guard, and three doc errors (#309)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
sambetts and others added 5 commits August 21, 2026 14:14
…g breaking install verification (#310)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…in the Copilot tab (#312) (#313)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…chart description (#314)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5fdf889f-b05e-4e31-b213-787104ddfde9
…rectness and scale fixes (#315)

Follow-up work on the Copilot Adoption area (/copilot-adoption), on top of the original tool in #292.

Explainability and capability
- "i" info tips on every assertion: what it claims, how it is worked out, the formula, the source.
- Capability parity with microsoft/AI-in-One-Dashboard: agents inventory with Keep/Review/Retire/New
  verdicts, unlicensed population, usage concentration, combined leaderboard, three-population trend.
- Executive visuals and a 12-sheet Excel export with live charts, written as dependency-free OpenXML.
- Overview restructured into named sections; enablement-plan rows drill through to the exact people.

Correctness
- NaN% methodology tab: CopilotAdoptionOptions was the only model without camelCase JsonProperty names.
- "Invalid column name 'department'": dbo.users has department_id, not department (4 occurrences).
- Rates divided capped counts by the uncapped licence count, so a 200k tenant could never report
  adoption above 25%. ScoredUsers is now the denominator.
- Agent interactions-per-user divided a 120-day numerator by a 28-day denominator, inflating it ~4x.
- "Last N days" started N days back, spanning N+1 dates, so the numerator's window was wider than the
  denominator's target. WindowStartUtc now sits beside TargetActiveDays so they cannot drift.
- Agent user count could be inflated by unattributed (NULL user) audit events, flipping an agent's
  verdict from Review to Keep.

Scale - the report was not slow at 200k users, it was broken
Four queries asked for several COUNT(DISTINCT ...) in one GROUP BY. SQL Server streams a single
distinct aggregate cheaply; two or more force a spool. All four exceeded the 90s command timeout, and
the failure mode is a warning on the page rather than an error, so it was invisible. Measured on a
synthetic 200,000-user / 12M-interaction tenant, medians of 3 runs discarding the cold run:

  LicensedUsersSql   28d   281s -> 73s    114.8M -> 772k logical reads
  LicensedUsersSql  365d   303s -> 135s   115.2M -> 819k
  AgentUsageSql            128s -> 23s      6.4M -> 2.7M
  UnlicensedUsageRows      131s -> 54s      8.4M -> 44.1M  (reads regress, see PR)
  WeeklyAdoptionTrend      315s -> 38s     24.5M -> 58.1M  (reads regress, see PR)

No schema change, no migration, no new index. Every rewrite was gated by an old-vs-new row-for-row
comparison before performance was considered: 33,999 / 200 / 33,999 / 27 rows, zero differing.
Two of the rewrites trade more logical reads for much less elapsed time; that trade is documented in
the PR rather than hidden, and points at the pre-aggregated rollups in the wiki backlog.

Telemetry
- One CopilotAdoptionAnalysis App Insights event per analysis (not per request), with per-step
  durations as measurements so they can be percentiled directly. Dimensions carry Outcome, TimedOut
  and SlowestStep so a degrading report is alertable. No tenant data.

Terminology and docs
- "Seat" replaced with "licence" in everything a user reads; identifiers, JSON names and SQL aliases
  deliberately unchanged so the wire contract holds.
- Wiki: spec, user guide, measured-at-scale findings, telemetry/monitoring queries, and the
  FakeDataGen adoption-persona scenario.

Test data
- Tests.FakeDataGen can now shape a tenant into adoption personas covering every funnel stage and
  several distinct engagement shapes, verified against the real scoring code at run time.

Copilot adoption suite 114/114.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 87347b97-407c-4dbe-bae5-e8880d0fc77d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment