OCE report: align the noise gate to the report's rolling window, Fixes AB#3731628 - #462
OCE report: align the noise gate to the report's rolling window, Fixes AB#3731628#462Shahzaib (shahzaibj) wants to merge 4 commits into
Conversation
|
✅ Work item link check complete. Description contains link AB#3731628 to an Azure Boards work item. |
|
✅ Work item link check complete. Description contains link AB#3731628 to an Azure Boards work item. |
21d721e to
fbb2e38
Compare
eae15d5 to
ee471b5
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>
fbb2e38 to
9e28651
Compare
a86b1cf to
d3e1e0a
Compare
… router, Fixes AB#3731627 (#461) ## 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.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. | Mode | Produces | |---|---| | `both` *(default)* | both reports **+** `oce-index-<curEnd>.html` | | `broker` | `oncall-wow-report-<curEnd>.html` | | `authapp` | `authapp-wow-report-<curEnd>.html` | ### Two 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 `both` mode instead emits two standalone reports plus a one-page index digest linking them. ### The playbooks are never read into one context In `both` mode they run as **parallel sub-agents**. This isn't only about wall-clock — their Kusto conventions are **mutually incompatible**: - Broker: HLL device counting; `sum(countDevices)` is **actively wrong**. - Authenticator: `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.ps1` and `find-suspect-prs.ps1` all gain `-App broker|authapp`. `validate-report.ps1` also 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.ps1` gains `-Repos` so it can scan the authenticator repo instead of broker/common. New `build-index.ps1` reads 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.md` shrinks substantially because its Broker content **moves** to `assets/playbooks/broker.md` rather than being deleted. ## Verification Full E2E run in default `both` mode: 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**: 1. #460 — novelty noise gate + Broker report redesign → `master` 2. **this PR** — Authenticator app report + router → PR 1's branch 3. #462 — align the noise gate to the report's rolling window → this branch Fixes [AB#3731627](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3731627) --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The novelty classifier graded Sunday-aligned CALENDAR weeks while the report displays a ROLLING 7-day window. The two windows drift apart by up to six days, so anything that turned in the last ~6 days -- exactly the period an on-call engineer cares about most -- was structurally invisible to the gate. Measured on the 2026-08-01 run: the gate's "current" week was 07/19-07/26 against a report window of 07/25-08/01, ONE day of overlap. authorization_pending read +63.2% in the report and -37.1% to the classifier; expired_token +26.7% vs -51.0%. Both were filed "ONGOING -- do not re-triage" directly beneath their own rising numbers. On the Authenticator side the same defect surfaced as red scoreboard pills (rolling-derived) above an empty "Needs attention" section (calendar-derived). Fix: bucket the 60-day trend and the sparklines with bin_at(<TIME>, 7d, datetime(<TREND_END>)) instead of startofweek(<TIME>). The final bucket then IS the report's displayed window, every bucket is a complete 7 days, and classifier WoW == displayed WoW by construction. --include-partial-end and TREND_CLASS_END become obsolete and are removed; bucket-trends.js now warns if --end is omitted, because its partial-end auto-drop heuristic would otherwise discard a genuinely complete final bucket. Because 60 is not a multiple of 7 the OLDEST bucket is the partial one -- the safe end to be partial on -- and --start drops it, leaving 8 complete weeks. A/B on real data: attention went 4 -> 5 keys; both mis-filed codes promoted to ACCELERATING; access_denied correctly DEMOTED (actually -53.2%, a former false positive). Alignment adds real signal and removes phantom signal -- it is not merely "more alerts". Adds validate-report.ps1 check 19: every red/amber scoreboard pill must be either promoted into the attention section or named in a .reconcile-note explaining why it is not escalated (below the classification floor / within its own normal band / ONGOING and flat). This closes the reported "red pill above an empty attention section" confusion at the report level, independent of window alignment. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
d3e1e0a to
b8ea91c
Compare
There was a problem hiding this comment.
Pull request overview
Aligns OCE trend classification and sparklines with the report’s rolling seven-day window and adds reconciliation for unexplained warning pills.
Changes:
- Replaces calendar-week bucketing with
curEnd-anchored rolling buckets. - Updates Broker and Authenticator templates, queries, and playbooks.
- Adds validator check 19 for warning-pill reconciliation.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
SKILL.md |
Documents rolling-window and reconciliation rules. |
template-readme.md |
Updates sparkline and trend guidance. |
report-template.html |
Updates Broker template and KQL example. |
authapp-report-template.html |
Reorders sections and adds reconciliation guidance. |
validate-report.ps1 |
Adds reconciliation validation. |
bucket-trends.js |
Adapts bucketing behavior and warnings. |
bootstrap-report.ps1 |
Computes rolling trend boundaries. |
wow-table-sparkline-series.kql |
Aligns Broker row sparklines. |
queries/README.md |
Updates query-token documentation. |
scenario-sparkline-series.kql |
Aligns Authenticator sparklines. |
scenario-60d-trend.kql |
Aligns Authenticator trends. |
authapp/README.md |
Updates Authenticator query workflow. |
60d-trend-types.kql |
Aligns error-type trends. |
60d-trend-codes.kql |
Aligns error-code trends. |
broker.md |
Updates Broker reporting workflow. |
authapp.md |
Updates Authenticator workflow and reconciliation. |
kusto-cheatsheet.md |
Documents Broker rolling bucketing. |
authapp-kusto-cheatsheet.md |
Documents Authenticator rolling bucketing. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Review feedback on #462 (Copilot reviewer), all four verified against the real 2026-08-22 reports before fixing. 1. Check 19 scanned only the first pill-bearing table. $scoreEnd stopped at the first <h2> after the first anchor, so on the Broker report it examined 8,066 of 124,728 characters -- every error-type pill was exempt. A tag-bad BrokerCommunicationException row sat in the unscanned remainder and passed only because it happened to be reconciled elsewhere. Now collects all of scoreboard / error-codes / error-types. Broker pill count goes 8 -> 9. 2. Check 19 counted HTML comments as reconciliation. Both templates carry an instructional comment INSIDE the attention section naming `Passkey WebAuthN Registration` as the worked example of an unreconciled pill -- the exact scenario the check was written for. Any report retaining that comment got a free pass on the motivating case. Comments are now stripped before matching. Verified with a four-case truth table: key only in a comment -> FAIL; key in a real .reconcile-note -> PASS; key absent -> FAIL; only the error-codes key reconciled -> FAIL naming the error-types key. 3. The 8-vs-9 week contract. The pipeline emits 8 complete rolling buckets (completeWeeks=8) and the shipped reports render "8 wk", but 25 references across both playbooks, both templates, classify-novelty.js and the validator still said 9 weeks. Correctness depended on the author following bootstrap output over the template text. Swept to 8, including two that needed rewording rather than a digit swap: the classifier history-depth note in broker.md, and the 60-day method blurb in report-template.html, which now describes rolling buckets and the dropped 4-day oldest bucket instead of "drop the partial start week". 4. run-kql.ps1's own 60-day example still used startofweek() and the old cutoff -- the precise pattern this PR removes. Anyone copying it would recreate the misaligned classifier input. Updated to bin_at(..., 7d, <TREND_END>) with a half-open filter, plus a note saying why not to substitute startofweek(). No .kql files touched. Both real reports still pass every hard check. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 11 comments.
Suppressed comments (1)
.github/skills/oncall-weekly-telemetry-report/assets/templates/template-readme.md:16
- The Authenticator template still contains nine values in every KPI
data-sparkarray (lines 487–492 and 869–870), despite this new eight-value contract. Bootstrap copies those examples and the validator does not check array length, so a generated report can retain nine points while claiming rolling eight-week coverage. Align the template arrays and enforce the expected length.
| KPI tiles (`.kpi-grid`) | Value, delta, `data-spark` array (8 rolling-week numbers) per tile |
…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>
The bug
The novelty classifier graded Sunday-aligned calendar weeks. The report displays a rolling 7-day window. Those two windows drift apart by up to six days — so anything that turned in the last ~6 days, exactly the period an on-call engineer cares about most, was structurally invisible to the noise gate.
Measured on the 2026-08-01 run: the gate's "current" week was
07/19 → 07/26against a report window of07/25 → 08/01. One day of overlap.authorization_pendingexpired_tokenBoth were filed "don't look at this" directly beneath their own rising numbers. On the Authenticator side the same defect surfaced differently: red scoreboard pills (rolling-derived) sitting above an empty "Needs attention" section (calendar-derived) — the exact confusion reported.
The fix
Bucket the 60-day trend and the sparklines with:
instead of
startofweek(<TIME>).The final bucket then is the report's displayed window, every bucket is a complete 7 days, and classifier WoW == displayed WoW by construction — not by convention, and not something a future change can quietly break.
--include-partial-endandTREND_CLASS_ENDbecome obsolete and are removed.bucket-trends.jsnow warns if--endis omitted, because its partial-end auto-drop heuristic (if (!endArg …)) would otherwise silently discard a genuinely complete final bucket — under rolling alignment a real 70% collapse could be thrown away as "looks partial".Because 60 isn't a multiple of 7, the oldest bucket is the partial one — the safe end to be partial on — and
--startdrops it, leaving 8 complete weeks.This is not "more alerts"
A/B on real data:
ACCELERATINGaccess_deniedcorrectly demoted — it was actually −53.2%, a false positive the calendar window had been surfacingAlignment adds real signal and removes phantom signal. A controlled A/B over the affected window confirmed the classifier's WoW input matched the displayed WoW on 8 of 8 sampled codes after the change, versus 0 of 8 before.
Also: reconciling red pills
Adds
validate-report.ps1check 19. Scoreboard tables colour a row from its own rolling delta; the attention section is populated from the classifier's novelty verdict. Those answer different questions, so a row can legitimately be red in the table and legitimately absent from attention — but a reader who sees that mismatch unexplained concludes the report is broken.Precedent:
Passkey WebAuthN Registrationshipped carryingtag-bad(−1.27 pts, worst delta in its table) directly above the words "Quiet week — 0 NEW or ACCELERATING". Both statements were true — the scenario peaks at ~732 bad-outcome devices, below the 1,000-device classification floor, so it is structurally excluded and can never appear in attention however sharply it moves.Every
tag-bad/tag-warnrow must now be either promoted into attention or named in a muted.reconcile-notegiving the reason, tested in order: (1) below the classification floor, (2) within its own normal band, (3) ONGOING and flat. Check 19 hard-fails an unreconciled pill.This closes the "red pill above an empty attention section" confusion at the report level, independently of window alignment.
Stack
This is 3 of 3:
masterFixes AB#3731628