OCE report: add Authenticator app telemetry and turn the skill into a router, Fixes AB#3731627 - #461
Conversation
|
✅ Work item link check complete. Description contains link AB#3731627 to an Azure Boards work item. |
|
❌ Invalid work item number: AB#3731627 Click here to learn more. |
github-actions
Bot
changed the title
|
✅ Work item link check complete. Description contains link AB#3731627 to an Azure Boards work item. |
21d721e to
fbb2e38
Compare
…und it, Fixes AB#3733390 (#460) ## Why The 60-day trend section of the weekly Broker report had grown into a **catalog**. Every code with a 60-day regression got a full row and a chart, whether or not anything had changed recently — so each week the on-call engineer was asked to re-triage the same long-standing known regressions. Meanwhile the section that actually matters, *"things that need attention this week"*, had **no charts at all**. That's backwards. The report was generating noise where nothing had changed and withholding evidence where something had. ## What this does Adds `assets/scripts/classify-novelty.js` — a **novelty classifier** that reads a `bucket-trends.js --json=` sidecar and labels each key against **its own 7-week baseline**: | Label | Meaning | |---|---| | `NEW` | absent (or negligible) in the baseline, present now | | `ACCELERATING` | already elevated, and materially worse this week | | `ONGOING` | elevated but flat — a known regression, not news | | `VOLATILE` | swings wide enough that this week isn't distinguishable from noise | | `RECOVERY` / `IMPROVING` | moving the right way | | `STABLE` | nothing to say | It also clusters related codes into **families**, so a single upstream failure doesn't consume six attention slots. The classifier is the **noise gate**. Its `attention` set (`NEW` + `ACCELERATING`), plus at most 2 wins, is all that renders visibly *with charts*. Everything else collapses into a fold — still one click away, never deleted, but no longer competing for the reader's attention. The Broker report template is redesigned around that gate: - Attention rows carry a **mandatory inline sparkline**, so a claim and its evidence sit on the same line. - The 60-day section becomes a **detector, not a catalog** — capped at 6 charts. - The attention list is capped at **8 visible rows**. `validate-report.ps1` gains hard checks **13–18** so a future run can't quietly regress the gate: row-body specificity, no flat top row, no suppressed-ratio chip, mandatory `.item-spark` on attention rows, the 8-row cap, and the 6-chart cap. ## What this is *not* This does **not** change what the classifier is fed. It grades Sunday-aligned calendar weeks, which is what it did before. That window is misaligned with the rolling 7-day window the report displays — a real bug, but a **separate** one, fixed in the follow-up PR so it can be reviewed on its own evidence. Check **12** in `validate-report.ps1` is deliberately left as a reserved gap. The prose refers to checks by number, and the Authenticator profile (next PR) fills that slot — keeping the numbering stable across the stack means the follow-up's validator diff is a pure insertion rather than a renumbering. ## Verification `validate-report.ps1 -Path oncall-wow-report-2026-08-18.html` — all hard checks pass, exit 0, including 13–18: ``` [OK] All 4 visible attention row(s) carry an inline sparkline [OK] Section 2 attention list is short (4 visible row(s)) [OK] 60-day section is a detector, not a catalog (0 visible chart(s), cap 6) [OK] Section 2 row bodies are row-specific [OK] No VOLATILE/RECOVERY row headlines a WoW percentage ``` ## Stack This is **1 of 3**. Each PR is reviewable in isolation: 1. **this PR** — novelty noise gate + Broker report redesign → `master` 2. #461 — Authenticator app report + router-ify the skill → this branch 3. #462 — align the noise gate to the report's rolling window → PR 2's branch Splitting this way keeps the Broker-behaviour changes separate from the purely-additive Authenticator support, so neither has to be reviewed through the other. Fixes [AB#3733390](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3733390) --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
… router One slash command now produces both weekly reports. Previously an on-call engineer had Broker telemetry only; Authenticator app health lived in a Kusto dashboard nobody opened during a rotation. SKILL.md becomes a thin router: it resolves the reporting window ONCE, picks a mode, and dispatches. All Broker analysis moves verbatim into assets/playbooks/broker.md; the new assets/playbooks/authapp.md is its Authenticator counterpart. The two playbooks are deliberately never read into one context -- in `both` mode they run as parallel sub-agents, because their Kusto conventions are mutually incompatible (Broker must never sum(countDevices) and uses HLL; Authenticator's sum(SucceededDCount) is the correct idiom there). Interleaving them risks writing one app's numbers with the other's rules. Modes: `both` (default) | `broker` | `authapp`. Outputs land in %USERPROFILE%\android-oce-reports\ as two separate reports plus, in both mode, an oce-index-<curEnd>.html digest built by the new build-index.ps1. Two reports rather than one merged document: the apps have different owners, different triage ladders and different escalation paths, and a merged report forces every reader through the half they do not own. Authenticator coverage: scenario funnels (Passkey / Entra MFA / Entra PSI / MSA NGC+SA), error-reason decomposition, abandonment, Broker API responsiveness, version share, and an optional App Center crash layer (--skip-crashes). bootstrap-report.ps1, run-kql.ps1, validate-report.ps1 and find-suspect-prs.ps1 gain -App broker|authapp (validate-report.ps1 also gains the Authenticator check profile, filling the reserved check 12 slot); find-suspect-prs.ps1 gains -Repos so it can scan the authenticator repo instead of broker/common. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
fbb2e38 to
9e28651
Compare
| > | ||
| > | Broker rule | Authenticator reality | | ||
| > |---|---| | ||
| > | Never `sum(countDevices)` — always `dcount_hll(hll_merge(countDevicesHll))` | There are no HLL columns. Distinct devices are **pre-computed**: `sum(SucceededDCount)` is correct and is what the dashboard does. | |
There was a problem hiding this comment.
🔴 Critical — sum(<Scenario>DCount) does not produce distinct devices. Every device number in the Authenticator report is inflated, by up to ~4×.
This line is the load-bearing correctness claim for the whole Authenticator side, so I checked the MV definitions live before flagging it. The first half is right (there genuinely are no HLL columns); the second half doesn't hold.
Passkey_WebAuthN_Registration_MV_V1 is defined as:
| summarize
SucceededDCount = dcountif(DeviceInfo_Id, OperationName == "PasskeyCredentialRequestSucceeded"),
...
by EventDate = bin(EventInfo_Time, 1h), // ← HOURLY, not daily
OsLevel, AppVersion, DeviceInfoMake…DCount is a distinct count per (hour × OsLevel × AppVersion × DeviceInfoMake) cell. Summing those cells re-counts every device appearing in more than one hour or dimension cell — the same error class as Broker's sum(countDevices). The constraint is real, but "no better option in the MV" isn't the same as "correct".
Measured on the live cluster, window 2026-08-18 → 2026-08-25:
| Scenario | sum(…DCount) |
True dcount(DeviceInfo_Id) |
Inflation |
|---|---|---|---|
| Passkey WebAuthN Registration (4 group-by cols) | 4,492 | 3,662 | +22.7% |
| Entra MFA PN+CFA (6 group-by cols) | 88,355,223 | 22,497,905 | 3.93× |
The second row is the one that hurts: version-share-wow.kql uses exactly that expression as the report's telemetry-active-device proxy, and Entra_MFA_Push_Notification_And_CheckForAuth_MV_V1 groups by six columns (hourly EventDate, OsLevel, AppVersion, DeviceInfoMake, IsWorkProfile, Source).
The template itself corroborates this. authapp-report-template.html ships Telemetry-active devices: 22.8 M — matching the true dcount (22.5M), not the 88.4M the prescribed query returns. The headline KPI can't be reproduced by the query the playbook tells you to run, which suggests the template was populated by a different method than the one documented here.
Blast radius: that number is the denominator for Crashes per 1,000 active devices and Crash-free active devices, so a ~4× inflated denominator understates crash rate by ~4× — in the section most likely to trigger or suppress an escalation.
Scoping — most of the report is fine. Funnel rates come from sum(Ini)/sum(Suc) over countif columns, which are plain event counts and sum correctly. Success/Failure/Unknown rates, WoW deltas and sparklines are all sound. The defect is confined to the …Devices columns: InitiatedDevices/SucceededDevices/FailedDevices/UnknownDevices in scenario-outcomes-wow.kql, version-share Devices, and the "Devices" / "Devices affected" table columns.
Two related sub-points:
UnknownDevices = InitiatedDevices − (SucceededDevices + FailedDevices)isn't valid even with correct dcounts — a device that both succeeded and failed is in both sets, so the terms don't decompose. Thecase(…, 0)clamp hides the underflow instead of surfacing it. The event-countUnknowndirectly above it is fine; only the device variant is unsound.EventDateis an hourly bin, not a date. Worth stating prominently here — filtering still works, but this is the root cause of the summing error.
Suggested fixes, cheapest first:
- For a true device count, go to the raw table:
dcount(DeviceInfo_Id)overpasskeyoperations/entramfasdkoperationswith the MV's own filters (that's how the numbers above were produced; fast enough at 7 days). - If the MV must be the source, relabel the column honestly — "device-activity units" or "device-hours", never "Devices" — and drop it from any per-device rate denominator.
- Fix
validate-report.ps1check 12e, whose failure message currently instructs authors to "Usesum(<Scenario>DCount)" — the validator actively teaches the wrong idiom.
Flagging as critical mainly because of reach: this rule is documented in ~6 places (this file ×2, authapp.md ×3, two query headers) and enforced by check 12e, so it will propagate into every future Authenticator query unless fixed at the source.
One thing worth settling separately: does the existing Authenticator dashboard use sum(…DCount) as well? If so this is a pre-existing measurement problem that the PR would codify rather than introduce — which makes it a finding to raise beyond this PR, not a reason to match the dashboard.
There was a problem hiding this comment.
Valid, and I verified all of it independently against the live cluster rather than take the numbers on trust. Reproduced within ingestion drift:
| Scenario | sum(...DCount) |
true dcount |
mine | yours |
|---|---|---|---|---|
| Passkey Registration | 4,494 | 3,665 | +22.6% | +22.7% |
| Entra MFA PN+CFA | 88,372,219 | 22,510,358 | 3.93x | 3.93x |
The MV definitions confirm the mechanism, and the framing that made it click for me: Broker stores an HLL sketch, which retains device identity and therefore merges; Authenticator collapses to an integer per cell, after which identity is gone and only addition is possible. So the right conclusion from "there are no HLL columns" was never "therefore sum them" -- it was "therefore these views cannot answer this question at all." TotalUniqueDevices is the same trap.
Your UnknownDevices sub-point also holds: 239 devices both succeeded and failed on Passkey in that window (6.6% of succeeded), so the sets overlap and the subtraction underflows into the clamp.
Two corrections to the finding, one in each direction.
Understated: you inferred from the template's 22.8 M that it was populated by a different method. Our actual generated reports ship 86.4 M -- the inflated value. So this isn't latent in docs, it reached the reader in every Authenticator report produced so far.
Overstated: the crash-rate blast radius is latent, not live. Crashes per 1,000 active devices isn't present in the generated reports at all -- the App Center layer needs a secret and didn't run. The wiring exists, so the risk is real for any run that includes crashes, but no shipped report currently understates a crash rate.
On the question you flagged for separate settling -- you were right to ask. The Livesite Dashboard uses sum(*DCount) in 78 places and plain dcount( in zero, including sum(NotificationInitiatedDCount) x4, the exact expression our version-share query uses. So the cheatsheet's "is what the dashboard does" is factually true; what's wrong is the inference from "the dashboard does it" to "it's correct."
That makes this a pre-existing measurement problem the PR would codify rather than introduce -- so we've deliberately kept the queries unchanged. Diverging in the report alone would leave an on-call engineer comparing 22.5M in the report against 86.4M on the dashboard with no way to tell which is right, and would break comparability with every historical figure. The fix belongs upstream in the MVs and the dashboard: filed as AB#3739409, with the HLL-sketch approach as the preferred remedy and raw-table dcount as the interim.
What I did change is the claim, in a22ea90 -- your suggestion 3, plus the four doc sites. The validator's failure text was the most consequential of them, since it instructed authors to "Use sum(<Scenario>DCount)" every time it fired and so propagated the idiom into new queries. All five now say the idiom matches the dashboard, over-counts, and that device columns are relative indicators -- safe for ranking, direction and WoW movement, never an absolute population or a rate denominator. The cheatsheet carries the measured table and the UnknownDevices note.
No .kql touched, so report numbers are unchanged and still match the dashboard. Skill re-aligns when AB#3739409 lands.
fadidurah
left a comment
fadidurah
left a comment
There was a problem hiding this comment.
LGTM, AI didn't find anything either
…e count Review feedback on #461 (cacosta33), verified independently against the live cluster: Authenticator MVs store device counts as a plain integer per (hourly bin x dimension) cell, so summing them re-counts every device seen in more than one cell. Measured +22.6% on Passkey Registration and 3.93x on Entra MFA PN+CFA (2026-08-18 -> 2026-08-25). Broker avoids this by storing an HLL sketch, which retains device identity and merges correctly. Authenticator has no equivalent column, so these views cannot answer "how many distinct devices" at all. TotalUniqueDevices has the same problem -- it is also a per-cell dcount. The queries are deliberately left unchanged. The Livesite Dashboard uses this idiom in 78 places and plain dcount() in none, so it is the team's de-facto convention and every historical figure is calibrated to it. A report printing 22.5M against a dashboard printing 86.4M would leave an on-call engineer with two numbers and no way to choose. The fix belongs upstream in the MVs and the dashboard, tracked as AB#3739409; this skill re-aligns once that lands. What changes here is only the claim. Five places asserted the idiom was correct -- most consequentially the validator's own failure text, which instructed authors to "Use sum(<Scenario>DCount)" every time it fired and so propagated the convention into new queries. They now state that it matches the dashboard, over-counts, and that device columns are relative indicators (safe for ranking, direction and week-over-week movement) rather than absolute populations. Also documents the related UnknownDevices defect: deriving it as Initiated - (Succeeded + Failed) is unsound for devices because a device can both succeed and fail in-window (239 devices, 6.6% of succeeded), so the sets overlap and the subtraction underflows into a case(..., 0) clamp. No .kql files touched -- report numbers are unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…p review items Second review pass on #462 (Copilot reviewer). All 11 comments verified against the real 2026-08-22 reports before fixing; all 11 were valid. MOST IMPORTANT -- check 19 could never fail for Authenticator. The Authenticator template orders sections #attention -> #scoreboard -> #trend60d, so bounding the promotion span at #trend60d swallowed the scoreboard itself. Every flagged scenario then matched its own table row and counted as "promoted". Proven by stripping every attention item from a real Authenticator report: the check still reported "All 6 red/amber pill(s) are reconciled". Now bounded at the next <h2>, which is #scoreboard for Authenticator and #trend60d for Broker. Post-fix the stripped report correctly fails, naming all six scenarios, while both real reports still pass. This one is on me twice over: the previous commit touched exactly this code, and its own fixture test showed the symptom (a should-fail case passing). I dismissed it as a fixture-ordering artifact -- true for Broker, false for Authenticator -- with the anchor ordering visible in my own output. Regressions from the previous commit: * 5 example data-trend arrays in report-template.html still held 7 points under captions the sweep had changed to "8 wk". Extended to 8, preserving each series' shape. * broker.md's output checklist still specified "8-9 weekly values" and 60-day arrays ending "on the current partial week (~9 points)", contradicting the 8-complete-week contract. The sweep's patterns missed it. * "a 8-week" x2 -- introduced by the 9->8 sweep. * --family-sep=none was on the bucket-trends.js invocation; it is a classify-novelty.js flag (bucket-trends.js parses --start --end --metric --summary --include-partial-end --json --peak-floor --key) and was silently ignored. Removed; the correct classify-novelty.js usage already carries it. Pre-existing, live on master (from #461): * authapp-report-template.html lost the /* opener on the "Novelty presentation" CSS comment, so the browser parsed the prose as a selector and dropped .item-spark styling entirely. Restored to match the Broker template; comment markers now balance 21/21. "classifier WoW == displayed WoW" is Broker-only (4 comments): Broker's classifier and WoW tables count the same thing, so alignment does make the numbers equal. Authenticator aligns the WINDOW but not the MEASURE -- its 60-day query emits bad-outcome volumes (Failed + Unknown) while the scoreboard headline is a success-RATE delta in percentage points. Traffic shifts can make those diverge in magnitude, or move them the same direction while telling opposite stories. The old wording told an author to debug --start/--end on a sign mismatch, which would discard valid evidence. Qualified in SKILL.md, in the Authenticator query docblock, and in bootstrap-report.ps1, which now prints per-app wording. Also fixed a direct self-contradiction in authapp.md, where one section claimed the two shared a basis while another still described the classifier as calendar Sun-Sat. The .kql change is comment-only; query logic is untouched. Both real reports still pass every hard check. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
3 participants
Why
On-call engineers had Broker telemetry in a weekly report and Authenticator app health in a Kusto dashboard nobody opened during a rotation. The ask was explicit: one slash command, both reports — not two commands, and not one merged document.
What this does
SKILL.mdbecomes a thin router. It resolves the reporting window once, picks a mode, and dispatches. All Broker analysis moves verbatim intoassets/playbooks/broker.md; the newassets/playbooks/authapp.mdis its Authenticator counterpart.both(default)oce-index-<curEnd>.htmlbrokeroncall-wow-report-<curEnd>.htmlauthappauthapp-wow-report-<curEnd>.htmlTwo reports, not one
The two apps have different owners, different triage ladders and different escalation paths. A merged report forces every reader through the half they don't own. The
bothmode instead emits two standalone reports plus a one-page index digest linking them.The playbooks are never read into one context
In
bothmode they run as parallel sub-agents. This isn't only about wall-clock — their Kusto conventions are mutually incompatible:sum(countDevices)is actively wrong.sum(SucceededDCount)is the correct idiom.Interleaving them in one context risks writing one app's numbers under the other app's rules. The router says so explicitly, and the shared hard-rules section calls out that app-specific rules are never interchangeable.
Authenticator coverage
Scenario funnels (Passkey / Entra MFA / Entra PSI / MSA NGC+SA), error-reason decomposition, abandonment, Broker API responsiveness, version share, and an optional App Center crash layer (
--skip-crashes, since it needs a secret).Plumbing
bootstrap-report.ps1,run-kql.ps1,validate-report.ps1andfind-suspect-prs.ps1all gain-App broker|authapp.validate-report.ps1also gains the Authenticator check profile — which fills the check 12 slot deliberately reserved in the previous PR, so this validator diff is a pure insertion with no renumbering.find-suspect-prs.ps1gains-Reposso it can scan the authenticator repo instead of broker/common.New
build-index.ps1reads the headline KPI tiles out of both finished reports and emits the digest. It is a digest, not an analysis — a cross-app finding gets written into both reports, and the index just links them.Scope
This PR is purely additive Authenticator support plus the router refactor. It contains no Broker behaviour changes — those are in the parent PR, and the window-alignment fix is in the child. That separation is the whole point of the split.
SKILL.mdshrinks substantially because its Broker content moves toassets/playbooks/broker.mdrather than being deleted.Verification
Full E2E run in default
bothmode: both reports generated, both validators pass, index built. Reports land in%USERPROFILE%\android-oce-reports\— outside the workspace, so they can't be committed by accident.Stack
This is 2 of 3:
masterFixes AB#3731627