Files
EVOLV/.claude/refactor/CONVENTIONS.md
znetsixe 91e4255ef5 Add refactor planning docs (.claude/refactor/)
Platform-wide refactor plan: README, CONVENTIONS, CONTRACTS,
MODULE_SPLIT, TASKS, OPEN_QUESTIONS. Source of truth for the
phased refactor across all 12 submodules.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-10 18:22:35 +02:00

5.9 KiB

Conventions

These rules apply to every file written or edited during the refactor. They override personal preference. Be explicit about deviations in OPEN_QUESTIONS.md.

File size

Type Soft target Hard cap
Domain module (one class / one concern) ≤ 200 lines 300 lines
Pure-function utility module ≤ 150 lines 250 lines
Test file (one .test.js) ≤ 300 lines 500 lines
Markdown spec (in this dir)

If you go over the soft target, ask: is this two concerns? If yes, split. Split before refactoring callers — the smaller pieces test easier.

Function size

  • Soft target: ≤ 30 lines.
  • Hard cap: 60 lines (excluding comments).
  • A switch with mostly-trivial cases counts as one statement, not many.
  • A long pure-math function (e.g. an integrator) is OK if it can't be meaningfully split.

Comments

Lead with the rule: default to no comments. Add one only when why is non-obvious to a reader who can already read the code.

Good comments:

// Latest-wins: if a new demand arrives mid-dispatch, queue it and
// pick up after the current dispatch settles. Without this gate
// every PS tick aborts in-flight pump ramps.

Bad comments:

// Set inflow to the value
this.inflow = value;
// Loop over machines
for (const m of machines) { ... }

Function-level docstring policy:

  • One short line above the function describing what it produces when the name alone isn't enough.
  • Skip JSDoc @param blocks unless the function is part of a public contract (the things in CONTRACTS.md). Inline destructuring + good names beats JSDoc that drifts.
  • Never write multi-paragraph docstrings.

Inline comments inside a function:

  • Use to flag a non-obvious invariant, a workaround, or a regression guard. Reference a ticket / commit SHA only if the workaround is load-bearing.
  • Never narrate what the next line does.

Naming

Thing Convention Example
File holding a class PascalCase.js matching the class name BasinGeometry.js
File of utilities / pure functions camelCase.js flowAggregator.js
Folder under src/ camelCase (concern, plural for collections) control/, strategies/, commands/
Class PascalCase class BasinGeometry
Function / method camelCase selectBestNetFlow()
Private method (convention only) leading _ _validateThresholdOrdering()
Constant UPPER_SNAKE_CASE CANONICAL_UNITS
Module-private leading _ on the local const _DEFAULTS = {...}
Test file <name>.<tier>.test.js flowAggregator.basic.test.js

Imports

  • A node may import from:
    • generalFunctions (the shared lib)
    • its own src/ tree
    • Node built-ins (events, path, ...)
    • declared dependencies in its package.json
  • A node MUST NOT import from another node's src/.
  • Cross-node coupling happens only through:
    • the shared generalFunctions API
    • Node-RED messages (Port 0/1/2)
    • the parent/child registration handshake (childRegistrationUtils)
  • Avoid deep imports inside generalFunctions. Always import from the package root: const { logger } = require('generalFunctions'). Exception: tests for generalFunctions itself.

Module shape

Default to one default export per file when the file is named after the thing it exports (a class, a singleton). Use named exports for collections of small utilities.

// File: BasinGeometry.js
class BasinGeometry { ... }
module.exports = BasinGeometry;
// File: flowAggregator.js
function selectBestNetFlow(ctx) { ... }
function updatePredictedVolume(ctx) { ... }
module.exports = { selectBestNetFlow, updatePredictedVolume };

Error handling

  • Validate at boundaries (Node-RED input handler, child registration). Trust internal calls — don't re-validate parameters that already passed an outer check.
  • Logging on a recoverable issue: logger.warn once, fall back to a safe default, continue. Don't throw.
  • Logging on an unrecoverable issue: logger.error and stop ticking the affected subsystem (don't crash Node-RED).
  • Hard fail (throw) only for invariant violations the caller can't recover from (e.g. config schema mismatch detected at construction time).

Logging

  • Use the generalFunctions logger exclusively. No console.log.
  • Log levels:
    • error: something is wrong and downstream behaviour will be affected.
    • warn: something is unexpected; falling back to a safe default.
    • info: state transitions of operational interest (mode changes, child registrations, calibrations).
    • debug: per-tick / per-event traces.
  • Do not ship enableLog: "debug" in any default config or example flow. Logs flood within seconds.

Testing

Three tiers per module, mirroring the existing structure:

test/
  basic/<module>.basic.test.js          # one module in isolation
  integration/<feature>.integration.test.js  # multiple modules together
  edge/<scenario>.edge.test.js          # edge cases / regressions

Rules:

  • Every new module from a refactor gets at least a basic test.
  • Every regression discovered during refactor gets an edge test pinning it.
  • Tests run with node --test. No external test framework.
  • A PR may not lower the green-test count.
  • Production-readiness ("trial-ready") still requires Docker E2E in addition to node --test. See per-node memory.

Pure-domain rule (specificClass and below)

Code under src/ (other than nodeClass.js) is pure domain. It must not:

  • Touch RED.*
  • Read process.env
  • Assume Node-RED is running

This makes every domain module testable from a plain Node process.

Observability of changes

When a refactor moves logic from one file to another:

  • Keep behaviour identical at first. Tests pin it.
  • Behavioural changes (renaming a topic, changing a payload shape) go in separate PRs that are explicitly behavioural.
  • git mv for pure relocations so blame stays useful.