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>
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
switchwith 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
@paramblocks unless the function is part of a public contract (the things inCONTRACTS.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
dependenciesin itspackage.json
- A node MUST NOT import from another node's
src/. - Cross-node coupling happens only through:
- the shared
generalFunctionsAPI - Node-RED messages (Port 0/1/2)
- the parent/child registration handshake (
childRegistrationUtils)
- the shared
- Avoid deep imports inside
generalFunctions. Always import from the package root:const { logger } = require('generalFunctions'). Exception: tests forgeneralFunctionsitself.
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.warnonce, fall back to a safe default, continue. Don't throw. - Logging on an unrecoverable issue:
logger.errorand 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
generalFunctionsloggerexclusively. Noconsole.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 mvfor pure relocations so blame stays useful.