]> Piment Noir Git Repositories - e-mobility-charging-stations-simulator.git/commit
refactor: convention alignment round 2 — canonical defaults, helpers, dead exports...
authorJérôme Benoit <jerome.benoit@piment-noir.org>
Mon, 6 Jul 2026 11:52:59 +0000 (13:52 +0200)
committerGitHub <noreply@github.com>
Mon, 6 Jul 2026 11:52:59 +0000 (13:52 +0200)
commite0a50c58f62cf7fb0e7d9b41366215e175af616f
tree9d5b0545487cbb28820cd80408c26c2e60494e58
parentf7640876ebe2863aeaaaa1fbfdc22549d6ed485a
refactor: convention alignment round 2 — canonical defaults, helpers, dead exports, tests (#1956)

* refactor: convention alignment round 2 — canonical defaults, helpers, dead exports, tests

Follow-up to #1950/#1952/#1953/#1954. Post-audit factorization; behavior-preserving.

Canonical defaults promoted to Constants.ts:
- UNIT_DIVIDER_CENTI/DECI (100/10); adopt UNIT_DIVIDER_KILO at 3 kW/kWh sites
- DEFAULT_RECONNECT_JITTER_PERCENT, SOC_MAXIMUM_PERCENT
- DEFAULT_AUTH_CACHE_MAX_ABSOLUTE_LIFETIME_MS
- DEFAULT_REMOTE_AUTH_CACHE_TTL_SECONDS
- MS_PER_DAY, SECONDS_PER_DAY (align with existing MS_PER_HOUR)

Canonical defaults promoted to OCPP20Constants.ts (OCPP-variable-read fallbacks):
- DEFAULT_RETRY_BACKOFF_{WAIT_MINIMUM,RANDOM_RANGE}_SECONDS
- DEFAULT_RETRY_BACKOFF_REPEAT_TIMES
- DEFAULT_CERT_SIGNING_WAIT_MINIMUM_SECONDS

Module-scope constants:
- WS_DEFLATE_{CONCURRENCY_LIMIT,SERVER_MAX_WINDOW_BITS,ZLIB_*} in UIWebSocketServer
- STATISTICS_PERCENTILE in PerformanceStatistics
- AUTH_CACHE_LIFETIME_{MIN,MAX}_SECONDS, AUTH_CACHE_MIN_ENTRIES,
  AUTH_TIMEOUT_{MIN,MAX}_SECONDS in ConfigValidator
- JITTER_PERCENT local to src/worker/ (module standalone; no cross-dep)
- SIGINT/SIGTERM_EXIT_CODE, MISSING_VALUE_PLACEHOLDER,
  TEMPLATE_NAME_SUFFIX, TRUNCATED_HASH_ID_LENGTH in ui/cli output
- SKIN_ERROR_RELOAD_COUNT_KEY in ui/web core

Helper adoption:
- millisecondsToSeconds (date-fns) at AbstractUIServer / InMemoryAuthCache / test
- isNotEmptyString in TemplateValidation
- WebSocketReadyState enum (ui-common) at CLI wsIcon/renderers and web stationStatus
- OCPP16ChargePointStatus / OCPP20ConnectorStatusEnumType enum keys instead
  of string literals in CLI STATUS_ABBREVIATIONS and web CONNECTOR_STATUS_VARIANT
- nonEmptyStringOrUndefined shared helper (ui/web/src/shared/utils) adopted
  by 3 composables + skin error extractor
- Extracted OCPPRequestService.logRequestHandlerError() consumed by OCPP16/20
  RequestService (byte-identical catch shell)

Barrel/module hygiene:
- Close barrel gap: meter-values/index.ts exposes disposeCoherentSessionRuntime
  (JSDoc already advertised it)
- UIServerAccessPolicy imports UI_SERVER_ACCESS_POLICY_DEFAULTS via utils/index
  barrel (drop deep import)
- ui/web skins route 8 imports through @/shared/utils/index barrel
- UIServiceWorkerBroadcastChannel type-only imports AbstractUIService via
  ui-server/index barrel (safe by type erasure; comment documents the constraint)

Dead barrel exports pruned (0 external consumers):
- src/utils: FieldError, applyConfigurationMigration, coerceConfigurationVersion,
  queueMicrotaskErrorThrowing (tests updated to import from source)
- charging-station/meter-values: ChargingCurvePoint
- ui-server/mcp: MCPToolSchema
- ocpp/auth: AuthStats, CacheStats, CertificateAuthProvider, CertificateInfo
- ocpp/2.0/__testable__: SendMessageFn, TestableRequestServiceOptions/Result
  re-exports; TestableOCPP20IncomingRequestService downgraded to module-private;
  TestableOCPP20VariableManager type re-export dropped

Anchor-comment reasons added on 3 eslint-disable directives (Utils /
StatisticUtils / UIServerFactory).

Test constants:
- Heartbeat interval sites use Constants.DEFAULT_HEARTBEAT_INTERVAL_MS /
  secondsToMilliseconds
- New tests/utils/TestNetworkConstants.ts: TEST_SUPERVISION_URL derived from
  Constants.DEFAULT_UI_SERVER_{HOST,PORT}; adopted at MessageChannelUtils.test
  + StorageTestHelpers + ConfigurationFixtures
- Rate-limit / mock-timers-tick 60000 literals replaced by minutesToMilliseconds(1)
  in UIServerSecurity / OCPP20CertSigningRetryManager / SignedMeterValues tests

Deliberately not applied (documented rationale):
- src/worker/WorkerAbstract isNotEmptyString adoption: src/worker/ is a
  standalone module with zero cross-module dependencies; retracted rather
  than adding an import to ../utils
- AbstractUIService → broadcast-channel/index value-import via barrel:
  empirically amplifies circular dependencies 62 → 71 because the barrel also
  re-exports ChargingStationWorkerBroadcastChannel whose transitive graph
  closes a runtime cycle back through ui-server/index; deep import kept

Quality gates:
- pnpm typecheck: clean (root + ui-common + cli + web)
- pnpm lint: 0 errors (1 pre-existing cspell warning unchanged)
- pnpm circular-deps: 62 (baseline unchanged)
- pnpm test: 2955/2961 pass (6 skipped pre-existing; 0 fail)

* [autofix.ci] apply automated fixes

* refactor: address self-review findings — DRY constant refs, helper adoption, naming

Fixes 9 findings surfaced by fan-out self-review of commit a25f4ba9.
Behavior-preserving; no public API change.

Findings addressed:

- H1 (DRY, Constants.ts) — `DEFAULT_AUTH_CACHE_MAX_ABSOLUTE_LIFETIME_MS` and
  `MS_PER_DAY` both held the literal `86_400_000` (same value, same commit).
  Class-static forward-reference is illegal in TypeScript (TS2729:
  "Property 'B' is used before its initialization"), so the shared literal is
  hoisted to module-scope `DAY_IN_MS` / `DAY_IN_SECONDS` helpers before the
  class and referenced from both derived defaults and canonical time-unit
  constants. Single source of truth restored.

- H2 (DRY, ConfigValidator.ts) — `AUTH_CACHE_LIFETIME_MAX_SECONDS = 86_400`
  duplicated `Constants.SECONDS_PER_DAY = 86_400`. Fixed cross-file by
  importing `Constants` (already reachable via `../../../../utils/index.js`)
  and referencing `Constants.SECONDS_PER_DAY`.

- M1 (helper adoption, ui/cli format.ts:102) — `fuzzyTime` still used
  `diff / 1000` while the same PR adopted `millisecondsToSeconds` from
  `date-fns` at 4 other sites. Replaced by `millisecondsToSeconds(diff)`
  for consistency.

- L1 (naming coherence, ConfigValidator.ts) — Renamed
  `AUTH_CACHE_LIFETIME_{MIN,MAX}_SECONDS` to
  `AUTH_CACHE_TTL_{MIN,MAX}_SECONDS` to align with the pre-existing
  `DEFAULT_AUTH_CACHE_TTL_SECONDS` naming family. Two words for one concept
  eliminated within the auth module.

- L2 (naming coherence, ui/cli output) — Renamed cli
  `MISSING_VALUE_PLACEHOLDER` to `EMPTY_VALUE_PLACEHOLDER` to align with
  ui/web's pre-existing `EMPTY_VALUE_PLACEHOLDER`. Values remain
  medium-specific (`'–'` cli / `'Ø'` web), only the semantic role name is
  shared. 6 sites updated.

- L3 (stranded export, format.ts) — `TRUNCATED_HASH_ID_LENGTH` had zero
  external consumers (only the in-file default arg on `truncateId`).
  Dropped `export` keyword.

- L4 (stranded export, format.ts) — `TEMPLATE_NAME_SUFFIX` had zero external
  consumers (only the in-file `stripTemplateSuffix` helper). Dropped
  `export` keyword; the helper itself keeps `export` (2 external consumers).

- L5 (documented duplication, WorkerUtils.ts) — JSDoc on `JITTER_PERCENT`
  now explicitly cross-references `Constants.DEFAULT_RECONNECT_JITTER_PERCENT`
  and documents the maintenance invariant. The two constants must stay
  synchronized manually because `src/worker/` is a standalone module (no
  cross-module value imports).

- I1 (anchor accuracy, StatisticUtils.ts:52) — The
  `no-unnecessary-condition` disable reason is reworded from a plausible-
  but-imprecise "JSON.parse violation" framing to the actual root cause:
  `baseIndex+1` can equal `sortedDataSet.length`, and TypeScript indexes
  as `number` (no `noUncheckedIndexedAccess`), so the runtime `!= null`
  guard is genuinely needed for out-of-bounds protection.

Empirical retraction verifications:

- B1-5 (cycle amplification 62 → 71) — Re-verified by applying only the
  retracted change on top of `daad8a9d` and running `pnpm circular-deps`:
  62 baseline, 71 after B1-5, 62 after revert. Retraction rationale holds.

- A1-17 (`src/worker/` standalone) — Re-verified by `grep -rE
  "^import.*\.\./" src/worker/`: zero cross-module value imports; the new
  L5 JSDoc documents the exception rationale in-source.

Quality gates:
- pnpm typecheck: clean (root + ui-common + cli + web)
- pnpm lint: 0 errors (1 pre-existing cspell warning unchanged)
- pnpm circular-deps: 62 (baseline unchanged)
- pnpm test: 2955/2961 pass (6 skipped pre-existing; 0 fail)

* docs: fix JSDoc accuracy from review v3 findings (G1, A1, A2)

Doc-only follow-up to commit 6ba8503c. Behavior unchanged.

- G1 (JITTER doc accuracy) — Reworded both `Constants.DEFAULT_RECONNECT_JITTER_PERCENT`
  and worker-local `JITTER_PERCENT` JSDoc from the empirically false claim
  "symmetric ±20 %" to "bounds the jitter within ±20 %". The `randomizeDelay`
  algorithm couples sign and magnitude through a single uniform draw, so with
  `random ∈ [0, 1)` and `sign = random < 0.5 ? -1 : +1` the output range is
  `(0.9·delay, delay] ∪ [1.1·delay, 1.2·delay)` — asymmetric, with a gap at
  `[delay, 1.1·delay)`. The new wording accurately caps the magnitude without
  overclaiming symmetry; the WorkerUtils docstring adds an explicit distribution
  note pointing to `randomizeDelay`. Algorithm itself pre-dates the PR and is
  behavior-preserved.

- A1 (InMemoryAuthCache @param defaults) — Three JSDoc `@param default:` fields
  were pointing at raw literals (3600, 86400000, 60000) instead of the promoted
  Constants members. Aligned with the pattern already used for `maxEntries`
  (which cites `Constants.DEFAULT_AUTH_CACHE_MAX_ENTRIES`): now all six
  `@param default:` values reference the canonical `Constants.*` symbol.

- A2 (DEFAULT_REMOTE_AUTH_CACHE_TTL_SECONDS wording) — JSDoc used the phrase
  "distinct from local `_TTL_SECONDS = 3600`" where `_TTL_SECONDS` is not an
  actual symbol. Replaced by explicit sibling reference
  `DEFAULT_AUTH_CACHE_TTL_SECONDS (3600, local cache default)`.

Quality gates:
- pnpm typecheck: clean (root + ui-common + cli + web)
- pnpm lint: 0 errors (1 pre-existing cspell warning unchanged)
- pnpm circular-deps: 62 (baseline unchanged)

* fix: web-test regression from A2-5 + review v4 findings + documented quality gates

Restores web test suite to full green after documented quality gates surfaced
regressions the prior review rounds missed. Behavior-preserving vs the intent
of round-1 refactor.

Regressions fixed (QG-1/QG-2/QG-3 — blockers surfaced by `pnpm --filter web
test:coverage`):

- `stationStatus.ts` — round-1 finding A2-5 replaced `switch (status?.toLowerCase())`
  by a Record keyed by `OCPP16ChargePointStatus.*` / `OCPP20ConnectorStatusEnumType.*`
  enum values. That change (a) dropped the case-insensitive contract asserted by
  `stationStatus.test.ts:74`, and (b) introduced a module-scope runtime access
  to `ui-common` enum values, which fails at module init when the containing
  test suite mocks `ui-common` (2 file-level FAILs in `useAddStationsForm.test.ts`
  and `useStartTxForm.test.ts`).

  Design chosen (Option F in review v4 report): keep the Record structure but
  key it by lowercase string literals and normalize input via `.toLowerCase()`
  on lookup. Retains the refactor benefit (Record > switch for O(1) lookup,
  clearer add/remove semantics), restores case-insensitive input, removes the
  module-scope enum dependency, and preserves behaviour for the 9 status values
  the switch covered. Enum imports dropped since no other value site remains.

  Web test suite: 31 files / 532 tests pass (previously 3 files failed / 1
  test failed).

Review v4 low-severity findings:

- v4-A1 `Constants.ts:77` — JSDoc "bounds within ±20 %" for
  `DEFAULT_RECONNECT_JITTER_PERCENT` implied a bidirectional range, but algebra
  of the sole consumer `computeExponentialBackOffDelay` (`jitter = delay ×
  jitterPercent × secureRandom()` with `secureRandom() ∈ [0, 1)`) yields
  `jitter ∈ [0, +20 %)` strictly non-negative. Reworded to describe the
  additive-uniform-draw semantic and cross-reference the two distinct consumer
  distributions.
- v4-A2 `WorkerUtils.ts:12` — "shared jitter policy" overclaimed behavioural
  parity between the two consumers (positive-only vs asymmetric with a
  probability-zero gap). Softened to "shared jitter magnitude cap".
- v4-B1 `ConfigurationFixtures.ts:67` — `uiServer.options` still hardcoded
  `host: 'localhost', port: 8080` after the sibling `supervisionUrls` field on
  the same object literal was migrated to `TEST_SUPERVISION_URL`. Extended
  `tests/utils/TestNetworkConstants.ts` to also export `TEST_UI_SERVER_HOST`
  and `TEST_UI_SERVER_PORT` (re-exports of `Constants.DEFAULT_UI_SERVER_HOST` /
  `Constants.DEFAULT_UI_SERVER_PORT`).
- v4-B4 `ConfigurationMigrations.test.ts:130,141` — string literals
  `'ws://localhost:8080'` in a file the PR already touched (barrel-to-source
  import migration) but where the URL literals were skipped in round 2.
  Migrated to `TEST_SUPERVISION_URL`.

Lint policy alignment (user directive "aucun warning"):

- `cspell.config.yaml` — added `subclassing` (used in
  `src/charging-station/ui-server/index.ts:10`, pre-existing since #1954). Root
  lint now reports 0 errors AND 0 warnings.

Deferred (documented in review v4 report, out of this PR's scope):

- v4-B2 `ConfigValidator.ts` MIN/MAX bounds harmonization (already tracked as M2)
- v4-B3 asymmetry between `1.6/__testable__/index.ts` (exports
  `TestableOCPP16IncomingRequestService`) and `2.0/__testable__/index.ts`
  (interface demoted to non-exported in round 2). Requires either restoring
  the 2.0 export or migrating 1.6 consumers to `ReturnType<typeof …>` —
  larger touch, kept for a dedicated follow-up.
- Pre-existing gap/asymmetry in `randomizeDelay`'s distribution
  (`(−10 %, 0] ∪ [+10 %, +20 %)` — magnitude and sign both derive from a single
  uniform draw). Documented in-source, not fixed by this refactor.

Quality gates (documented per `.serena/memories/task_completion_checklist`):

- Root: `pnpm format` / `typecheck` / `lint` (0 err, 0 warn) / `build` / `test`
  (2955 pass / 6 skipped / 0 fail)
- CLI: `pnpm format` / `typecheck` / `lint` / `build` / `test` (152/152 pass)
- Web: `pnpm format` / `typecheck` / `lint` / `build` / `test:coverage`
  (532/532 pass, 31 test files)
- Circular deps: 62 (baseline unchanged)

* docs: tighten prose in 5 comments — state-describing, not narrative

Removes historical/imperative framing ("kept as", "hoisted to", "if a future
edit converts...", "must not access... because") in favour of concise
statements of what is.

- `Constants.ts:9` module-scope helper: 3→2 lines
- `Constants.ts:80` DEFAULT_RECONNECT_JITTER_PERCENT JSDoc: 6→4 lines
- `WorkerUtils.ts:1` JITTER_PERCENT JSDoc: 9→5 lines
- `UIServiceWorkerBroadcastChannel.ts:1` type-only barrier: 5→3 lines
- `stationStatus.ts:64` CONNECTOR_STATUS_VARIANT: reworded, same length

Net: -9 lines. All invariants preserved (TS2729, cross-module boundary,
runtime-cycle prevention, no-module-scope-enum). Behavior unchanged.

Quality gates:
- Root: format / typecheck / lint (0 err, 0 warn) / build / test (2955 pass)
- CLI: 5/5 clean (152/152 pass)
- Web: 5/5 clean (532/532 pass, test:coverage)

* fix: address review v5 findings — imperative comment, dead exports, module-scope enum

Applies the 3 actionable findings from review round 5. Behavior-preserving.

- v5-A1 `SkinLoadError.vue:28-32` — Reworded `resetToDefault` JSDoc from 4-line
  imperative `NOTE: ...should clear the reload-count sessionStorage key...`
  to 1-line state-describing `Counter is reset by useSkin.switchSkin on
  successful load.` Missed by the round-4 comment tightening sweep.

- v5-B1 `TestNetworkConstants.ts` / `ConfigurationFixtures.ts` — Retired
  `TEST_UI_SERVER_HOST` / `TEST_UI_SERVER_PORT` exports (1 external consumer
  each, both at the same `ConfigurationFixtures.ts:71` expression — criterion 10
  requires ≥2 consumers). Inlined `Constants.DEFAULT_UI_SERVER_HOST` /
  `Constants.DEFAULT_UI_SERVER_PORT` at the single call site.
  `TEST_SUPERVISION_URL` remains exported (5 consumers).

- v5-B2 `ui/web/src/shared/utils/stationStatus.ts` — Removed the module-scope
  value import `import { WebSocketReadyState } from 'ui-common'` and switched
  `getWebSocketStateVariant` to 4 module-local numeric constants
  `WS_STATE_{CONNECTING,OPEN,CLOSING,CLOSED} = 0/1/2/3`. Eliminates the same
  ui-common runtime module-scope dependency that the round-4 fix removed for
  `CONNECTOR_STATUS_VARIANT`. No behavior change (values match WHATWG WebSocket
  readyState spec).

Quality gates:
- Root: 5/5 clean, 2942/2948 pass
- CLI: 5/5 clean, 152/152 pass
- Web: 5/5 clean, 532/532 pass (test:coverage)
- Circular deps: 62 (baseline)

* docs: drop narrative comments in stationStatus.ts

Removes two comment blocks that narrated rationale ("mirror of...",
"module-local to keep...", "test suites mock...") instead of describing
what is. The `WS_STATE_*` constants and lowercase Record keys are
self-documenting.

- `stationStatus.ts:62-66` — 4-line rationale block removed; `cspell:ignore`
  directive retained
- `stationStatus.ts:83-85` — 3-line rationale block removed

Quality gates: root/web 5/5 clean, 532/532 pass.

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
61 files changed:
cspell.config.yaml
src/charging-station/ChargingStation.ts
src/charging-station/CoherentMeterValuesManager.ts
src/charging-station/HelpersConfig.ts
src/charging-station/TemplateValidation.ts
src/charging-station/broadcast-channel/UIServiceWorkerBroadcastChannel.ts
src/charging-station/meter-values/index.ts
src/charging-station/ocpp/1.6/OCPP16RequestService.ts
src/charging-station/ocpp/1.6/OCPP16ServiceUtils.ts
src/charging-station/ocpp/2.0/OCPP20CertSigningRetryManager.ts
src/charging-station/ocpp/2.0/OCPP20Constants.ts
src/charging-station/ocpp/2.0/OCPP20RequestService.ts
src/charging-station/ocpp/2.0/OCPP20ServiceUtils.ts
src/charging-station/ocpp/2.0/__testable__/index.ts
src/charging-station/ocpp/OCPPRequestService.ts
src/charging-station/ocpp/OCPPServiceUtils.ts
src/charging-station/ocpp/OCPPSignedMeterDataGenerator.ts
src/charging-station/ocpp/auth/cache/InMemoryAuthCache.ts
src/charging-station/ocpp/auth/index.ts
src/charging-station/ocpp/auth/strategies/RemoteAuthStrategy.ts
src/charging-station/ocpp/auth/utils/ConfigValidator.ts
src/charging-station/ui-server/AbstractUIServer.ts
src/charging-station/ui-server/UIServerAccessPolicy.ts
src/charging-station/ui-server/UIServerFactory.ts
src/charging-station/ui-server/UIWebSocketServer.ts
src/charging-station/ui-server/mcp/index.ts
src/performance/PerformanceStatistics.ts
src/utils/Constants.ts
src/utils/StatisticUtils.ts
src/utils/Utils.ts
src/utils/index.ts
src/worker/WorkerUtils.ts
tests/charging-station/ChargingStation-Configuration.test.ts
tests/charging-station/ocpp/1.6/OCPP16ServiceUtils-SignedMeterValues.test.ts
tests/charging-station/ocpp/2.0/OCPP20CertSigningRetryManager.test.ts
tests/charging-station/ui-server/UIServerSecurity.test.ts
tests/performance/storage/StorageTestHelpers.ts
tests/utils/ConfigurationMigrations.test.ts
tests/utils/MessageChannelUtils.test.ts
tests/utils/TestNetworkConstants.ts [new file with mode: 0644]
tests/utils/Utils.test.ts
tests/utils/helpers/ConfigurationFixtures.ts
ui/cli/src/client/lifecycle.ts
ui/cli/src/output/format.ts
ui/cli/src/output/renderers.ts
ui/web/src/core/Constants.ts
ui/web/src/core/index.ts
ui/web/src/shared/components/SkinLoadError.vue
ui/web/src/shared/composables/useAddStationsForm.ts
ui/web/src/shared/composables/useSkin.ts
ui/web/src/shared/composables/useStartTxForm.ts
ui/web/src/shared/utils/index.ts
ui/web/src/shared/utils/nonEmptyString.ts [new file with mode: 0644]
ui/web/src/shared/utils/stationStatus.ts
ui/web/src/skins/classic/ClassicLayout.vue
ui/web/src/skins/classic/components/charging-stations/CSData.vue
ui/web/src/skins/modern/components/ConnectorRow.vue
ui/web/src/skins/modern/components/SimulatorBar.vue
ui/web/src/skins/modern/components/StationCard.vue
ui/web/src/skins/modern/components/dialogs/SetSupervisionUrlDialog.vue
ui/web/src/skins/modern/utils/errors.ts