From aac71eb1290b6f6b47027f92ca983a335b85bd8d Mon Sep 17 00:00:00 2001 From: znetsixe Date: Tue, 26 May 2026 17:57:34 +0200 Subject: [PATCH] feat(dashboardapi): diff-skip regen via flows:started predicate (#36) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Subscribes to Node-RED's flows:started runtime event, caches the {diff} payload on the dashboardAPI source, and short-circuits the child.register handler when none of {dashboardAPI id, child id, grandchild ids} appears in diff.added/changed/removed/rewired. Predicate verified by S1 spike (#32). - src/nodeClass.js: _attachLifecycleHook subscribes, _attachCloseHandler cleans up. No-op when RED.events isn't available (unit-test friendly). - src/specificClass.js: subtreeChanged() predicate + subtreeIdsFor() helper. - src/commands/handlers.js: registerChild consults predicate before composing; logs {event:'regen-skipped', outcome:'no-diff'} when skipping. - test/basic/slice36-diff-predicate.basic.test.js: 8 cases — null/empty diff, affected/unaffected ids, tab-id over-triggering avoidance, grandchild inclusion, no-grandchild case. Cold start (no cached diff yet) always regenerates — safe default. Edge case documented in #32: when a brand-new child is wired to a registered parent, the new child's id is in diff.added but not yet in registeredChildren when flows:started fires. Mitigation (b) per spike findings: one-deploy race accepted for R&D — next deploy picks up the new registration. Closes #36 --- src/commands/handlers.js | 21 ++++++ src/nodeClass.js | 27 +++++++ src/specificClass.js | 28 +++++++ .../slice36-diff-predicate.basic.test.js | 73 +++++++++++++++++++ 4 files changed, 149 insertions(+) create mode 100644 test/basic/slice36-diff-predicate.basic.test.js diff --git a/src/commands/handlers.js b/src/commands/handlers.js index 4916ed6..0a70aa6 100644 --- a/src/commands/handlers.js +++ b/src/commands/handlers.js @@ -24,12 +24,33 @@ function resolveChildNode(childId, ctx) { // On child.register: build the dashboard graph (root + direct children) and // emit one Grafana upsert HTTP request per dashboard on Port 0. +// +// Diff-skip behavior (PRD F-1, S1 spike #32): if the latest flows:started +// payload's `diff` indicates that NEITHER the dashboardAPI itself NOR this +// child NOR its grandchildren changed, skip composition and log no-diff. The +// first call after startup (no cached diff yet) regenerates unconditionally. function registerChild(source, msg, ctx) { const childSource = resolveChildSource(msg.payload, ctx); if (!childSource?.config) { throw new Error('Missing or invalid child node'); } + const subtreeIds = source.subtreeIdsFor(ctx.node?.id, childSource); + const changed = source.subtreeChanged(source.lastFlowsStartedDiff, subtreeIds); + if (!changed) { + if (source.logger?.info) { + source.logger.info({ + event: 'regen-skipped', + outcome: 'no-diff', + trigger: 'child.register', + dashboardApiId: ctx.node?.id, + childId: childSource?.config?.general?.id, + subtreeSize: subtreeIds.size, + }); + } + return; + } + const dashboards = source.generateDashboardsForGraph(childSource, { includeChildren: Boolean(msg.includeChildren ?? true), }); diff --git a/src/nodeClass.js b/src/nodeClass.js index 3c7f7d7..00608ac 100644 --- a/src/nodeClass.js +++ b/src/nodeClass.js @@ -26,6 +26,29 @@ class nodeClass { this._attachInputHandler(); this._attachCloseHandler(); + this._attachLifecycleHook(); + } + + // Subscribe to Node-RED's `flows:started` event to cache the deploy diff so + // the child.register handler can decide whether *this* dashboardAPI's + // subtree was affected. Predicate documented in Gitea issue #32 spike. + _attachLifecycleHook() { + if (!this.RED?.events?.on) return; + this._flowsStartedListener = (payload) => { + const diff = payload?.diff || null; + this.source.lastFlowsStartedDiff = diff; + this.source.lastFlowsStartedAt = Date.now(); + if (this.source?.logger?.debug) { + const summary = diff + ? Object.fromEntries( + ['added', 'changed', 'removed', 'rewired', 'linked', 'flowChanged'] + .map((k) => [k, (diff[k] || []).length]) + ) + : null; + this.source.logger.debug({ event: 'flows:started', type: payload?.type, diff: summary }); + } + }; + this.RED.events.on('flows:started', this._flowsStartedListener); } _buildConfig(uiConfig) { @@ -78,6 +101,10 @@ class nodeClass { _attachCloseHandler() { this.node.on('close', (done) => { + if (this._flowsStartedListener && this.RED?.events?.off) { + this.RED.events.off('flows:started', this._flowsStartedListener); + this._flowsStartedListener = null; + } if (typeof done === 'function') done(); }); } diff --git a/src/specificClass.js b/src/specificClass.js index 911a316..6acbe05 100644 --- a/src/specificClass.js +++ b/src/specificClass.js @@ -168,6 +168,34 @@ class DashboardApi { return out; } + // Predicate from Gitea issue #32 spike (S1 findings). Given the diff payload + // from Node-RED's flows:started event and a set of node ids that constitute + // "my subtree", decides whether the subtree changed on this deploy. + // `null` diff (first deploy / startup) → always regen (safe default). + subtreeChanged(diff, subtreeIds) { + if (!diff) return true; + const mine = new Set(subtreeIds); + for (const field of ['added', 'changed', 'removed', 'rewired']) { + const arr = diff[field] || []; + if (arr.some((id) => mine.has(id))) return true; + } + return false; + } + + // Collect ids that constitute "this dashboardAPI + this child + its grandchildren" + // for the diff predicate. Pulls grandchildren via the existing extractChildren walk. + subtreeIdsFor(dashboardApiNodeId, childSource) { + const ids = new Set(); + if (dashboardApiNodeId) ids.add(dashboardApiNodeId); + const childId = childSource?.config?.general?.id; + if (childId) ids.add(childId); + for (const { childSource: gc } of this.extractChildren(childSource)) { + const gcId = gc?.config?.general?.id; + if (gcId) ids.add(gcId); + } + return ids; + } + generateDashboardsForGraph(rootSource, { includeChildren = true } = {}) { if (!rootSource?.config) { this.logger.warn('generateDashboardsForGraph skipped: root source missing config'); diff --git a/test/basic/slice36-diff-predicate.basic.test.js b/test/basic/slice36-diff-predicate.basic.test.js new file mode 100644 index 0000000..88508cf --- /dev/null +++ b/test/basic/slice36-diff-predicate.basic.test.js @@ -0,0 +1,73 @@ +'use strict'; + +const test = require('node:test'); +const assert = require('node:assert/strict'); + +const DashboardApi = require('../../src/specificClass.js'); + +test('subtreeChanged: null diff → always regen (safe default for cold start)', () => { + const api = new DashboardApi({}); + assert.equal(api.subtreeChanged(null, new Set(['a', 'b'])), true); + assert.equal(api.subtreeChanged(undefined, new Set(['a', 'b'])), true); +}); + +test('subtreeChanged: empty diff arrays → no regen needed', () => { + const api = new DashboardApi({}); + const diff = { added: [], changed: [], removed: [], rewired: [], linked: [], flowChanged: [] }; + assert.equal(api.subtreeChanged(diff, new Set(['a', 'b'])), false); +}); + +test('subtreeChanged: id in added → regen', () => { + const api = new DashboardApi({}); + const diff = { added: ['x', 'b'], changed: [], removed: [], rewired: [] }; + assert.equal(api.subtreeChanged(diff, new Set(['a', 'b'])), true); +}); + +test('subtreeChanged: id in changed → regen', () => { + const api = new DashboardApi({}); + const diff = { added: [], changed: ['a'], removed: [], rewired: [] }; + assert.equal(api.subtreeChanged(diff, new Set(['a', 'b'])), true); +}); + +test('subtreeChanged: only unrelated ids → no regen', () => { + const api = new DashboardApi({}); + const diff = { added: ['z'], changed: ['y'], removed: ['x'], rewired: ['w'] }; + assert.equal(api.subtreeChanged(diff, new Set(['a', 'b'])), false); +}); + +test('subtreeChanged: tab id in diff but not in subtree → no regen', () => { + // Tab id over-triggering avoidance: when an unrelated tab changes, its + // tab id lands in changed/added but should not affect this dashboardAPI. + const api = new DashboardApi({}); + const diff = { added: [], changed: ['unrelated_tab'], removed: [], rewired: [] }; + assert.equal(api.subtreeChanged(diff, new Set(['dashboardApiId', 'childA'])), false); +}); + +test('subtreeIdsFor: includes dashboardAPI id + child id + grandchild ids', () => { + const api = new DashboardApi({}); + const grandchild = { + config: { general: { id: 'gc-1' }, functionality: { softwareType: 'measurement' } }, + }; + const grandchildEntry = { child: grandchild, position: 'downstream', softwareType: 'measurement' }; + const child = { + config: { general: { id: 'child-1' }, functionality: { softwareType: 'pumpingStation' } }, + childRegistrationUtils: { + registeredChildren: new Map([['gc-1', grandchildEntry]]), + }, + }; + const ids = api.subtreeIdsFor('dApi-1', child); + assert.equal(ids.has('dApi-1'), true); + assert.equal(ids.has('child-1'), true); + assert.equal(ids.has('gc-1'), true); + assert.equal(ids.size, 3); +}); + +test('subtreeIdsFor: handles child with no grandchildren', () => { + const api = new DashboardApi({}); + const child = { + config: { general: { id: 'child-1' }, functionality: { softwareType: 'measurement' } }, + }; + const ids = api.subtreeIdsFor('dApi-1', child); + assert.equal(ids.size, 2); + assert.ok(ids.has('dApi-1') && ids.has('child-1')); +});