From b9f3d0679cbc7de0a3fd2391d710eb4e533cd8bc Mon Sep 17 00:00:00 2001 From: you Date: Tue, 24 Mar 2026 21:03:07 +0000 Subject: [PATCH] docs: add engineering principles to AGENTS.md DRY, SOLID, code reuse, dependency injection, testability, type safety, performance. These were being violated repeatedly (5 implementations of disambiguation, .toFixed on strings, etc). Now explicitly codified as rules. --- AGENTS.md | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 3e0efd9..192c2d7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -190,6 +190,41 @@ Tests that need live mesh data can use `https://analyzer.00id.net` — all API e - Anything with edge cases (null handling, boundary values) - UI interactions that exercise frontend code branches +## Engineering Principles + +These aren't optional. Every change must follow these principles. + +### DRY — Don't Repeat Yourself +If the same logic exists in two places, it MUST be extracted into a shared function. We had **5 separate implementations** of hash prefix disambiguation across the codebase — that's a maintenance nightmare and a bug factory. One implementation, imported everywhere. + +**Before writing new code, search the codebase for existing implementations.** `grep -rn 'functionName\|pattern' public/ server.js` takes 2 seconds and prevents duplication. + +### SOLID Principles +- **Single Responsibility**: Each function does ONE thing. A 200-line function that fetches, transforms, renders, and caches is wrong. Split it. +- **Open/Closed**: Add behavior by extending, not modifying. Use callbacks, options objects, or configuration — not `if (caller === 'live')` branches inside shared code. +- **Dependency Injection**: Functions should accept their dependencies as parameters, not reach into globals. `resolveHops(hops, nodeList)` — not `resolveHops(hops)` where it secretly reads `window.allNodes`. This makes functions testable in isolation. +- **Interface Segregation**: Don't force callers to depend on things they don't need. If a function returns 20 fields but the caller uses 3, consider a simpler return shape or let the caller pick. + +### Code Reuse +- **Shared helpers go in shared files.** Frontend: `roles.js`, `hop-resolver.js`. Backend: `server-helpers.js`, `decoder.js`. +- **Don't copy-paste between files.** If `live.js` needs the same algorithm as `packets.js`, import it from a shared module. If the shared module doesn't exist yet, create one. +- **Parameterize, don't duplicate.** If two callers need slightly different behavior, add a parameter — don't fork the function. + +### Testability +- **Write functions that are easy to test.** Pure functions (input → output, no side effects) are ideal. If a function reads from the DOM, the DB, and localStorage, it's untestable without mocking everything. +- **Dependency injection enables testing.** Pass the node list, the map reference, the API function as parameters. Tests can substitute fakes. +- **Test the real code, not copies.** Don't paste a function into a test file and test the copy. Import/require the actual module. If the module isn't importable (IIFE, browser-only), refactor it so it is — or use `vm.createContext` like `test-frontend-helpers.js` does. +- **Every bug fix gets a regression test.** If it broke once, it'll break again. The test proves it stays fixed. + +### Type Safety (without TypeScript) +- **Cast at the boundary.** Data from the DB, API, or localStorage may be strings when you expect numbers. Cast early: `Number(val)`, `parseInt(val)`, `String(val)`. Don't let type mismatches propagate deep into logic where they cause cryptic `.toFixed is not a function` errors. +- **Null-check before method calls.** `val != null ? Number(val).toFixed(1) : '—'` — not `val.toFixed(1)`. + +### Performance Awareness +- **No per-item API calls.** Fetch bulk data once, filter/transform client-side. +- **No O(n²) in hot paths.** The packets page has 30K+ rows. A nested loop over all packets × all nodes = 20 billion operations. Use Maps/Sets for lookups. +- **Cache expensive computations.** If you compute the same thing on every render, cache it and invalidate on data change. + ## Common Pitfalls | Pitfall | Times it happened | Prevention |