# 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:
```js
// 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:
```js
// Set inflow to the value
this.inflow = value;
```

```js
// 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.

```js
// File: BasinGeometry.js
class BasinGeometry { ... }
module.exports = BasinGeometry;
```

```js
// 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.