Skip to content

Commit 4e153b6

Browse files
committed
refactor: split graph engine and fix sparse metric evaluation
Separate data access, rule registry and traversal with explicit parameters and binding checks. Preserve event counts from one interval and observed top-N waits. Pass explicit JS test paths for Node 18 and 22 compatibility.
1 parent 7af8440 commit 4e153b6

10 files changed

Lines changed: 3651 additions & 3121 deletions

‎README.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1331,7 +1331,9 @@ animates to make room for their full content, with the clicked node anchored
13311331
and zoom unchanged; click the node again to close. Reduced-motion preferences
13321332
are respected. The module lives in
13331333
`src/pg_diag/render/graph/` (`graph.json` is the declarative graph,
1334-
`pg-diag-graph.js` the evaluation engine, `pg-diag-graph-render.js` the
1334+
`pg-diag-graph-data.js` handles data access and time-series helpers,
1335+
`pg-diag-graph-rules.js` contains the evaluation rules,
1336+
`pg-diag-graph.js` traverses the graph and combines scores, `pg-diag-graph-render.js` is the
13351337
renderer) and is specified in `DIAGNOSTIC_GRAPH_SPEC.md` next to it.
13361338

13371339
Repeated table samples store their column schema once in `snapshot_schemas` and

‎src/pg_diag/render/graph/DIAGNOSTIC_GRAPH_SPEC.md‎

Lines changed: 65 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,25 @@ computed here from the item data.
1818
| File | Purpose |
1919
| --- | --- |
2020
| `graph.json` | Declarative graph: nodes, parent links, cause links, item bindings, requirements. Data only. Validated by `tests/unit/test_diagnostic_graph.py` against `content/report.yaml`. |
21-
| `pg-diag-graph.js` | Engine (UMD, global `PgDiagGraph`, also `require`-able in node): artifact accessors, fact extractors, node evaluators, traversal, coverage and hints. No DOM access. |
21+
| `pg-diag-graph-data.js` | Status-aware artifact access, decoding, facts, timestamp alignment, windows and unit conversion. UMD global `PgDiagGraphData`; no DOM access. |
22+
| `pg-diag-graph-rules.js` | Thresholds, shared calculations and one registry of raw evaluators. UMD global `PgDiagGraphRules`; depends only on data. |
23+
| `pg-diag-graph.js` | Public `PgDiagGraph.evaluate` API, traversal, pressure/caps, propagation, coverage and hints. UMD/CommonJS; depends on data and rules, with no DOM access. |
2224
| `pg-diag-graph-render.js` | Renderer (global `PgDiagGraphRender`): SVG edges and nodes, expandable inline detail cards with bound items and hints, animated tree layout. Uses the report theme through CSS variables only. |
2325
| `pg-diag-graph.css` | Structural styles plus the score gradient stops as `--dg-*` variables. |
2426
| `DIAGNOSTIC_GRAPH_SPEC.md` | This document. |
2527

26-
The renderer inlines the three assets and the graph definition into
28+
The renderer inlines the assets and the graph definition into
2729
`templates/report.html` through `render/html.py` placeholders
2830
(`__PG_DIAG_GRAPH_CSS__`, `__PG_DIAG_GRAPH_DEFINITION__`, `__PG_DIAG_GRAPH_JS__`,
2931
`__PG_DIAG_GRAPH_RENDER_JS__`). The page script exposes
3032
`window.pgDiagReport.navigateToItem(itemId)` so the graph can scroll the report
3133
to a bound item with the same behaviour as a related-item link in an instruction.
3234

35+
`__PG_DIAG_GRAPH_JS__` contains data, rules and engine in that order. Python
36+
concatenates these resources; no bundler, external script fetch or new build step
37+
is needed. CommonJS resolves the same dependencies with `require`. The public
38+
`PgDiagGraph` entry point and evaluation result retain their shape.
39+
3340
## 2. Graph model
3441

3542
`graph.json`:
@@ -74,6 +81,12 @@ Rules:
7481
Mixed log tables (`server_lifecycle`, `system_incidents`, and the compatibility
7582
`crash_recovery_events`) use named, event-aware evaluators instead of a blanket
7683
row weight; a binding may provide context without proving a fault in that branch.
84+
- Every read by a rule or its helpers MUST be declared in the node's bindings
85+
or an ancestor's bindings. Shared resource inputs belong to grouping nodes as
86+
facts; specific diagnostic sources belong to the consuming node. Ancestor
87+
facts authorize reads but do not open a child's evaluation gate. Tests audit
88+
actual reads through `evaluate(..., {onRead(nodeId, itemId, method)})`, including
89+
empty-source fallbacks and resource-pressure calculations.
7790
- Every item id declared in `content/report.yaml` MUST be bound to at least one
7891
node, and every bound id MUST exist there. Items that exist in the catalog but
7992
not in a given artifact (older artifact, filtered report, one-shot mode) are
@@ -86,6 +99,11 @@ Rules:
8699
- `evaluator` names a function in the engine registry. Nodes without it use
87100
the `generic` evaluator (section 4). Roots and pure grouping nodes use
88101
`aggregate`.
102+
- `params` specifies inputs for a parameterized evaluator. Throughput uses
103+
`metric`; interface counters use `event` (`errors`/`drops`); TCP/UDP settings
104+
use `protocol`; client waits use `waitEvent` (`ClientRead`/`ClientWrite`).
105+
Missing or invalid values produce an evaluator error. No rule infers its
106+
behavior from a node id suffix; renaming nodes must preserve calculations.
89107
- `pressure`, when present, identifies the symptom against which a candidate
90108
contributor is assessed: `cpu_user`, `cpu_system`, `cpu_iowait`, `ram`, or
91109
`disk`. Damping is applied once, after the evaluator, not once per helper.
@@ -127,7 +145,7 @@ Resource branches follow observed symptoms, then possible contributors:
127145

128146
1. `PgDiagGraph.evaluate(artifact, definition)` indexes `artifact.items` and
129147
classifies every binding: `present` (collection status `ok` and the result
130-
has at least one table row, one finite chart point, or non-empty text),
148+
has at least one table row, a usable chart series, or non-empty text),
131149
`empty` (status `ok`/`empty` without data), `skipped`, `unsupported`,
132150
`error`, or `absent` (not in the artifact).
133151
2. Leaves are evaluated first, then parents, then roots (post-order over the
@@ -150,18 +168,47 @@ Resource branches follow observed symptoms, then possible contributors:
150168
`server_log.*` item was collected, an absent log item is a catalog
151169
difference and produces no log hint.
152170

171+
The evaluator context is created by `createAccess`: `rows`, `series` and `text`
172+
read only sources with status `ok`. `item` also permits status `empty` for hidden
173+
zero metadata. Failed, skipped, unsupported and unknown-status payloads cannot
174+
contribute even if retained in the artifact. Raw decoders remain public for
175+
inspection; diagnostic rules use the context. Each series needs two finite
176+
samples, except event charts and event counter deltas which need one. Valid positive observations may
177+
still be used with collection warnings; hidden zeros require complete coverage.
178+
153179
## 4. Evaluation
154180

155181
Scores are numbers in `[0, 1]`; `status` is `ok` (< 0.34), `warn` (< 0.67),
156182
`crit` (≥ 0.67), or `no_data`. Helpers:
157183

158184
- `scale(value, warn, crit)` maps a metric to `[0, 1]`: 0 below `warn`, 1 above
159185
`crit`, linear in between (reversed when `warn > crit`).
186+
- Rules return raw scores. The engine applies resource pressure once, then the
187+
node cap, then takes the maximum with children. There are no registration-time
188+
wrappers or evaluator-local damping defaults.
160189
- Chart statistics use finite points only: `mean`, `max`, `p95`, `last`, `n`.
161190
A series with fewer than 2 finite points is treated as absent.
162-
Event charts (`tooltip_kind = log_event` or `query_event`) are an exception:
163-
one finite point is a real event and counts as present, including when it
164-
represents many occurrences in one minute.
191+
Event charts (`tooltip_kind = log_event` or `query_event`) and event counter
192+
deltas are exceptions: one finite point is already measured evidence,
193+
including many deadlocks or writer-pressure events in a single interval.
194+
Counter deltas with unit `count` are identified by `semantic_role`; older
195+
artifacts may identify event counts by quantity or an event-specific unit.
196+
A count-valued gauge such as sessions still requires two finite samples.
197+
- `alignSeries` matches equivalent UTC instants, not array positions. All
198+
internal sums explicitly choose `missing: "strict"` for components of a total,
199+
or `missing: "observed"` for independent events and sparse top-N wait profiles. Strict sums retain gaps as
200+
unknown; ordinary time-series statistics still require two comparable points
201+
after alignment. Duplicate timestamps are ambiguous and remain unknown.
202+
The public `sumSeries` helper retains index fallback for legacy value-only
203+
arrays; artifact chart series always use timestamps.
204+
- Wait-profile LWLock/Buffer/IPC counts are evaluated independently of the
205+
availability of a total. Their share uses sums over the observed window, not
206+
means of series with different sample counts, and describes the observed
207+
top-N session samples rather than all backend activity.
208+
- Window and unit helpers live in the data module: actual delta duration precedes
209+
runtime fallback, log wall-clock durations do not use the browser timezone,
210+
relation blocks use `block_size`, and I/O operations use byte counters or
211+
`op_bytes`.
165212
- Table cells are decoded by column `encoding`: `decimal_string` → number
166213
(`Number()`; values above 2^53 lose precision, which is acceptable for
167214
scoring), `json_number` as is, `json_boolean` as is, others as text.
@@ -257,9 +304,11 @@ Scores are numbers in `[0, 1]`; `status` is `ok` (< 0.34), `warn` (< 0.67),
257304
- Deadlock charts and endpoint deltas observe the same counter. Use their
258305
maximum as the window observation, never their sum; keep the separate log
259306
window labelled and do not add it to snapshot counts.
260-
- Shared evaluator caches include explanations, facts and evidence alongside
261-
the value and replay them into each consuming node. Repeated bloat and
262-
resource-pressure evidence must not depend on root traversal order.
307+
- Shared diagnostic calculations run in each consuming node's context, keeping
308+
reasons, facts and dependency reads together. Raw artifact decoding is cached
309+
by result identity; artifacts are treated as immutable during evaluation.
310+
No node-scoped evidence or pressure score is cached across nodes, so traversal
311+
order cannot change the result.
263312
- Fact extractors read inventory items: CPU count from `os.cpu_info`, RAM from
264313
`os.memory_info`/`os.total_ram`, disk media from `os.lshw_disk` (NVMe / SSD /
265314
rotational), build flags from `overview.pg_config` (`--enable-cassert`,
@@ -388,7 +437,7 @@ and runbook sections 3.6, 4.5 and 6.
388437
without host items), post-order/max propagation, hint generation, log-source
389438
fallback and overlap, typed log signals, idle versus lagging standby, CPU
390439
component separation, I/O-wait-specific contributors, missing pressure,
391-
separate swap/available-memory signals, and cached explanations in repeated
440+
separate swap/available-memory signals, and complete explanations in repeated
392441
branches. `diagnostic_graph_render.test.js` tests sibling alignment, subtree
393442
bounds, wrapped label separation, full-height inline cards and obstacle-free
394443
cause routes with cards open.
@@ -403,6 +452,12 @@ and runbook sections 3.6, 4.5 and 6.
403452
positions, open/close symmetry, anchor preservation, rapid interruptions,
404453
reduced motion, complete card content and cleanup during re-render.
405454

455+
`tests/js/diagnostic_graph_architecture.test.js` checks the single registry and
456+
explicit parameters, real dependency reads, failed-payload isolation across all
457+
rules, sample eligibility, timestamp/missing-value policies, source windows,
458+
pressure/cap/child ordering, renamed/reordered graph equivalence and browser UMD
459+
versus CommonJS parity. Existing metric regression and renderer tests remain.
460+
406461
## 7. Change policy
407462

408463
- Adding an item to the catalog requires binding it here; the unit test fails

0 commit comments

Comments
 (0)