Skip to content

Add authored-notebook context plumbing (#4223) - #4425

Merged
erikdarlingdata merged 5 commits into
devfrom
fix/4223-notebook-authored-context
Sep 26, 2026
Merged

erikdarlingdata merged 5 commits into
devfrom
fix/4223-notebook-authored-context

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Sep 26, 2026 •

Copy link
Copy Markdown
Owner

Refs #4223.

Why

The alert-notebook endpoint's authored templates are pure and synchronous
today: they build cells from what Map already has in scope, and never
touch a store. The next authored families need the endpoint to read a
custom alert rule (Custom:<id>) or a recent analysis finding
(Analysis: {category} [{hash8}]) before the template can render, without
turning every builder into an async, store-aware function. This is the
core step: the plumbing those families will register into, with no family
template added yet.

What changes

Darling/PerformanceMonitor.Darling.Service/AlertNotebookEndpoint.cs:

  • A new immutable record AuthoredContext(CustomRule, CustomRuleMissing, Finding, FindingMissing),
    with a shared AuthoredContext.Empty for families that need nothing.
  • AuthoredTemplateEntry gains an optional BuildCellsWithContext delegate
    alongside the existing BuildCells. Its constructor rejects an entry with
    both builders set or neither, so exactly one builder exists per entry.
    Every existing registration row (new AuthoredTemplateEntry("authored/x", V, BuildX))
    compiles unchanged.
  • AuthoredTemplateEntry.Invoke(...) is the single call site for both
    shapes: it calls BuildCellsWithContext with the context for a context
    family, or BuildCells for a plain one. The endpoint's call site and the
    existing AlertNotebookAuthoredTemplateTests theories now go through
    Invoke instead of calling BuildCells directly.
  • A new, currently empty prefix table s_authoredPrefixTemplates and
    AuthoredContextKind enum (None, CustomRule, AnalysisFinding).
    ResolveAuthored resolves exact names first (unchanged), then the
    longest matching prefix, ordinal case-insensitive; no match falls
    through to the mechanical template. ResolveAuthoredPrefixed takes the
    prefix table as a parameter so tests can exercise prefix routing against
    a table that isn't empty.
  • PrefetchAsync reads exactly the store its resolved AuthoredContextKind
    names: a Custom:<id> rule via CustomAlertRuleStore.GetAsync, or an
    Analysis: {category} [{hash8}] finding via
    DarlingAnalysisService.GetRecentFindingsAsync, matched on the finding
    whose full StoryPathHash starts with all 8 hash characters. A bad
    parse, a deleted rule, or no matching finding sets the relevant missing
    flag and returns an otherwise-empty context — never an exception,
    per Alert deliveries lack Datadog-parity structure: no structured context tags, no linked triage artifact #2710's degrade rule. A store failure (not cancellation) is reported
    through DarlingWebFailureLog.Report and adds a note to the response;
    cancellation propagates. The endpoint calls this only when the resolved
    entry has BuildCellsWithContext set and its kind isn't None, so a
    no-context family costs zero store reads.

New file Darling/Darling.Tests/AlertNotebookAuthoredContextTests.cs pins
all of the above (construction validation, Invoke routing to the right
builder, exact-vs-prefix and longest-prefix-wins routing, Custom: id
parsing, the 8-character hash match, live cancellation and missing-rule
behavior, and the no-store-read guarantee for a no-context family).

Darling/Darling.Tests/AlertNotebookAuthoredTemplateTests.cs: only the
entry.BuildCells(...) call expressions became entry.Invoke(..., AlertNotebookEndpoint.AuthoredContext.Empty);
nothing else in that file changed.

Test plan

  • dotnet build on PerformanceMonitor.Darling.Service and Darling.Tests
    (with -p:EnableWindowsTargeting=true): 0 errors on both.
  • Ran in-process on macOS (Darling.Tests.dll -class ...), against a
    throwaway timescale/timescaledb:2.30.1-pg18 container migrated with
    PgMigrations.MigrateAsync:
    • AlertNotebookAuthoredContextTests: 24/24, including four live cases against the real container:
      • a cancelled pre-fetch throws;
      • a missing rule id reads as missing, with no exception;
      • a bad id parse reads as missing, with no store read;
      • a deleted finding reads as missing, with no exception.
    • The two conditions for this shape, pinned:
      • every registered entry holds exactly one builder, and both-set or neither-set throws at construction;
      • the endpoint's pre-fetch gate (ShouldPrefetch) is false for every registered family without a context builder, so none of them makes a store read.
    • The eight-character finding match (PickFindingByHash): two findings sharing a 4-character prefix are told apart at the 8th character.
    • AlertNotebookAuthoredTemplateTests, AlertNotebookEndpointTests and DocCommentHygieneTests: 154/154 combined, at the final head.
    • AlertNotebookEndpointTests: 29/29 passed.
    • DocCommentHygieneTests (required — this PR adds members near existing
      doc blocks): 77/77 passed.
  • RED on dev: not run as a build — the new members (AuthoredContext,
    AuthoredContextKind, Invoke, ResolveAuthored, PrefetchAsync) don't
    exist on dev, so the new test file and the edited theories are a
    compile-RED against the pre-fix code, not a runtime failure.
  • Full suite: not run (time budget); the classes above cover every file
    this PR touches.
  • Lite.Tests: untouched by this PR (no Lite-side changes).
  • The local container was removed after the run.

Mutation checks

  • Zero pre-fetch for a no-context family. Removing the ShouldPrefetch check where the notebook is built (always pre-fetch) makes BuildCellsAsync_NoContextFamily_MakesZeroPrefetchCalls fail (a pre-fetch counter moved for "Blocking Detected"). Restored, all pass.
  • One builder per entry. Removing the constructor's exactly-one-builder check makes Constructor_BothBuildersSet_Throws and Constructor_NeitherBuilderSet_Throws fail. Those two are the guard; a table-wide theory can't fail while every registered row is valid, so it's a backstop only.

CHANGELOG entry

SECTION: None
ENTRY: None: internal notebook plumbing; the families that use it follow (#4223)

Adds the immutable AuthoredContext record, an optional context builder
on AuthoredTemplateEntry validated at construction (exactly one
builder), an Invoke call site, exact-then-longest-prefix routing with
an empty prefix table, and an async pre-fetch (custom rule by id,
analysis finding by 8-character hash) that runs only for context
entries.

Refs #4223.
@erikdarlingdata
erikdarlingdata marked this pull request as ready for review September 26, 2026 15:26
@erikdarlingdata
erikdarlingdata merged commit a74e857 into dev Sep 26, 2026
15 of 16 checks passed
@erikdarlingdata
erikdarlingdata deleted the fix/4223-notebook-authored-context branch September 26, 2026 15:26
erikdarlingdata added a commit that referenced this pull request Sep 26, 2026
Adds the authored Custom-rules alert notebook. It's the first family that uses the context step from #4425.

- Registers a Custom: prefix row (AuthoredContextKind.CustomRule) in s_authoredPrefixTemplates. The builder is in AlertNotebookEndpoint.Templates.CustomRules.cs.
- The notebook holds a header, a status cell and one composed panel. The panel uses the rule's own stored plan: the same source, measure or ratio, aggregate and filters. It's forced to hour buckets over the alert window. The rule's thresholds (warning and critical, or both bounds of a range) appear as the panel's reference lines.
- A deleted rule or a bad id gives a note that the rule no longer exists. A definition that can't be read gives a note saying so. Neither throws.
- The pin that the production prefix table is empty now checks that it holds exactly the registered context families.
- Tests: AlertNotebookTemplateCustomRulesTests, plus the shared template and context theories.

Refs #4223
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.

1 participant