Skip to content

Rescore via static reporting-job nodes (alt. to #2414)#2417

Open
jaym wants to merge 2 commits intomainfrom
jdm/graph-rescore
Open

Rescore via static reporting-job nodes (alt. to #2414)#2417
jaym wants to merge 2 commits intomainfrom
jdm/graph-rescore

Conversation

@jaym
Copy link
Copy Markdown
Contributor

@jaym jaym commented Apr 24, 2026

Summary

Alternative approach to #2414 for score rollup via the graph executor. Same end goal — let callers (typically the server) recompute aggregate policy/control/framework/asset scores from pre-computed leaf scores without re-running MQL — but without the Producer abstraction.

The core idea: at build time, when we know which reporting jobs already have scores, swap their graph node for a StaticReportingJobNode that holds the fixed score. Aggregate reporting jobs (POLICY, CONTROL, FRAMEWORK) remain normal ReportingJobNodes and roll up their children through the existing ScoringSystem logic. No external envelope-addressing producer, no preseed, no node-state hijacking.

  • GraphBuilder.WithRescore(scores map[string]*policy.Score) flips the builder into rescore mode. In rescore mode:

    • No executionManager is started.
    • ExecutionQueryNodes, ReportingQueryNodes, DatapointNodes, and the CollectionFinisherNode are not built.
    • Each reporting job whose QrId (after the "root" → assetMrn rename) is a key in scores becomes a StaticReportingJobNode holding the supplied score; otherwise it's a normal aggregate node.
    • Execute() exits as soon as the initial priority-queue pass propagates static scores through their Notify edges and into the ScoreCollector.
  • New public entry point executor.RescoreResolvedPolicy(assetMrn, rp, scores, scoreCollector).

  • builderFromResolvedPolicy switched to nil-safe proto getters so minimal ResolvedPolicy test fixtures work.

Scope note

This PR intentionally covers Value rollup only — no RiskScore rollup. Risk-score rollup can be layered on later as an orthogonal option.

Test plan

  • go test ./policy/executor/... passes, including 8 new TestRescore_* tests.
  • TestRescore_RealResolvedPolicy exercises a real server-produced resolved policy (159 reporting jobs, 146 leaf scores) and verifies every leaf Value round-trips and every rolled-up score has a valid type.
  • go test ./policy/... passes.
  • go build ./... clean.
  • End-to-end sanity: cnspec scan local still runs normally (the rescore path is opt-in via WithRescore; the default ExecuteResolvedPolicy path is unchanged apart from the nil-safe getters in builderFromResolvedPolicy).

🤖 Generated with Claude Code

Introduces a rescore mode in the graph executor: GraphBuilder.WithRescore
wires reporting jobs whose QrId is in the supplied scores map as
StaticReportingJobNodes holding the pre-computed score, while aggregate
reporting jobs still roll up their children via the normal
ReportingJobNode. No queries run, no executionManager, no datapoint
finisher — the initial priority-queue pass propagates static scores
through the graph and the executor exits.

This gives the server a way to recompute aggregate policy, control,
framework, and asset scores from pre-computed leaf scores (e.g. after
an exception or scoring-system change) without re-executing any MQL.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@mondoo-code-review mondoo-code-review Bot left a comment

Choose a reason for hiding this comment

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

New rescore mode enables re-aggregating policy scores without re-running queries

Additional findings (file/line not in diff):

  • 🔵 policy/executor/internal/graph.go:104 — Every node (including static leaf nodes) is pushed into the priority queue with maxPriority. In rescore mode all static nodes will emit their score on the first recalculate(), and all aggregate nodes will also recalculate() — but aggregate nodes will produce nil until their children have delivered scores via consume. Because the priority queue uses maxPriority for every initial push, processing order within the first drain depends on insertion order from the map iterator (non-deterministic).

This works correctly today because recalculate returning nil just means no propagation, and the child's recalculate will re-push the parent. However, this means aggregate nodes may be popped and recalculated multiple times: once during the initial sweep (returning nil), then again after each child delivers a score.

Consider seeding only leaf (static) nodes into the initial queue in rescore mode, or using the already-computed priorityMap values instead of maxPriority for the initial push, to avoid redundant recalculations on aggregate nodes.

Comment thread policy/executor/internal/nodes.go
Comment thread policy/executor/rescore_test.go
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 24, 2026

Test Results

  1 files  ±0   38 suites  ±0   1m 21s ⏱️ -20s
709 tests +8  708 ✅ +8  1 💤 ±0  0 ❌ ±0 
710 runs  +8  709 ✅ +8  1 💤 ±0  0 ❌ ±0 

Results for commit a2d1cc4. ± Comparison against base commit 96c926c.

♻️ This comment has been updated with latest results.

…consume

In rescore mode, only seed StaticReportingJobNode entries into the initial
priority queue; aggregate nodes will be queued via consume + cascade once
a static node emits, eliminating redundant nil-recalculates during the
initial sweep.

Surface mis-wiring on the static node: consume() now emits a debug log
instead of silently dropping the envelope, so an inadvertently-added
inbound edge is visible during development.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@mondoo-code-review mondoo-code-review Bot left a comment

Choose a reason for hiding this comment

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

Both previous findings have been addressed: static nodes now log unexpected consume calls, and rescore mode skips non-static seeds.

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