fix(core): scope METER_VALUE_ANOMALY to cumulative energy registers per connector

.changeset/fix-meter-value-anomaly-bucketing.md

@@ -0,0 +1,5 @@
+---
+'@ocpp-debugkit/toolkit': patch
+---
+
+Fix `METER_VALUE_ANOMALY` false positives (#127). The rule flattened every `sampledValue` across measurand, phase, unit, location and connector into a single series and asserted monotonicity, but only cumulative `Energy.*.Register` measurands are monotonic and non-negative per OCPP 1.6 section 7.28, so any charge point reporting more than one measurand per sample, or any multi-connector station, produced warnings on nearly every `MeterValues` message. Readings are now bucketed by `(connectorId, measurand, phase, unit, location)` and the monotonic and non-negative checks apply only to cumulative energy registers (an absent `measurand` defaults to `Energy.Active.Import.Register`); other measurands are ignored by this rule. Reported by shiv3 from the ocpp-cp-simulator integration.

CURRENT_STATE.md

@@ -4,21 +4,35 @@
 
 ## Current Version
 
-`0.3.1` — Integrations & OSS Credibility (published)
+`0.4.0`, Open OCPP Trace Interop (published 2026-07-17). A `0.4.1` patch is in
+progress (METER_VALUE_ANOMALY false-positive fix, #127).
 
 ## Active Milestone
 
-**v0.4.0 - Open OCPP Trace Interop (feature complete)**
+**v0.4.1 - detection correctness patch, then v0.5.0 (OCPP 2.0.1)**
 
-v0.3.1 is published to npm (16 detection rules, 15 scenarios, trace diffing,
-rich scenario assertions, and the ci/anonymize/diff CLI commands). The interop
-milestone reads and writes the shared Open OCPP Trace v1.1 interchange format,
-checked against the specification's conformance fixtures: the input adapter
-(#121) and the exporter with the `convert` CLI command (#122) have both
-landed. Remaining: cut the `v0.4.0` release.
+v0.4.0 shipped the Open OCPP Trace interop: the toolkit reads and writes the
+shared v1.1 interchange format (input adapter #121, exporter + `convert` CLI
+#122), checked against the specification's conformance fixtures. **0.4.1**
+fixes a false-positive bug shiv3 found from the simulator integration: the
+`METER_VALUE_ANOMALY` rule flattened all measurands and connectors into one
+series (#127). Next milestone is **v0.5.0 (OCPP 2.0.1)**, where the same
+per-connector/EVSE model this fix introduces is required across detection.
 
 ## What's Done
 
+### METER_VALUE_ANOMALY correctness fix (0.4.1, Issue #127)
+
+- ✅ Rule 14 now buckets readings by `(connectorId, measurand, phase, unit,
+  location)` and applies the monotonic + non-negative checks only to cumulative
+  `Energy.*.Register` measurands (absent `measurand` defaults to
+  `Energy.Active.Import.Register`); other measurands are ignored
+- ✅ Eliminates false positives on multi-measurand samples and multi-connector
+  stations; genuine energy-register anomalies still detected (meter-anomaly
+  scenario and conformance contract unchanged); 3 regression tests added
+- Companion issue drafted for the same connector-blindness in
+  `STATUS_TRANSITION_VIOLATION` (rule 8)
+
 ### GitHub Infrastructure
 
 - ✅ GitHub milestones created (M0, M0.5, v0.1.0, v0.2.0, v0.3.0, v1.0.0)

packages/toolkit/src/core/detection.test.ts

@@ -1418,6 +1418,100 @@ describe('detectFailures', () => {
       const failures = detectFailures(events, sessions);
       expect(failures.some((f) => f.code === 'METER_VALUE_ANOMALY')).toBe(false);
     });
+
+    // Regression tests for the per-(connector, measurand) bucketing fix (#127).
+    const meterSession = (meterEvents: Event[]): Event[] => [
+      makeEvent('s1', 'ms', 'Call', 'StartTransaction', { connectorId: 1, idTag: 'TAG-127' }, 1000),
+      makeEvent(
+        's2',
+        'ms',
+        'CallResult',
+        null,
+        { idTagInfo: { status: 'Accepted' }, transactionId: 42 },
+        1500,
+        'CSMS_TO_CS',
+      ),
+      ...meterEvents,
+      makeEvent(
+        's9',
+        'me',
+        'Call',
+        'StopTransaction',
+        { transactionId: 42, reason: 'Local' },
+        9000,
+      ),
+    ];
+
+    const energyPlusPower = (id: string, ts: number, energy: string): Event =>
+      makeEvent(
+        id,
+        id,
+        'Call',
+        'MeterValues',
+        {
+          connectorId: 1,
+          transactionId: 42,
+          meterValue: [
+            {
+              sampledValue: [
+                { measurand: 'Energy.Active.Import.Register', value: energy, unit: 'Wh' },
+                { measurand: 'Power.Active.Import', value: '3000', unit: 'W' },
+              ],
+            },
+          ],
+        },
+        ts,
+      );
+
+    it('does not flag a rising Energy register interleaved with a constant Power sample (#127)', () => {
+      const events = meterSession([
+        energyPlusPower('a1', 2000, '600'),
+        energyPlusPower('a2', 3000, '625'),
+        energyPlusPower('a3', 4000, '650'),
+      ]);
+      const failures = detectFailures(events, buildSessionTimeline(events));
+      expect(failures.some((f) => f.code === 'METER_VALUE_ANOMALY')).toBe(false);
+    });
+
+    it('still flags a decreasing Energy register when other measurands are present', () => {
+      const events = meterSession([
+        energyPlusPower('b1', 2000, '600'),
+        energyPlusPower('b2', 3000, '500'),
+      ]);
+      const anomalies = detectFailures(events, buildSessionTimeline(events)).filter(
+        (f) => f.code === 'METER_VALUE_ANOMALY',
+      );
+      expect(anomalies).toHaveLength(1);
+    });
+
+    it('does not flag two connector registers that share a transaction (bucketed by connectorId)', () => {
+      const meter = (id: string, ts: number, connectorId: number, energy: string): Event =>
+        makeEvent(
+          id,
+          id,
+          'Call',
+          'MeterValues',
+          {
+            connectorId,
+            transactionId: 42,
+            meterValue: [
+              {
+                sampledValue: [
+                  { measurand: 'Energy.Active.Import.Register', value: energy, unit: 'Wh' },
+                ],
+              },
+            ],
+          },
+          ts,
+        );
+      const events = meterSession([
+        meter('c1', 2000, 1, '6000'),
+        meter('c2', 3000, 2, '100'),
+        meter('c3', 4000, 1, '6100'),
+      ]);
+      const failures = detectFailures(events, buildSessionTimeline(events));
+      expect(failures.some((f) => f.code === 'METER_VALUE_ANOMALY')).toBe(false);
+    });
   });
 
   describe('UNRESPONSIVE_CSMS', () => {

packages/toolkit/src/core/detection.ts

@@ -883,34 +883,64 @@ function detectHeartbeatIntervalViolation(events: Event[]): Failure[] {
   return failures;
 }
 
+/**
+ * Cumulative energy registers, the only measurands with a monotonic,
+ * non-negative invariant per OCPP 1.6 section 7.28.
+ */
+const CUMULATIVE_MEASURANDS = new Set([
+  'Energy.Active.Import.Register',
+  'Energy.Reactive.Import.Register',
+  'Energy.Active.Export.Register',
+  'Energy.Reactive.Export.Register',
+]);
+
+/** OCPP 1.6: when `measurand` is absent it defaults to Energy.Active.Import.Register. */
+const DEFAULT_MEASURAND = 'Energy.Active.Import.Register';
+
 /**
  * Rule 14: METER_VALUE_ANOMALY
- * Detects non-monotonic (decreasing) or negative meter readings during
- * an active transaction.
+ * Flags a cumulative energy register that decreases, or is negative, during an
+ * active transaction.
+ *
+ * Only the cumulative `Energy.*.Register` measurands are monotonic and
+ * non-negative (OCPP 1.6 section 7.28). Other measurands (Power, Current,
+ * Voltage, Temperature, SoC, ...) legitimately rise and fall, so this rule
+ * ignores them. Readings are bucketed by (connectorId, measurand, phase, unit,
+ * location) so independent series never contaminate each other's monotonicity,
+ * for example a constant Power sample interleaved with a rising Energy register,
+ * or two connectors' meters on the same transaction.
  */
 function detectMeterValueAnomaly(_events: Event[], sessions: Session[]): Failure[] {
   const failures: Failure[] = [];
 
   for (const session of sessions) {
     if (session.transactionId === null) continue;
 
-    // Collect meter values from MeterValues calls in order
     const meterEvents = session.events.filter(
       (e) => e.messageType === 'Call' && e.action === 'MeterValues',
     );
-
     if (meterEvents.length === 0) continue;
 
-    // Extract numeric meter values from the payload
-    // OCPP 1.6 MeterValues: { connectorId, transactionId, meterValue: [{ timestamp, sampledValue: [{ value, ... }] }] }
-    const readings: { eventId: string; value: number }[] = [];
+    // Collect cumulative-register readings, in event order, into per-series
+    // buckets so cross-measurand and cross-connector values never mix.
+    // OCPP 1.6 MeterValues: { connectorId, transactionId, meterValue: [{ timestamp, sampledValue: [...] }] }
+    const buckets = new Map<string, { eventId: string; value: number }[]>();
 
     for (const event of meterEvents) {
       const payload = event.payload as {
+        connectorId?: unknown;
         meterValue?: {
-          sampledValue?: { value?: unknown }[];
+          sampledValue?: {
+            value?: unknown;
+            measurand?: unknown;
+            phase?: unknown;
+            unit?: unknown;
+            location?: unknown;
+          }[];
         }[];
       };
+      const connectorId =
+        typeof payload?.connectorId === 'number' ? payload.connectorId : 'unknown';
 
       const meterValues = payload?.meterValue;
       if (!Array.isArray(meterValues)) continue;
@@ -920,48 +950,57 @@ function detectMeterValueAnomaly(_events: Event[], sessions: Session[]): Failure
         if (!Array.isArray(sampledValues)) continue;
 
         for (const sv of sampledValues) {
+          const measurand = typeof sv?.measurand === 'string' ? sv.measurand : DEFAULT_MEASURAND;
+          if (!CUMULATIVE_MEASURANDS.has(measurand)) continue;
+
           const rawValue = sv?.value;
-          if (typeof rawValue === 'string') {
-            const numValue = Number.parseFloat(rawValue);
-            if (!Number.isNaN(numValue)) {
-              readings.push({ eventId: event.id, value: numValue });
-            }
-          } else if (typeof rawValue === 'number') {
-            readings.push({ eventId: event.id, value: rawValue });
-          }
+          let numValue: number;
+          if (typeof rawValue === 'string') numValue = Number.parseFloat(rawValue);
+          else if (typeof rawValue === 'number') numValue = rawValue;
+          else continue;
+          if (Number.isNaN(numValue)) continue;
+
+          const phase = typeof sv?.phase === 'string' ? sv.phase : '';
+          const unit = typeof sv?.unit === 'string' ? sv.unit : '';
+          const location = typeof sv?.location === 'string' ? sv.location : '';
+          const key = `${connectorId}|${measurand}|${phase}|${unit}|${location}`;
+
+          const bucket = buckets.get(key);
+          if (bucket) bucket.push({ eventId: event.id, value: numValue });
+          else buckets.set(key, [{ eventId: event.id, value: numValue }]);
         }
       }
     }
 
-    if (readings.length === 0) continue;
-
-    // Check for negative values
-    for (const reading of readings) {
-      if (reading.value < 0) {
-        failures.push({
-          code: 'METER_VALUE_ANOMALY',
-          description: `Negative meter value detected: ${reading.value} in session ${session.sessionId} (transaction ${session.transactionId})`,
-          severity: SEVERITY.METER_VALUE_ANOMALY,
-          eventIds: [reading.eventId],
-          suggestedSteps: SUGGESTED_STEPS.METER_VALUE_ANOMALY,
-        });
+    for (const readings of buckets.values()) {
+      // A cumulative register cannot be negative.
+      for (const reading of readings) {
+        if (reading.value < 0) {
+          failures.push({
+            code: 'METER_VALUE_ANOMALY',
+            description: `Negative meter value detected: ${reading.value} in session ${session.sessionId} (transaction ${session.transactionId})`,
+            severity: SEVERITY.METER_VALUE_ANOMALY,
+            eventIds: [reading.eventId],
+            suggestedSteps: SUGGESTED_STEPS.METER_VALUE_ANOMALY,
+          });
+        }
       }
-    }
 
-    // Check for non-monotonic (decreasing) values
-    for (let i = 1; i < readings.length; i++) {
-      const prev = readings[i - 1];
-      const curr = readings[i];
-      if (!prev || !curr) continue;
+      // A cumulative register must not decrease.
+      for (let i = 1; i < readings.length; i++) {
+        const prev = readings[i - 1];
+        const curr = readings[i];
+        if (!prev || !curr) continue;
 
-      if (curr.value < prev.value) {
-        failures.push({
-          code: 'METER_VALUE_ANOMALY',
-          description: `Non-monotonic meter reading: value decreased from ${prev.value} to ${curr.value} in session ${session.sessionId} (transaction ${session.transactionId})`,
-          severity: SEVERITY.METER_VALUE_ANOMALY,
-          eventIds: [prev.eventId, curr.eventId],
-          suggestedSteps: SUGGESTED_STEPS.METER_VALUE_ANOMALY,
-        });
+        if (curr.value < prev.value) {
+          failures.push({
+            code: 'METER_VALUE_ANOMALY',
+            description: `Non-monotonic meter reading: value decreased from ${prev.value} to ${curr.value} in session ${session.sessionId} (transaction ${session.transactionId})`,
+            severity: SEVERITY.METER_VALUE_ANOMALY,
+            eventIds: [prev.eventId, curr.eventId],
+            suggestedSteps: SUGGESTED_STEPS.METER_VALUE_ANOMALY,
+          });
+        }
       }
     }
   }