Skip to content

feat(meter-values): coherent MeterValues generator (issue #40)#1935

Draft
jerome-benoit wants to merge 12 commits into
mainfrom
feat/issue-40-coherent-meter-values
Draft

feat(meter-values): coherent MeterValues generator (issue #40)#1935
jerome-benoit wants to merge 12 commits into
mainfrom
feat/issue-40-coherent-meter-values

Conversation

@jerome-benoit

@jerome-benoit jerome-benoit commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a physics-based coherent MeterValues generator for OCPP 1.6 and 2.0.x transactions (closes #40). Emitted voltage, current, power, imported-energy register, and SoC are all derived from a single physics chain, enforcing three invariants by construction:

  • INV-1: P = V × I × phases (AC) / P = V × I (DC). Aggregate residual bounded by ROUNDING_SCALE half-width (≤ 0.005 W). Per-phase L-N Power residual bounded by 2 × ROUNDING_SCALE half-width (≤ 0.01 W = aggregate emit + per-phase division).
  • INV-2: SoC(t+1) ≥ SoC(t) and ΔSoC = ΔE / batteryCapacityWh × 100 — monotone, saturating at 100 %.
  • INV-3: ΔE = P_clamped × Δt / 3_600_000 (per-sample intermediate; not an emitted OCPP measurand) and E(t+1) ≥ E(t) — energy register monotone non-decreasing.

Output is deterministic: identical (randomSeed, transactionId) inputs produce identical PRNG streams (and therefore identical per-measurand noise sequences); emitted energy and SoC additionally depend on intervalMs and elapsed session time.

Changes

New module (src/charging-station/meter-values/):

  • CoherentMeterValuesGenerator.ts — physics sample computation, energy-register ownership, session lifecycle helpers, measurand-allow-list filtering per OCPP 2.0.1 SampledDataCtrlr.Tx{Started,Updated,Ended}Measurands and OCPP 1.6 MeterValuesSampledData. resolveEnabledMeasurands uses presence-aware semantics: key-absent defaults to Energy.Active.Import.Register for ergonomic parity, key-present honors the CSV verbatim (empty ⇒ empty allow-list per OCPP 2.0.1 J02.FR.11). Per-phase measurand emission iterates every matching template and dispatches by (measurand, phase) under a balanced 3-phase Y assumption:
    • Voltage: L-N phases emit the sampled phase voltage; L-L phases emit √phases × sampled phase voltage (skipped when phases < 2); N emits 0.
    • Power.Active.Import: aggregate emits total; L-N emits P / phases; L-L, N, and phases <= 0 are unsupported (log-and-skip).
    • Current.Import: any line phase (L1/L2/L3 or L1-N/L2-N/L3-N) emits sample.currentA; N emits 0; L-L is unsupported.
    • Energy.Active.Import.Register: aggregate emits total register; L-N emits register / phases (per-phase energy contribution — Σ across L-N templates equals the aggregate register within emit-unit rounding granularity); L-L, N, and phases <= 0 are unsupported. OCPP 2.0.1 RegisterValuesWithoutPhases is not consulted; per-phase emission is driven by the connector template's phase qualifier.
    • SoC: aggregate scalar; phase-qualified templates rejected.
    • Unit-conversion factor selection extracted to resolveUnitDivider(measurand, unit) + KILO_UNIT_BY_MEASURAND Map at module scope.
  • EvProfiles.ts — piecewise-linear charging-curve interpolation and weighted EV profile selection. interpolateChargingCurve has a debug-only sortedness assertion with typed BaseError for test-time safety; production path (loadEvProfilesFile) sorts in place.
  • Prng.ts — Mulberry32 PRNG with FNV-1a label-based stream splitting.
  • types.tsCoherentSession, EvProfile, ICoherentContext, and Zod validation schemas.
  • index.ts — barrel with 11 externally-consumed symbols; internal helpers are intentionally not re-exported.

Integration (src/charging-station/ocpp/):

  • OCPPServiceUtils.ts — strategy gate in buildMeterValue: routes to coherent path when coherentMeterValues=true and a session exists; legacy path is unchanged otherwise.
  • OCPP16ResponseService.ts and OCPP20ResponseService.ts — create coherent session on StartTransaction / TransactionEvent(Started) accept. Both beginEndMeterValues extra MeterValues.req send sites (start + outOfOrderEndMeterValues) gated on isNotEmptyArray(sampledValue), composing the Signed Meter Values whitepaper §3.3.4 (StartTxnSampledData present + non-empty) with the legacy MeterValuesSampledData fallback: both empty ⇒ no send (no schema-suspect empty wrapper).
  • OCPP20ServiceUtils.ts and OCPP20IncomingRequestService.ts — create coherent session in the request-builder path so Transaction.Started MeterValues route through the coherent gate (idempotent w.r.t. the response-handler call). Both request-time create sites destroyCoherentSession on sendTransactionEvent failure, symmetric with the OCPP 1.6 resetConnectorOnStartTransactionError pattern. Both thread TxUpdatedMeasurands for the periodic and TriggerMessage MeterValue paths, and the TriggerMessage handler threads OCPP20ReadingContextEnumType.TRIGGER so emitted SampledValue.context correctly reflects the trigger origin per OCPP 2.0.1 §3.66. Empty-allow-list TransactionEvent(Updated) on periodic and TriggerMessage(MeterValues) paths sends the event with the meterValue field omitted (spec-strict per J02.FR.11; the message itself is still delivered per E02.FR.10 — the JSON schema minItems: 1 would reject an empty sampledValue wrapper).
  • OCPP16ServiceUtils.ts — coherent branch in buildTransactionBeginMeterValue threads vendor param StartTxnSampledData when configured (per OCPP 1.6 Signed Meter Values whitepaper) with fallback to MeterValuesSampledData.
  • OCPP16ResponseService.ts and OCPP20ServiceUtils.ts — destroy coherent session before the postTransactionDelay sleep to prevent state leaks; OCPP 2.0 handleResponseTransactionEvent also destroys defensively for the unknown-connector Ended branch with a diagnostic warn, symmetric with OCPP 1.6.

Canonical defaults (src/utils/Constants.ts):

  • Constants.DEFAULT_COHERENT_RAMP_UP_DURATION_MS = 5000
  • Constants.DEFAULT_COHERENT_VOLTAGE_NOISE_PERCENT = 0.01
  • Constants.MS_PER_HOUR = 3_600_000
  • Constants.UNIT_DIVIDER_KILO = 1000
    Tunables and shared physical constants live in the canonical defaults map per AGENTS.md; local duplicates removed from OCPPServiceUtils.ts and CoherentMeterValuesGenerator.ts.

New template fields (src/types/ChargingStationTemplate.ts):

  • coherentMeterValues — opt-in flag.
  • evProfilesFile — path to EV profiles JSON (relative to src/assets/).
  • randomSeed — optional deterministic seed; falls back to FNV-1a hash of hashId.

Tests (new):

  • tests/charging-station/meter-values/CoherentMeterValuesGenerator.test.ts — physics invariants, unit conversion, energy-register ownership, determinism, capacity-clamp INV-1 residual, defensive NaN/finite guards, runtime PRNG isolation, per-phase emission (L-N/L-L/N × Voltage/Power/Current/Energy).
  • tests/charging-station/meter-values/StrategyDispatch.test.ts — strategy gate: flag off, flag on + session, flag on + no session.
  • tests/charging-station/meter-values/EvProfiles.test.ts, Prng.test.ts — module unit tests.
  • tests/charging-station/ocpp/1.6/OCPP16-CoherentMeterValues.test.ts — end-to-end StartTransaction → MeterValues → StopTransaction with meterStop cross-check.
  • tests/charging-station/ocpp/2.0/OCPP20ResponseService-CoherentSession.test.ts — OCPP 2.0 session wiring on TransactionEvent(Started).
  • tests/charging-station/ocpp/2.0/OCPP20ServiceUtils-PostTransactionDelay.test.ts — session destroy across the postTransactionDelay sleep.

Verification

  • pnpm lint — 0 errors, 0 warnings.
  • pnpm typecheck — 0 errors.
  • pnpm test — 2920 pass / 0 fail / 6 skipped.

Breaking changes

None. The coherent path is opt-in via coherentMeterValues: true in the station template. Existing templates are unaffected.

Follow-up

Deferred architectural, spec-compliance, and cleanup items tracked in #1936.

PR Checklist

  • Please add a description of your changes.
  • Please ensure title follows conventional commits.
  • We need your changes to come with unit tests in order to keep this project in quality and easy to maintain.
  • If your changes contain any new feature please be sure to document it.
  • Please add a link to the open issue or task that this pull request will resolve.

Does this PR introduce a breaking change?

  • Yes
  • No

Closes #40

Implements physics-coherent MeterValues (V->P->I->dE->SoC) gated by template
flag coherentMeterValues. Session lifecycle on ChargingStation with txId
snapshotted before resetConnectorStatus. Strategy gate after versioned
sampled-value dispatcher, before legacy random measurand generation.
Deterministic Mulberry32 PRNG with per-label stream splitting. New module
under src/charging-station/meter-values/. Golden invariants harness green.

Refs: #40
RED phase for M1..M4 findings from /tmp/issue-40/review-consolidated.md:
- M1: voltage PRNG must advance state across samples
- M2: deltaEnergyWh must be clamped to remaining battery capacity at 100% SoC
- M3-OCPP16: coherent session destroyed even if station stops during postTransactionDelay
- M3-OCPP20: same for OCPP 2.0 sibling path
- M4: stopEnergyWh assertion strengthened to remove self-reference tautology

Currently 4 tests fail (expected RED); M4 rewrite passes (strengthening only).
- M1: cache voltage PRNG on CoherentSession (was reconstructed each
  sample, producing a stalled seed sequence). PRNG state now advances
  across samples as documented.
- M2: clamp powerW to remaining battery capacity so a sample crossing
  100 % SoC cannot over-charge the register. Everything downstream
  (I, ΔE, register) is recomputed from clamped power, preserving
  INV-1 and INV-3.
- M3-OCPP16 / M3-OCPP20: destroy the coherent session BEFORE awaiting
  postTransactionDelay so an intervening station stop cannot leak the
  session. destroyCoherentSession is idempotent so the post-sleep
  path remains valid.

Regression tests (M1..M4): pass 23/23.
Full suite: pass 2908/0/6 skipped.
…y + tighten M4 register cross-check

Phase 6 verification findings addressed:
- N1 (gpt-5.5 HIGH, opus MED): capacity-clamp fallback risked INV-1 breach.
  Investigation: CURRENT_ROUNDING_SCALE=2 keeps V*I within <=0.1 W of
  reportedPowerW on realistic mains, well under INV-1 tolerance (+/-1 W).
  Simplified: floor reportedPowerW to maxPowerFromCapacityW after utility
  recompute (absorbs float drift without touching currentA).
- V1-M4 secondary (sonnet 'partial'): assertion now reads MV[last] (was
  MV[2], tautological). Independent Sigma(P*dt) primary check unchanged.
@hyperspace-insights

Copy link
Copy Markdown
Contributor

Summary

The following content is AI-generated and provides a summary of the pull request:


feat(meter-values): Physics-based Coherent MeterValues Generator

New Feature

✨ Implements a physics-aware, opt-in coherent MeterValues generation mode for the charging-station simulator (closes #40). When enabled (coherentMeterValues: true in the station template), all measurands in a sample are derived from a single physics chain (V → P → I → ΔE → SoC) rather than sampled independently. This makes the simulator suitable for testing CSMS data-quality validation, billing accuracy, and smart charging feedback loops.

Key invariants enforced by construction:

  • P = V × I × phases (AC) / P = V × I (DC) within ±1 W rounding tolerance
  • ΔE = P × Δt / 3_600_000 with E(t+1) ≥ E(t) (monotone)
  • SoC(t+1) ≥ SoC(t), ΔSoC = ΔE / batteryCapacityWh × 100
  • P ≤ min(EVSE_max, EV_curve(SoC), chargingProfileLimit)
  • SoC ≥ 100 → P = 0

Deterministic reproducibility is ensured via a seeded Mulberry32 PRNG with per-stream label-based splitting.

Changes

New Core Module (src/charging-station/meter-values/)

  • CoherentMeterValuesGenerator.ts: Physics-based sample generator. Handles voltage noise, current derived from clamped power, capacity clamping near SoC=100, SoC integration, and register ownership.
  • EvProfiles.ts: EV profile file loader (fail-soft), weight-based profile selection, and piecewise-linear charging curve interpolation.
  • prng.ts: Mulberry32 PRNG with FNV-1a stream-splitting (deriveSeed, hashLabel, mixSeed) for deterministic, non-shifting random streams.
  • types.ts: Shared types (CoherentSession, EvProfile, EvProfilesFile, ICoherentContext) and Zod validation schemas.
  • index.ts: Barrel export for the new module.

Template & Configuration

  • src/assets/ev-profiles-template.json: Example EV profiles file (city EV 40 kWh, long-range 77 kWh, DC fast 90 kWh).
  • src/types/ChargingStationTemplate.ts: Added coherentMeterValues?, evProfilesFile?, randomSeed? fields.
  • src/charging-station/TemplateSchema.ts: Zod schema additions for the three new template fields.

ChargingStation Integration

  • src/charging-station/ChargingStation.ts: Added coherentSessions Map, createCoherentSession(), destroyCoherentSession(), getCoherentSession() public methods, and initializeCoherentEvProfiles() private initializer.
  • src/charging-station/Helpers.ts: Added a safety comment noting that transactionId is deleted in resetConnectorStatus; callers must snapshot it before the call.
  • src/charging-station/index.ts: Exports new meter-values types.

OCPP Hooks (session lifecycle)

  • src/charging-station/ocpp/OCPPServiceUtils.ts: Strategy gate in buildMeterValue dispatches to buildCoherentMeterValue when coherent mode is active; otherwise the legacy path is byte-identical. Extracted createVersionedSampledValueDispatcher helper.
  • src/charging-station/ocpp/1.6/OCPP16ResponseService.ts: Calls createCoherentSession after accepted StartTransaction; calls destroyCoherentSession before postTransactionDelay sleep (prevents mid-sleep state leak, regression M3-OCPP16) and on all stop/error paths.
  • src/charging-station/ocpp/2.0/OCPP20ServiceUtils.ts: Same pre-sleep destroy pattern (regression M3-OCPP20).
  • src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts: Snapshots transactionId before resetConnectorStatus and destroys coherent session on error rollback.

Build

  • scripts/bundle.js: Copies ev-profiles*.json assets to the dist bundle.

Tests (new)

  • tests/charging-station/meter-values/CoherentMeterValuesGenerator.test.ts: Physics invariants (INV-1/2/3), SoC saturation, monotonicity, register ownership, Wh/kWh unit conversion, voltage PRNG non-stall (M1), SoC overshoot clamp (M2), determinism. 23 assertions.
  • tests/charging-station/meter-values/EvProfiles.test.ts: Curve interpolation (boundaries, midpoints, empty), profile selection, fail-soft file loading (missing, invalid JSON, schema violation, sort, SoC bounds swap).
  • tests/charging-station/meter-values/prng.test.ts: Mulberry32 determinism, range, stream independence, hash stability, non-shifting deriveSeed.
  • tests/charging-station/meter-values/StrategyDispatch.test.ts: Strategy gate — flag off (legacy), flag on without session (legacy fallback), flag on with injected session (coherent path).
  • tests/charging-station/ocpp/1.6/OCPP16-CoherentMeterValues.test.ts: End-to-end OCPP 1.6 transaction with coherent mode: invariants, meterStop == last register, determinism, rejected start creates no session, M3 regression (session destroyed before sleep).
  • tests/charging-station/ocpp/2.0/OCPP20ServiceUtils-PostTransactionDelay.test.ts: M3 regression on OCPP 2.0 path.
  • tests/charging-station/helpers/StationHelpers.ts: Mock charging station updated with coherentSessions store and createCoherentSession / destroyCoherentSession / getCoherentSession stubs.

  • 🔄 Regenerate and Update Summary
  • ✏️ Insert as PR Description (deletes this comment)
  • 🗑️ Delete comment
PR Bot Information

Version: 1.26.11

@hyperspace-insights hyperspace-insights Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The PR introduces a well-structured physics-based coherent MeterValues generator with good test coverage and careful session lifecycle management. However, there are several correctness issues to address: INV-3 energy/power consistency (using pre-rounding power for ΔE while reporting rounded power), the current value not being recomputed after the capacity-clamp is applied to reportedPowerW (breaking INV-1 in edge cases), the computeCoherentSample function reading currentType/numberOfPhases from live context rather than the immutable session (risking mid-transaction inconsistency), and the transaction-ID seed derivation using XOR which can collapse stream independence for certain string transaction IDs.

PR Bot Information

Version: 1.26.11

const roundedCurrent = roundTo(currentA, CURRENT_ROUNDING_SCALE)
const roundedPower = roundTo(reportedPowerW, POWER_ROUNDING_SCALE)

const deltaEnergyWh = (reportedPowerW * options.intervalMs) / MS_PER_HOUR

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Logic Error: computeCoherentSample uses reportedPowerW (pre-rounding) to compute deltaEnergyWh at line 281, but then emits roundedPower (post-rounding, re-clamped) as the reported power. This creates a subtle INV-3 inconsistency: the energy delta is derived from the unrounded power value, while the reported power is the rounded value. The two can diverge by more than the documented ±1 W tolerance on each sample, and over many samples the cumulative drift can be significant.

The energy delta should be computed from roundedPower (the value actually emitted to the CSMS) so INV-3 (P × Δt / 3.6e6 = ΔE) holds on the reported values:

Suggested change
const deltaEnergyWh = (reportedPowerW * options.intervalMs) / MS_PER_HOUR
const deltaEnergyWh = (roundedPower * options.intervalMs) / MS_PER_HOUR

Double-check suggestion before committing. Edit this comment for amendments.


Please provide feedback on the review comment by checking the appropriate box:

  • 🌟 Awesome comment, a human might have missed that.
  • ✅ Helpful comment
  • 🤷 Neutral
  • ❌ This comment is not helpful

Comment on lines +216 to +217
const currentType = context.stationInfo?.currentOutType ?? CurrentType.AC
const numberOfPhases = currentType === CurrentType.AC ? context.getNumberOfPhases() : 1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Logic Error: numberOfPhases and currentType are read from the live context at compute-time (lines 216-217), but the same values were already captured into session at creation time (session.numberOfPhases, session.currentType). If station config changes between session creation and sampling (e.g. hot-reload of template), the physics chain would use mismatched values — voltage/EVSE cap from the session's connectorId but phases/currentType from current context. The session-captured values should be used for consistency with the session profile.

Suggested change
const currentType = context.stationInfo?.currentOutType ?? CurrentType.AC
const numberOfPhases = currentType === CurrentType.AC ? context.getNumberOfPhases() : 1
const currentType = session.currentType
const numberOfPhases = session.numberOfPhases

Double-check suggestion before committing. Edit this comment for amendments.


Please provide feedback on the review comment by checking the appropriate box:

  • 🌟 Awesome comment, a human might have missed that.
  • ✅ Helpful comment
  • 🤷 Neutral
  • ❌ This comment is not helpful

*/
export const getEvProfilesFile = (stationInfo: ChargingStationInfo): string | undefined => {
return stationInfo.evProfilesFile != null
? join(dirname(fileURLToPath(import.meta.url)), 'assets', basename(stationInfo.evProfilesFile))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: getEvProfilesFile uses basename(stationInfo.evProfilesFile) which strips any directory component from the configured path before joining with the assets dir. This means a template entry like "evProfilesFile": "ev-profiles/custom.json" would silently resolve to assets/custom.json instead of failing with a clear error. The same comment-documented convention for getIdTagsFile should be verified to be intentional; if so, a doc comment explaining the security rationale (prevent path traversal) would clarify the intent.


Please provide feedback on the review comment by checking the appropriate box:

  • 🌟 Awesome comment, a human might have missed that.
  • ✅ Helpful comment
  • 🤷 Neutral
  • ❌ This comment is not helpful

Comment on lines +265 to +279
let reportedPowerW: number
if (currentType === CurrentType.AC) {
currentA = ACElectricUtils.amperagePerPhaseFromPower(numberOfPhases, powerW, roundedVoltage)
reportedPowerW = ACElectricUtils.powerTotal(numberOfPhases, roundedVoltage, currentA)
} else {
currentA = DCElectricUtils.amperage(powerW, roundedVoltage)
reportedPowerW = DCElectricUtils.power(roundedVoltage, currentA)
}
// Float-round can push reportedPowerW back over maxPowerFromCapacityW —
// floor it. At CURRENT_ROUNDING_SCALE=2, V·I still reconstructs
// reportedPowerW within ≤0.1 W on typical mains, so INV-1 tolerance (±1 W)
// is preserved.
reportedPowerW = Math.min(reportedPowerW, maxPowerFromCapacityW)
const roundedCurrent = roundTo(currentA, CURRENT_ROUNDING_SCALE)
const roundedPower = roundTo(reportedPowerW, POWER_ROUNDING_SCALE)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The maxPowerFromCapacityW clamp at line 277 is applied to reportedPowerW (which was derived by recomputing V·I from the rounded current), but roundedCurrent at line 278 is computed from the unclamped currentA computed at line 267/270. If rounding pushes reportedPowerW above maxPowerFromCapacityW, the clamp corrects reportedPowerW but roundedCurrent remains unclamped, breaking INV-1 (P = V·I·phases) for the clamped sample. The current should be recomputed from the clamped reportedPowerW.

Suggested change
let reportedPowerW: number
if (currentType === CurrentType.AC) {
currentA = ACElectricUtils.amperagePerPhaseFromPower(numberOfPhases, powerW, roundedVoltage)
reportedPowerW = ACElectricUtils.powerTotal(numberOfPhases, roundedVoltage, currentA)
} else {
currentA = DCElectricUtils.amperage(powerW, roundedVoltage)
reportedPowerW = DCElectricUtils.power(roundedVoltage, currentA)
}
// Float-round can push reportedPowerW back over maxPowerFromCapacityW —
// floor it. At CURRENT_ROUNDING_SCALE=2, V·I still reconstructs
// reportedPowerW within ≤0.1 W on typical mains, so INV-1 tolerance (±1 W)
// is preserved.
reportedPowerW = Math.min(reportedPowerW, maxPowerFromCapacityW)
const roundedCurrent = roundTo(currentA, CURRENT_ROUNDING_SCALE)
const roundedPower = roundTo(reportedPowerW, POWER_ROUNDING_SCALE)
let currentA: number
let reportedPowerW: number
if (currentType === CurrentType.AC) {
currentA = ACElectricUtils.amperagePerPhaseFromPower(numberOfPhases, powerW, roundedVoltage)
reportedPowerW = ACElectricUtils.powerTotal(numberOfPhases, roundedVoltage, currentA)
} else {
currentA = DCElectricUtils.amperage(powerW, roundedVoltage)
reportedPowerW = DCElectricUtils.power(roundedVoltage, currentA)
}
// Float-round can push reportedPowerW back over maxPowerFromCapacityW —
// floor it, then recompute current from clamped power so INV-1 holds.
if (reportedPowerW > maxPowerFromCapacityW) {
reportedPowerW = maxPowerFromCapacityW
if (currentType === CurrentType.AC) {
currentA = ACElectricUtils.amperagePerPhaseFromPower(numberOfPhases, reportedPowerW, roundedVoltage)
} else {
currentA = DCElectricUtils.amperage(reportedPowerW, roundedVoltage)
}
}
const roundedCurrent = roundTo(currentA, CURRENT_ROUNDING_SCALE)
const roundedPower = roundTo(reportedPowerW, POWER_ROUNDING_SCALE)

Double-check suggestion before committing. Edit this comment for amendments.


Please provide feedback on the review comment by checking the appropriate box:

  • 🌟 Awesome comment, a human might have missed that.
  • ✅ Helpful comment
  • 🤷 Neutral
  • ❌ This comment is not helpful

Comment on lines +78 to +79
const txSeed = deriveSeed(rootSeed, String(transactionId))
return mulberry32(deriveSeed(txSeed, label))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: deriveSeed uses symmetric XOR to mix the transaction ID into the root seed. When a string transactionId happens to equal a stream label (e.g. "VOLTAGE_NOISE"), txSeed == deriveSeed(rootSeed, label), causing deriveSeed(txSeed, label) == rootSeed (XOR is self-inverse), which collapses that stream's seed to the root — breaking stream independence. Use mixSeed (which uses multiplication) for the transaction-ID step since it already exists for this purpose.

Suggested change
const txSeed = deriveSeed(rootSeed, String(transactionId))
return mulberry32(deriveSeed(txSeed, label))
const txSeed = mulberry32(deriveSeed(rootSeed, String(transactionId)))()
return mulberry32(deriveSeed(txSeed >>> 0, label))

Double-check suggestion before committing. Edit this comment for amendments.


Please provide feedback on the review comment by checking the appropriate box:

  • 🌟 Awesome comment, a human might have missed that.
  • ✅ Helpful comment
  • 🤷 Neutral
  • ❌ This comment is not helpful

autofix-ci Bot and others added 3 commits July 1, 2026 17:05
Consolidated fixes for all 30 findings from the multi-agent code review of
PR #1935. Preserves the coherent MeterValues feature scope; adds OCPP 2.0
end-to-end wiring; hardens correctness and idiomaticity.

Blocking (2):
- B1 fix INV-1 breach in capacity-clamp branch: derive current as exact
  fraction (P / (V·phases)), round at emit, then derive emitted P from
  rounded V·I·phases. Prior integer-amp rounding could inflate V·I·phases
  above the capacity-clamped power by up to V·phases·0.5 W. New AC 3-phase
  and DC regression tests lock the invariant.
- B2 wire OCPP 2.0.1 support: add createCoherentSession call in
  OCPP20ResponseService.handleResponseTransactionEvent (Started case),
  mirroring OCPP 1.6. New 5-scenario integration test covering Accepted,
  implicit Accept, rejected idToken, force-override, and non-Started events.

High (4):
- H1 clear coherentSessions in ChargingStation.stop() finally; dispose
  runtime state per session before delete.
- H2 README: add three template-parameter rows (coherentMeterValues,
  evProfilesFile, randomSeed) and a new 'EV profile file format' subsection
  documenting the ev-profiles-template.json schema.
- H3 strip process residue: remove /tmp/issue-40/* references (3 files),
  'Phase 2 merged finding #N' and 'Fix Phase 4 M-N' markers (8+ locations);
  replace with technical rationale.
- H4 label INV-1/INV-2/INV-3 in the class-level JSDoc using the
  PR-body-canonical numbering (INV-1=V·I=P, INV-2=SoC monotone,
  INV-3=ΔE=P·Δt). Remove undefined INV-4 reference.

Medium (13):
- M1 DRY resolveRootSeed via hashLabel (byte-equivalent, test-locked).
- M2 move voltagePrng runtime state off CoherentSession to a module-scope
  WeakMap keyed on session identity (no cross-station coupling); add
  disposeCoherentSessionRuntime wired into destroyCoherentSession + stop().
- M3 collapse five identical rounding scales into a single ROUNDING_SCALE.
- M4 add explanatory comment on the boundary 'as MeterValue' cast
  (OCPP16/20 SampledValue.context enums structurally diverge).
- M5 correct Prng.ts JSDoc: 'SplitMix32-derived' -> 'Mulberry32 + FNV-1a'.
- M6 remove 'byte-identical' over-claims; use 'reproducible' / 'identical'.
- M7 defensive early-return zero-sample when intervalMs <= 0 to prevent
  NaN contamination when SoC has already saturated.
- M8 trim meter-values/index.ts barrel from 22 to 11 externally-consumed
  symbols.
- M9 mark 7 immutable CoherentSession fields readonly.
- M10 add ChargingStation.injectCoherentSession() public method and use it
  in 3 test sites, replacing 'station as unknown as { coherentSessions }'
  private-field injection.
- M11 fix 'transaction id' -> 'transactionId' in Prng.ts comment.
- M12 replace global Date.now monkey-patch with per-iteration
  mock.method(Date, 'now', ...) from node:test.
- M13 remove dead chargingProfileLimitW parameter
  (getConnectorMaximumAvailablePower already folds in charging profiles).

Low / Nit:
- C4 clear-on-stop (covered under H1).
- C5 XOR-commutativity in deriveSeed: deferred with explanatory comment
  (birthday bound ~2^16 well beyond simulator scale; a non-commutative mix
  would desync existing golden tests).
- D-7 rename prng.ts -> Prng.ts (and prng.test.ts -> Prng.test.ts) to
  match repo PascalCase filename convention.
- D-8 move getEvProfilesFile from EvProfiles.ts to Helpers.ts next to
  getIdTagsFile.
- D-9 fix resolveTemplates JSDoc: remove false 'mirrors EVSE lookup' claim.
- D-11 CoherentMeterValuesDefaults now exposes all tunable constants.
- D-18 use AvailabilityType.Operative enum instead of string cast.
- I1 auto-resolved by B1 (ROUNDING_SCALE=2 now semantically meaningful
  for current).
- T2 use getErrorMessage() (repo convention) instead of (error as Error).
- S2 simplify templatesFor test helper — remove 'as unknown as { unit }'
  casts.
- S5 consolidate duplicate BuildVersionedSampledValueFn type into the
  canonical BuildVersionedSampledValue exported from meter-values.

Verification:
- pnpm lint      0 errors, 0 warnings
- pnpm typecheck 0 errors
- pnpm test      2918 pass / 0 fail / 6 skipped
- New regression tests locking B1 (INV-1 clamp AC3+DC), M7 (NaN guard),
  M2 (session hygiene + WeakMap dispose), M1 (hashLabel equivalence),
  B2 (OCPP 2.0 session wiring, 5 scenarios).

Closes #40
Consolidated fixes for all Medium+Low+Nit findings from the second
multi-agent review round of PR #1935. Two Blocking findings (S1 OCPP 1.6
signed-meter-values wrapper, S2 begin MeterValue routing) — S2 implemented,
S1 deferred to a follow-up (post-hoc signing in startUpdatedMeterValues
already covers the OCPP 1.6 periodic path; the remaining gap is only
TriggerMessage/broadcast callsites, best served by a dedicated PR with a
proper signing-key test fixture).

## Blocking (1 of 2 addressed; other deferred)

- **S2 (implemented)** — `Transaction.Started` / `Transaction.Begin`
  MeterValue is now generated by the coherent path. Made
  `ChargingStation.createCoherentSession` idempotent so early
  request-builder creation is safe; reordered OCPP 2.0 flows
  (`OCPP20ServiceUtils.startTransactionOnConnector`,
  `OCPP20IncomingRequestService` RequestStartTransaction handler) to
  create the session BEFORE `buildTransactionStartedMeterValues`;
  reordered OCPP 1.6 flow (`OCPP16ResponseService.handleResponseStartTransaction`)
  and added a coherent-mode branch to
  `OCPP16ServiceUtils.buildTransactionBeginMeterValue` that routes
  through `buildMeterValue` when a session is live.

- **S1 (deferred)** — OCPP 1.6 signed-meter-values wrapper for the
  coherent path. Rationale: `startUpdatedMeterValues` in
  `OCPP16ServiceUtils` applies post-hoc signing after `buildMeterValue`
  returns, so the OCPP 1.6 periodic coherent path IS signed today. The
  actual gap is only the TriggerMessage / worker-broadcast callsites
  where signing is skipped. Fixing it cleanly requires a signing-key
  test fixture that is best set up in a dedicated PR.

## Medium (8)

- **M-01/M-02/M-03/M-07 combined defensive-guard block**
  (`computeCoherentSample`): early-return zero-sample when `intervalMs ≤ 0`
  (existing), `batteryCapacityWh ≤ 0` or non-finite (M-01), or
  `voltageOut ≤ 0` or non-finite (M-02). `rampUpDurationMs` guard now
  requires `> 0 && Number.isFinite(...)` (M-03). Prevents NaN poisoning
  and INV-1/INV-3 incoherence across the four sources.
- **M-04** — Reverted the proposed Zod refinement for sorted `chargingCurve`
  after design analysis showed `loadEvProfilesFile`'s in-place sort is
  the authoritative defense and the refinement would break the loader's
  "accepts unsorted, sorts in place" contract. Documented rationale in
  `EvProfileSchema` JSDoc.
- **M-06** — Renamed `ChargingStation.injectCoherentSession` →
  `__injectCoherentSession` (dunder-prefix test-seam convention);
  tightened mock parameter type from `unknown` to `CoherentSession`;
  updated the 3 test call sites.
- **M-07** — Fixed the determinism claim in README.md and PR body:
  `interval` is a physics parameter, not a PRNG seed input. New prose
  makes this explicit.
- **M-08** — Coherent path now respects OCPP 2.0.1 `SampledDataCtrlr.TxUpdatedMeasurands`
  / `TxEndedMeasurands` / `TxStartedMeasurands` and OCPP 1.6
  `MeterValuesSampledData`. Added `resolveEnabledMeasurands` helper in
  `OCPPServiceUtils.ts` and threaded a `ReadonlySet<MeterValueMeasurand>`
  allow-list into `buildCoherentMeterValue`. Governs J02.FR.11 / E02.FR.09
  / E06.FR.11.

## Deferred to follow-up issue (3)

- **M-05** — Extract `CoherentMeterValuesManager.getInstance(chargingStation)`.
  Sibling per-station concerns (AutomaticTransactionGenerator, IdTagsCache,
  SharedLRUCache) use the multiton pattern; the coherent module currently
  keeps state on `ChargingStation`. Not a correctness issue; architectural
  refactor best done standalone.
- **N-03** — Blocked on M-05 (semantic circularity in `ICoherentContext.getCoherentSession`
  cleaned up naturally when the manager owns the session store).
- **N-04** — `Prng.ts` filename kept as-is; renaming to `PRNG.ts` on a
  case-insensitive macOS FS would require a second two-step rename with
  no functional benefit.

## Low/Nit (10)

- **N-01** — Dropped `disposeCoherentSessionRuntime` from the
  `meter-values/index.ts` barrel; direct sub-module import in
  `ChargingStation.ts` (barrel exposes only externally-consumed symbols).
- **N-02** — Rephrased the `Fix B1:` process-comment in
  `CoherentMeterValuesGenerator.ts` to invariant-only technical rationale
  (H3 miss from the previous round).
- **N-05** — Renamed voltage locals in `computeCoherentSample`:
  `voltageNominal`/`voltageV`/`roundedVoltage` →
  `nominalV`/`sampledV`/`roundedV` (fewer names for the same quantity).
- **N-06** — Added defensive `destroyCoherentSession` in the OCPP 2.0
  `handleResponseTransactionEvent` `Ended` branch when connectorId is
  unknown (symmetric with OCPP 1.6 defensive destroy in
  `handleResponseStopTransaction`).
- **N-07** — Tightened `deriveSeed` JSDoc with quantified birthday
  bound: expected collisions ≈ N²/2^33; at simulator scale (≤ 5×10⁴
  pairs) ≈ 0.3 — negligible.
- **N-08** — Tightened AC1/AC3/DC test tolerances from ≤ 1/3/1 W to
  ≤ 0.01 W (post-Option D tight bound is 0.005 W).
- **N-09** — Uniform `getErrorMessage(error)` in EvProfiles.ts
  ZodError branch (was direct `error.message`, inconsistent with the
  else branch).
- **N-10** — Removed dead exports `CoherentMeterValuesDefaults` and
  `mixSeed` (grep-verified zero external usages).
- **N-11 F-04** — `@file` tag "MeterValue generator" (singular) →
  "MeterValues generator" (plural OCPP term).
- **N-11 F-07** — README schema example id aligned with the actual
  `ev-profiles-template.json` (`compact-ev-40kwh` → `city-ev-40kWh`).
- **N-11 F-08** — README documents the `initialSocPercentMin` ≤
  `initialSocPercentMax` swap-and-warn behavior.

## Verification

- `pnpm typecheck` — 0 errors
- `pnpm lint` — 0 errors, 0 warnings
- `pnpm test` — 2878 pass / 0 fail / 6 skipped

Refs #40
jerome-benoit and others added 5 commits July 2, 2026 00:15
Consolidated fixes for all findings from the third multi-agent review round.
No blocking findings remain; two content-lane blocks (barrel count, README
example) plus 4 cross-validated HIGH nits addressed.

## HIGH (4)

- **H1** — Correct false JSDoc claim at `ChargingStation.ts:319`. The
  `__injectCoherentSession` docstring stated the `__` prefix was enforced
  by a `no-restricted-syntax` ESLint rule; no such rule exists. Reworded
  to "convention only — not currently enforced by a lint rule".
- **H2** — Trim 4 dead type re-exports from `src/charging-station/index.ts`
  (`ChargingCurvePoint`, `EvProfile`, `EvProfilesFile`, `ICoherentContext`).
  Grep-verified zero external consumers. `CoherentSession` kept (used by
  4 test files). Barrel is now honest about its "only externally-consumed
  symbols" contract.
- **H3** — Sync README `city-ev-40kWh` schema example to
  `src/assets/ev-profiles-template.json` exactly: `maxPowerW` 7400→11000,
  `weight` 1.0→3, `initialSocPercentMin` 10→15, `initialSocPercentMax`
  80→55, 3-point curve → 4-point taper. First-time readers now see the
  same values in both docs.
- **H4** — Validate CSV entries in `resolveEnabledMeasurands`
  (`OCPPServiceUtils.ts`). `Object.values(MeterValueMeasurand)`-membership
  guard drops unknown entries and logs a per-station-debounced warning.
  Prevents silent typo tolerance (e.g. `Voltege` in config CSV) while
  accepting every OCPP-defined measurand (unlike the narrower
  `isMeasurandSupported` allowlist).

## MED (3)

- **M1** — Thread `TxUpdatedMeasurands` into `buildMeterValue` at both
  OCPP 2.0 sites that previously bypassed it: `OCPP20ServiceUtils.ts`
  `startUpdatedMeterValues` (periodic path) and
  `OCPP20IncomingRequestService.ts` `TriggerMessage(MeterValues)` handler.
  Closes the J02.FR.11 spec gap where CSMS-configured measurand filters
  were ignored on the periodic Updated path.
- **M2** — Debug-only sortedness assertion in `interpolateChargingCurve`
  (`EvProfiles.ts`). Production path (`loadEvProfilesFile`) sorts in
  place; the assertion catches misuse from the `__inject*` test seam
  (unsorted curves silently returned the tail power-fraction — a hidden
  test footgun). `process.env.NODE_ENV !== 'production'` gate keeps
  runtime cost at zero in production.
- **M3** — Session-leak safety net at 2 OCPP 2.0 request-time create sites:
  `OCPP20ServiceUtils.ts` `startTransactionOnConnector` wraps
  `sendTransactionEvent` in `try/catch`; `OCPP20IncomingRequestService.ts`
  `RequestStartTransaction` handler augments the existing `.catch` chain.
  Both call `destroyCoherentSession(txId)` before re-throwing/logging,
  symmetric with the OCPP 1.6 `resetConnectorOnStartTransactionError`
  pattern. Prevents session Map growth on WebSocket-send rejection.

## Content + Low/Nit

- **P-01** — Extend the defensive guard block in `computeCoherentSample`
  with `!Number.isFinite(options.nowMs)`. Matches the pattern for the
  other 4 guarded inputs. Not reachable in production (`Date.now()`
  always finite) but closes the test-injection hole.
- **P-02** — Document `rampUpDurationMs = Number.EPSILON` equivalence
  with 0 (both collapse to immediate full-power). Comment only.
- **D-05** — Add `logger.warn` to the OCPP 2.0
  `handleResponseTransactionEvent` Ended-with-unknown-connector branch,
  mirroring the OCPP 1.6 diagnostic parity at
  `OCPP16ResponseService.ts:534-536`.
- **N-03** — Expand `ComputeSampleOptions.voltageNoise` inline comment
  into a full JSDoc with `@remarks` and `@defaultValue`.
- **N-04** — Document the WHY for `createCoherentSession` idempotency
  (OCPP 2.0.x has two call sites per transaction from S2 fix).
- **N-05** — Document the `<quantity><Unit>` naming convention on
  `CoherentSample` (all fields follow `currentA`/`powerW`/`voltageV`).

## Deferred with rationale (documented decisions)

- **A6** — `buildTransactionBeginMeterValue` reads
  `connectorStatus.transactionId` internally rather than accepting an
  explicit param. Single call site, safe mutation ordering already
  documented; adding a param would add noise for zero safety gain.
- **D-03** — `buildTransactionBeginMeterValue` dual-responsibility
  (coherent short-circuit + legacy path) tracked in follow-up issue
  #1936 as new M-06 item.

## Verification

- `pnpm typecheck` — 0 errors
- `pnpm lint` — 0 errors, 0 warnings
- `pnpm test` — 2899 pass / 0 fail / 6 skipped

Refs #40
Consolidated fixes for all findings from the fourth multi-agent review round
(5 parallel reviewers: Oracle elegance/TS · Oracle math/physics · general
OCPP-spec-via-qmd · explore harmonization · general content/terminology).
Design phase cross-validated 5 architecturally-loaded decisions via Oracle.

## HIGH (6)

- **H1 [R4C]** — Thread `OCPP20ReadingContextEnumType.TRIGGER` into the
  `TriggerMessage(MeterValues)` handler at `OCPP20IncomingRequestService.ts:611`.
  Emitted `SampledValue.context` now correctly labels values taken in
  response to `TriggerMessage` per OCPP 2.0.1 §3.66 ReadingContextEnumType;
  previously defaulted to `Sample.Periodic`.

- **H2 [R4C]** — Presence-aware `resolveEnabledMeasurands` semantics in
  `OCPPServiceUtils.ts`. Discriminates key-absent (defaults to
  `{Energy.Active.Import.Register}` for ergonomic parity) from key-present
  (honors the CSV verbatim, including empty ⇒ empty allow-list per
  OCPP 2.0.1 J02.FR.11). Removes the unconditional force-add at line 1115.

- **H3 [R4C]** — Per-phase measurand emission in `buildCoherentMeterValue`.
  Restructured to iterate all templates per measurand (was: first-matching
  only) with phase-aware value resolution:
  `Voltage L-N ⇒ V`; `L-L ⇒ √phases × V`.
  `Power L-N ⇒ P/phases`; L-L unsupported (log-and-skip).
  `Current line ⇒ sample.currentA` (balanced 3-φ Y).
  `SoC / Energy.Active.Import.Register` phase-qualified templates rejected.
  Emit order: `SoC → V → P → I → Energy` across measurands; within a
  measurand: `no-phase → L1/L1-N → L2/L2-N → L3/L3-N → L1-L2 → L2-L3 →
  L3-L1 → N → other`. Closes legacy-parity regression for 3-phase stations
  with phase-qualified templates.

- **H4 [R4D]** — `throw new BaseError(...)` in `EvProfiles.ts`
  `interpolateChargingCurve` (was: bare `Error`), aligning with AGENTS.md
  TypeScript conventions on typed errors.

- **H5 [R4D]** — `assert.ok(cs != null, 'Expected connector 1 to exist')`
  in `OCPP20ServiceUtils-PostTransactionDelay.test.ts` (was: `throw new
  Error`), matching the pattern used elsewhere in the test suite.

- **H6 [R4E]** — README §EV profile file format now documents the
  connector-level-only `MeterValues` template resolution scope under
  coherent mode (EVSE-level inheritance not applied; tracked as follow-up).

## MED (12)

- **M1 [R4A]** — `computeCoherentSample` now takes the resolved
  `session` as a parameter (was: fetched twice via `context.getCoherentSession`).
  Removes 2 unreachable defensive branches (\~25 LOC) and tightens the
  contract into a pure physics function.

- **M2 [R4C]** — OCPP 1.6 `buildTransactionBeginMeterValue` threads
  vendor param `StartTxnSampledData` when configured, falling back to
  `MeterValuesSampledData` when absent — per the OCPP 1.6 Signed Meter
  Values whitepaper.

- **M3 [R4D]** — Move `MS_PER_HOUR` and `UNIT_DIVIDER_KILO` to
  `Constants` class (`src/utils/Constants.ts`). Was duplicated as
  file-scope constants in `OCPPServiceUtils.ts` and
  `CoherentMeterValuesGenerator.ts`. All references now use
  `Constants.MS_PER_HOUR` / `Constants.UNIT_DIVIDER_KILO`.

- **M4 [R4D]** — Move `VOLTAGE_NOISE_PERCENT` to
  `Constants.DEFAULT_COHERENT_VOLTAGE_NOISE_PERCENT`. Tunables belong in
  the canonical defaults map per AGENTS.md.

- **M5 [R4D]** — Move `DEFAULT_RAMP_UP_DURATION_MS` to
  `Constants.DEFAULT_COHERENT_RAMP_UP_DURATION_MS`. Same rationale.

- **M6 [R4D]** — Merge same-specifier `import type` statements in
  `types.ts` and `EvProfiles.ts` (was: 2 statements each for the same
  module). Aligns with the project's `import/no-duplicates` /
  `verbatimModuleSyntax` convention.

- **M8/M11 [R4D]** — `describe('Prng', ...)` and
  `describe('StrategyDispatch', ...)` renames to match the
  module-name-only convention in `tests/TEST_STYLE_GUIDE.md`.

- **M9 [R4E]** — README precision: "emitted measurand list" no longer
  claims `ΔE` is emitted (it is an internal per-sample intermediate);
  INV-1 spelled out per current-type (`P = V·I·phases` for AC, `P = V·I`
  for DC).

- **M10 [R4E]** — Rewrite forward-looking comment at
  `OCPP16ServiceUtils.ts:152-158` to describe the current-branch semantics
  (`StartTxnSampledData` override with fallback to
  `MeterValuesSampledData`) instead of the deferred S1 signing wiring.

## LOW/NIT batch

- **R4A-LOW-01** — `advanceEnergyRegister` rewritten with
  `Math.max(0, ... ?? 0)` — one-expression clamp-and-init (was: two-step
  nullish-then-negative guard, ~14 LOC → 6 LOC).

- **R4A-LOW-02** — 3 near-identical `CoherentSample` zero-literal returns
  in `computeCoherentSample` collapsed to a single `buildZeroSample`
  helper. Two of the three defensive branches also removed by M1.

- **R4A-NIT-01** — `findTemplate` replaced by `groupTemplatesByMeasurand`
  (needed for H3 per-phase iteration anyway); legacy `for..of` scan is now
  gone.

- **R4B-LOW-01/02** — INV-1/INV-3 docstring precision: emit-time rounding
  bound stated as "`ROUNDING_SCALE` half-width (\~0.005 W scalar)"; INV-3
  divergence bound quantified (\~0.12 Wh over 24 h at 1 Hz).

- **R4B-LOW-05** — `EvProfileSchema` JSDoc documents monotone-non-increasing
  `powerFraction` as a caller responsibility (not schema-enforced;
  real curves are flat through CC before tapering).

- **R4B-NIT-06** — AC `numberOfPhases <= 0` now triggers the zero-sample
  defensive branch (was: silently produced `P = 0` via
  `divisor = V × 0 = 0`).

- **R4B-NIT-07** — `interpolateChargingCurve` JSDoc documents the
  closed-closed interval semantic (left segment wins at interior nodes).

- **R4D-LOW-06/07/08** — `StationHelpers.ts` mock: `coherentSessions`,
  `createCoherentSession`, `getCoherentSession` return types tightened
  from `unknown` to `CoherentSession | undefined` (and
  `Map<number | string, CoherentSession>`).

- **R4D-NIT-03/04/05/06** — `meter-values/index.ts` barrel comment now
  explicitly enumerates the intentionally-omitted internal exports
  (`computeCoherentSample`, `advanceEnergyRegister`, `createStreamPrng`,
  `disposeCoherentSessionRuntime`, option-bag types, EvProfiles helpers,
  Prng primitives, Zod schemas).

- **R4E-LOW-05** — `Prng.ts` header no longer claims a specific LOC
  bound ("kept intentionally small").

- **R4E-LOW-06** — Expanded JSDoc on `ComputeSampleOptions` and
  `CreateSessionOptions` interfaces (per-field semantics, units, defaults).

## Deferred to follow-up #1936 (documented rationale)

- **M7 [R4D]** — `StationHelpers.ts` (954 LOC) modular split. Structural
  refactor with 30+ test file consumers; TODO comment added at the top of
  the file linking to #1936. Detailed split sketch documented in the
  design phase.

- **H6 code-side** — Extend `ICoherentContext` with `getEvseStatus` to
  restore EVSE-level template inheritance. Round-4 lands docs-only.

- **R4B-LOW-03/04** — Physics model design notes (`cos φ = 1` assumption,
  linear-vs-S-curve ramp shape). Not blockers.

## Non-findings (verified compliant)

- OCPP 2.0.1 `TxUpdatedInterval` correctly wired for periodic scheduling.
- `Transaction.Begin` / `Transaction.End` enum literals correct.
- Coherent path does not spoof `signedMeterValue`; signing gate intact.
- Coherent path does not bleed into `AlignedDataCtrlr` /
  `ClockAlignedDataInterval` handling.
- Existing hyphenated test file names (`OCPP16-CoherentMeterValues.test.ts`
  etc.) match the `ChargingStation-Configuration.test.ts` sub-domain
  precedent — not a divergence.

## Verification

- `pnpm typecheck` — 0 errors
- `pnpm lint` — 0 errors, 0 warnings
- `pnpm test` — 2920 pass / 0 fail / 6 skipped

Refs #40
…h (issue #40)

## OCPP 2.0.1 spec-strict emit shape

- `TransactionEvent(Updated)` no longer serializes an empty `meterValue`
  wrapper when `TxUpdatedMeasurands` resolves to an empty allow-list.
  Periodic `startUpdatedMeterValues` short-circuits; TriggerMessage
  (MeterValues) active-tx branch sends the event without the `meterValue`
  field. Matches OCPP 2.0.1 J02.FR.11 ("no meter values are sent") and
  fixes the JSON-schema `minItems: 1` violation on empty sampledValue.

## OCPP 1.6 whitepaper composition (StartTxnSampledData × beginEndMeterValues)

- `OCPP16ResponseService` gates the extra `MeterValues.req` sends at
  both the start (`beginEndMeterValues`) and end (`outOfOrderEndMeterValues`)
  branches on `isNotEmptyArray(sampledValue)`. Composes cleanly with the
  Signed Meter Values whitepaper §3.3.4 rule (`StartTxnSampledData`
  present + non-empty) and the legacy `MeterValuesSampledData` fallback:
  both empty ⇒ no send, avoiding a schema-suspect empty sampledValue
  wrapper.

## Coherent path — per-phase physics polish

- **Per-phase Energy.Active.Import.Register**: L-N templates now emit
  `register / phases` (balanced 3-φ Y contribution per phase); L-L and
  `N` phases skipped with a warn. OCPP 2.0.1
  `SampledDataCtrlr.RegisterValuesWithoutPhases` config-driven suppression
  not consulted (documented follow-up).
- **Neutral phase**: new `Neutral` phase-family classifier distinguishes
  bare `N` from `LineToNeutral` (`L1`/`L1-N`/etc.). `N`-qualified
  Voltage and Current templates emit 0 (balanced 3-φ Y: I_N = 0 by
  phasor sum, V_N = 0 by reference-node definition).
- **L-L voltage on 1-phase**: log-and-skip. Previously the `√1 × V`
  degeneracy silently emitted nominal V under an L-L label.
- **L-N power on phases≤0**: log-and-skip (previously fell through to
  aggregate power under a per-phase label — unreachable in practice due
  to the outer defensive guard but semantically inconsistent).
- **Register clamp sync**: `preRegisterWh` in `computeCoherentSample`
  applies `Math.max(0, ... ?? 0)` matching `advanceEnergyRegister`;
  reported `sample.energyRegisterWh` and post-advance persisted state
  now agree even under corrupted negative register state.

## Elegance + TS state-of-the-art

- Extract `resolveUnitDivider(measurand, unit)` +
  `KILO_UNIT_BY_MEASURAND` at module scope, removing the inline
  `(A && B) || (C && D)` boolean at the emit site.
- `PHASE_RANK` converted from `ReadonlyMap` to object literal with
  `as const satisfies Record<MeterValuePhase, number>` for
  compile-time exhaustiveness; `phaseRank`'s runtime fallback dropped.
- `resolvePhasedValue` returns exact fractions (no internal rounding);
  rounding happens once at the emit site after unit-divider scaling.
  Removes double-rounding asymmetry between aggregate and per-phase paths.
- `groupTemplatesByMeasurand` uses `Map.groupBy` (Node 22+) in place of
  the hand-rolled loop; per-bucket phase sort preserved.
- Merged split `import type { ConnectorStatus }` with sibling type-import
  from the same specifier.

## Documentation

- README §Phase-qualified measurands: "nominal V" → "sampled V"
  everywhere (the coherent path emits `sample.voltageV`, i.e.
  post-noise rounded voltage — not the station's nominal); documents
  `N` phase behavior and per-phase Energy register emission.
- `resolvePhasedValue` JSDoc updated to match implementation.
- `ComputeSampleOptions` and `CreateSessionOptions` JSDoc converted
  from bullet-list style to inline per-field JSDoc (matching
  `CoherentSession` and `ICoherentContext` in `types.ts`).
- `types.ts` file header rewritten to state the current interface
  contract instead of deferred tooling-contingency rationale.
- `Constants` docstrings for coherent tunables /
  `MS_PER_HOUR` / `UNIT_DIVIDER_KILO` shortened to one-liners matching
  the concision of sibling entries.
- `CoherentMeterValuesGenerator.ts` header carries a follow-up TODO
  for the 250-LOC ceiling exceedance (split scope documented).
- Explanatory comment on the module-scope `warnedInvalidMeasurands`
  WeakMap in `OCPPServiceUtils.ts` distinguishing its GC-keyed intent
  from a true singleton.
- Internal-only comment on `VersionedSampledValueDispatch` interface.

## Tests

- New per-phase emission integration test: 3-phase AC session with
  L-N/L-L voltage, aggregate + per-phase Power, L1/N Current, and L-N
  Energy templates; asserts V_L1_N=230, V_L1_L2≈√3·230, Σ per-phase P
  ≈ aggregate P, I_N=0, L-L voltage skipped on 1-phase, L-N energy
  register emitted as `register / phases`.
- `describe('OCPP 1.6 coherent MeterValues integration')` renamed to
  `describe('OCPP16CoherentMeterValues')` (module-name convention).
- OCPP 1.6 integration test replaces its local `MS_PER_HOUR = 3_600_000`
  literal with a `Constants.MS_PER_HOUR` alias (canonical source).
- Removed leaked internal-review annotations from 9 describe/it labels
  (`(regression: ...)` suffixes) — these violated documentation
  timeliness by embedding non-behavioral session metadata into the
  behavioral test tree.

## Verification

- `pnpm typecheck` — 0 errors
- `pnpm lint` — 0 errors, 0 warnings
- `pnpm test` — 2923 pass / 0 fail / 6 skipped

Refs #40
…olish (issue #40)

## OCPP 2.0.1 spec-strict emit shape

- Periodic `TransactionEvent(Updated)` no longer drops the whole message
  when `TxUpdatedMeasurands` resolves empty. Sends the event with the
  `meterValue` field omitted, matching the R5 fix already in place on the
  `TriggerMessage(MeterValues)` active-tx branch. Preserves the periodic
  heartbeat cadence at the OCPP level while honoring J02.FR.11
  ("no meter values are sent") — the message is still delivered, only
  the sampled-value payload is elided.

## Coherent path — phase-degeneracy symmetry

- `Energy.Active.Import.Register` L-N templates on `phases <= 0` now
  log-and-skip (return `undefined`), matching `Power.Active.Import`'s
  behavior for the same misconfiguration. Previously the register branch
  fell through to emit the aggregate under a per-phase template label.

## Elegance + TS state-of-the-art

- `resolvePhasedValue` signature narrowed from
  `(measurand, template, sample, phases, connectorStatus)` to
  `(measurand, phase, sample, phases, connectorStatus)`. The function
  only reads `template.phase`; taking the whole template widened the
  coupling. Caller passes `template.phase` at the loop head.
- `resolveUnitDivider` gains a JSDoc block (sole helper in the file that
  previously lacked one) and reorders the guard to
  `unit != null && KILO_UNIT_BY_MEASURAND.get(measurand) === unit` for
  intent-first reading ("only kilo-divide when a unit is present").
- `LEGACY_EMIT_ORDER` renamed to `MEASURAND_EMIT_ORDER`. The constant
  is the canonical emission order of the coherent path, not a
  compatibility shim; the JSDoc note preserves the "mirrors the legacy
  path" provenance.
- `buildZeroSample` now owns all rounding for the zero-sample path.
  Callers pass raw `socPercent` / `voltageV`; the helper applies
  `roundTo` internally. Rounding-responsibility is unified in one place.

## Documentation

- INV-1 docstring documents both the aggregate residual (≤ 0.005 W) and
  the per-phase L-N residual (≤ 0.01 W = 2 × `ROUNDING_SCALE` half-width),
  quantifying the two-step rounding bound (aggregate emit + per-phase
  division).
- `resolvePhasedValue` JSDoc documents the per-phase Energy register
  conservation bound: Σ across all L-N templates equals the aggregate
  register within emit-unit rounding granularity (Wh: ≤ phases · 0.005 Wh;
  kWh: ≤ phases · 5 Wh).
- `VersionedSampledValueDispatch.signingState` JSDoc: unresolvable
  `{@link buildVersionedSampledValue}` replaced with plain prose
  (the reference is a local closure, not an exported symbol).
- `resolveEnabledMeasurands` `@param measurandsKey` prose clarified:
  `undefined` (or omitted) defaults to `StandardParametersKey.`
  `MeterValuesSampledData` for OCPP 1.6; returns `undefined` (no filter)
  for all other versions.
- `meter-values/index.ts` barrel comment simplified. Dropped the
  enumerated intentionally-omitted list (drifts with internals); kept
  the intent sentence only.
- `ChargingStation.createCoherentSession` JSDoc: dropped the
  request-builder/response-handler call-site narrative; kept the API
  contract ("idempotent; returns `undefined` when coherent mode is
  disabled or no valid EV profile file is loaded").

## README

- `§Template resolution scope`: dropped the internal
  `getSampledValueTemplate` helper name and the backlog-tracking sentence.
  Documents current behavior only (connector-level templates, no
  EVSE-level inheritance under coherent mode).
- `§Phase-qualified measurands`: dropped the "tracked as a follow-up"
  tail on the `RegisterValuesWithoutPhases` note.
- Charging station template configuration table: `three phased` →
  `three-phase`, `line to line voltage` → `line-to-line voltage`
  (hyphenation consistent with the rest of the docs).

## Tests

- Added mandatory `afterEach(() => standardCleanup())` block to
  `CoherentMeterValuesGenerator.test.ts`, `EvProfiles.test.ts`, and
  `Prng.test.ts` per `tests/TEST_STYLE_GUIDE.md` §Mandatory Cleanup.
- `describe('OCPP20ServiceUtils — PostTransactionDelay')` renamed to
  `describe('OCPP20ServiceUtilsPostTransactionDelay')` — module-name
  concatenated form matching the existing coherent-test naming.
- `describe('B2 - OCPP 2.0.1 TransactionEvent Started → coherent session
  wiring')` renamed to `describe('OCPP20ResponseServiceCoherentSession')`
  — module-name-only form; the descriptive/spec-prefix suffix was
  redundant with the inner `it` labels.

## Verification

- `pnpm typecheck` — 0 errors
- `pnpm lint` — 0 errors, 0 warnings
- `pnpm test` — 2920 pass / 0 fail / 6 skipped

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

[FEATURE] Physics-based coherent MeterValues generation with EV simulation model

1 participant