Skip to content

OCE report: align the noise gate to the report's rolling window, Fixes AB#3731628 - #462

Open
Shahzaib (shahzaibj) wants to merge 4 commits into
masterfrom
shjameel-microsoft-oce-window-fix
Open

OCE report: align the noise gate to the report's rolling window, Fixes AB#3731628#462
Shahzaib (shahzaibj) wants to merge 4 commits into
masterfrom
shjameel-microsoft-oce-window-fix

Conversation

@shahzaibj

@shahzaibj Shahzaib (shahzaibj) commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

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/26 against a report window of 07/25 → 08/01. One day of overlap.

Code Report showed Classifier saw Verdict printed
authorization_pending +63.2% −37.1% ONGOING — do not re-triage
expired_token +26.7% −51.0% ONGOING — do not re-triage

Both 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:

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 — not by convention, and not something a future change can quietly break.

--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 (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 --start drops it, leaving 8 complete weeks.

This is not "more alerts"

A/B on real data:

  • attention set went 4 → 5 keys
  • both mis-filed codes promoted to ACCELERATING
  • access_denied correctly demoted — it was actually −53.2%, a false positive the calendar window had been surfacing

Alignment 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.

⚠️ One correction to earlier framing, worth stating plainly for reviewers: the "promoted both to ACCELERATING and demoted access_denied" result is specific to the 2026-08-01 window, and the prose now says so. On other windows the same fix produces different (still correct) label changes. The claim being made here is about input correctness, not about any one code's verdict.

Also: reconciling red pills

Adds validate-report.ps1 check 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 Registration shipped carrying tag-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-warn row must now be either promoted into attention or named in a muted .reconcile-note giving 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:

  1. OCE report: add novelty noise gate and redesign the Broker report around it, Fixes AB#3733390 #460 — novelty noise gate + Broker report redesign → master
  2. OCE report: add Authenticator app telemetry and turn the skill into a router, Fixes AB#3731627 #461 — Authenticator app report + router → PR 1's branch
  3. this PR — rolling-window alignment + check 19 → PR 2's branch

Fixes AB#3731628

@shahzaibj
Shahzaib (shahzaibj) requested a review from a team as a code owner August 20, 2026 01:28
@github-actions github-actions Bot changed the title OCE report: align the noise gate to the report's rolling window OCE report: align the noise gate to the report's rolling window, Fixes AB#3731628 Aug 20, 2026
@github-actions

Copy link
Copy Markdown

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

@github-actions

Copy link
Copy Markdown

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

@shahzaibj
Shahzaib (shahzaibj) force-pushed the shjameel-microsoft-oce-authapp-v2 branch from 21d721e to fbb2e38 Compare August 25, 2026 22:05
@shahzaibj
Shahzaib (shahzaibj) force-pushed the shjameel-microsoft-oce-window-fix branch from eae15d5 to ee471b5 Compare August 25, 2026 22:05
Shahzaib (shahzaibj) added a commit that referenced this pull request Aug 25, 2026
…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>
@shahzaibj
Shahzaib (shahzaibj) force-pushed the shjameel-microsoft-oce-authapp-v2 branch from fbb2e38 to 9e28651 Compare August 25, 2026 23:55
@shahzaibj
Shahzaib (shahzaibj) force-pushed the shjameel-microsoft-oce-window-fix branch 2 times, most recently from a86b1cf to d3e1e0a Compare August 27, 2026 07:30
Shahzaib (shahzaibj) added a commit that referenced this pull request Aug 28, 2026
… 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>
@shahzaibj
Shahzaib (shahzaibj) force-pushed the shjameel-microsoft-oce-window-fix branch from d3e1e0a to b8ea91c Compare August 28, 2026 05:38
@shahzaibj
Shahzaib (shahzaibj) changed the base branch from shjameel-microsoft-oce-authapp-v2 to master August 28, 2026 05:38
@shahzaibj
Shahzaib (shahzaibj) requested a balanced review from Copilot August 28, 2026 05:42

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Comment thread .github/skills/oncall-weekly-telemetry-report/assets/scripts/validate-report.ps1 Outdated
Comment thread .github/skills/oncall-weekly-telemetry-report/assets/scripts/validate-report.ps1 Outdated
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>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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-spark array (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 |

Comment thread .github/skills/oncall-weekly-telemetry-report/assets/scripts/validate-report.ps1 Outdated
Comment thread .github/skills/oncall-weekly-telemetry-report/assets/playbooks/authapp.md Outdated
Comment thread .github/skills/oncall-weekly-telemetry-report/SKILL.md
Comment thread .github/skills/oncall-weekly-telemetry-report/assets/scripts/bootstrap-report.ps1 Outdated
Comment thread .github/skills/oncall-weekly-telemetry-report/assets/playbooks/broker.md Outdated
Comment thread .github/skills/oncall-weekly-telemetry-report/assets/playbooks/authapp.md Outdated
Shahzaib (shahzaibj) and others added 2 commits August 28, 2026 00:29
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants