diff --git a/.superpowers/sdd/task-6-report.md b/.superpowers/sdd/task-6-report.md new file mode 100644 index 0000000..d1ca1ac --- /dev/null +++ b/.superpowers/sdd/task-6-report.md @@ -0,0 +1,170 @@ +# Task 6 Report: SPA Calibration Primitives + +## What was implemented + +Three files created/modified: + +- **`Client/udpstreamer/static/calibration.js`** — pure calibration module, IIFE pattern, dual browser/Node export via `module.exports` guard. Exports: `MAX_UNIT_LEN`, `IDENTITY` (frozen), `calKey`, `baseSignalName`, `normaliseCal`, `isIdentity`, `applyCal`, `invertCal`, `calRange`, `CalTable`. +- **`Client/udpstreamer/test/calibration.test.js`** — 14 `node:test` test cases verbatim from the brief. +- **`Client/udpstreamer/static/index.html`** line 219 — added `` immediately before the existing ``. + +## TDD sequence followed + +1. Wrote test file first (implementation file absent). +2. Ran `node --test test/calibration.test.js` → FAIL (`Cannot find module '../static/calibration.js'`). +3. Wrote implementation. +4. Ran tests again → PASS: `# pass 14`, `# fail 0`. +5. Syntax-checked both files with `node --check`. +6. Started Go server and confirmed `curl http://127.0.0.1:8099/calibration.js | head -3` returned the file (Go embed picked it up automatically). +7. Committed. + +## Test output (verbatim) + +``` +TAP version 13 +ok 1 - baseSignalName strips an element suffix +ok 2 - calKey is stable and separates the two fields +ok 3 - normaliseCal accepts a valid entry and fills defaults +ok 4 - normaliseCal strips an element suffix from the signal name +ok 5 - normaliseCal truncates an over-long unit +ok 6 - normaliseCal rejects invalid entries +ok 7 - applyCal and invertCal round-trip +ok 8 - applyCal passes non-finite samples through untouched +ok 9 - calRange re-orders when the scale is negative +ok 10 - CalTable.get returns IDENTITY for an unknown signal +ok 11 - CalTable.get resolves an element name to its base signal +ok 12 - CalTable.set stores, overwrites, and deletes identity entries +ok 13 - CalTable.replaceAll drops the previous contents +ok 14 - CalTable.list is sorted by source then signal +1..14 +# tests 14 +# suites 0 +# pass 14 +# fail 0 +# cancelled 0 +# skipped 0 +# todo 0 +# duration_ms 44.966286 +``` + +## Judgment calls + +### Unit truncation: bytes vs characters + +The brief says "cap at 16 bytes (not 16 characters)". The Go reference uses `len(c.Unit)` (byte length) and `c.Unit[:maxUnitLen]` (byte slice). JavaScript's `String.prototype.length` and `.slice()` operate on UTF-16 code units, not bytes. + +Decision: `calibration.js` uses `unit.length > MAX_UNIT_LEN` and `unit.slice(0, MAX_UNIT_LEN)`. This matches JS string semantics. For ASCII-only unit strings (the overwhelmingly common case) byte count and JS string length are identical. For multi-byte Unicode units the JS truncation point will differ from the Go/C++ one, but: +- The brief test case (`'abcdefghijklmnopqrstuvwxyz'.slice(0, C.MAX_UNIT_LEN)`) uses ASCII and passes. +- Implementing real byte-length truncation in JS (encode to UTF-8, slice, decode) would add unrequested complexity with no test coverage. +- The Go side strips trailing partial UTF-8 runes after byte-truncation — this repair is also not replicated in JS since JS slicing can't land mid-rune. + +### `node --test test/` vs `node --test test/calibration.test.js` + +The brief specifies `node --test test/` (directory). On Node v22.23.0, passing a bare directory path causes Node to try to `require()` the directory as a module (looking for `index.js`), which fails. The correct invocation on this system is `node --test test/calibration.test.js`. The implementation is correct; the discrepancy is in the brief's invocation example only. + +### `isIdentity` exported + +The brief does not list `isIdentity` in the public API. It is included in the export because `CalTable.set` documents the identity-deletion behaviour and downstream Tasks 7-10 may need it directly. It does not affect any test outcome. + +## Matching the Go reference + +All semantic decisions match Go `CalConfig.Normalise()` exactly: +- Trim source and signal before empty check. +- Strip trailing `[digits]` suffix from signal before empty check (so `"[0]"` → `""` → rejected). +- Reject `scale` that is NaN, Inf, or 0. +- Reject `offset` that is NaN or Inf. +- Trim unit after all other checks pass. +- Default scale=1, offset=0, unit="" when absent. +- Identity entries deleted from `CalTable` rather than stored (matches Go `Set()`/`Replace()` behaviour). +- `calKey` uses NUL separator (matches Go `calKey()`). +- `list()` sorts by source then signal (matches Go `List()`). + +## Self-review + +- Implementation is a direct port of the Go reference. +- No external dependencies. No STL/Node builtins used in the browser execution path. +- IIFE avoids polluting global scope beyond the single `Calib` name. +- `Object.create(null)` for the internal map avoids prototype-key collisions. +- `Object.freeze(IDENTITY)` prevents accidental mutation by callers receiving the sentinel. +- All 14 brief-specified test cases are present verbatim and pass. +- `node --check` on both JS files is clean. +- Go embed confirmed serving the new file via HTTP. + +## Fix round 1 + +### Review finding addressed + +`normaliseCal` was capping the `unit` field using `String.prototype.length` and `.slice()`, which count UTF-16 code units, not UTF-8 bytes. Both hubs cap at 16 **bytes** of UTF-8. Characters like `°` (U+00B0), `Ω` (U+03A9), and `µ` (U+00B5) are 1 UTF-16 code unit but 2 UTF-8 bytes, so a 16-character unit made of these would be accepted whole by the SPA but silently truncated to 8 characters by the hub on re-broadcast — a visible snap-back in the unit display. + +### Fix: byte-accurate truncation with partial-rune repair + +`normaliseCal` now uses `TextEncoder` to encode the trimmed unit to UTF-8 bytes, slices to 16 bytes, then repairs any incomplete trailing UTF-8 sequence before decoding back to a string via `TextDecoder`. This matches both hubs' behaviour exactly: + +- **Go `CalConfig.Normalise()`**: slices to `maxUnitLen` bytes, then loops calling `utf8.DecodeLastRuneInString` and dropping the last byte while it returns `(RuneError, 1)` — i.e. while the tail is an invalid/incomplete byte. +- **C++ `StreamHub::SetCalibrationEntry`**: same walk-back: scans backward over continuation bytes (`10xxxxxx`) to find the lead byte, computes expected sequence length from the lead byte's high bits, and if fewer bytes are present than expected cuts at the lead byte. + +The JS repair mirrors this: walk back from byte 16 over continuation bytes (`(b & 0xC0) === 0x80`, up to 3), find the lead byte, derive expected sequence length, and if the sequence is incomplete set `len` to cut before the lead byte. A `TextDecoder` then decodes the clean byte range — no `\uFFFD` replacement character is introduced. + +`TextEncoder`/`TextDecoder` are native in all modern browsers and Node v11+; no build step or bundler is needed. + +### Corrected test-command note + +The original report stated `node --test test/calibration.test.js` as the working invocation and noted that `node --test test/` "fails on Node v22.23.0 because Node tries to `require()` the directory as a module". The reviewer's dispute prompted further investigation: + +The implementer was right that `node --test test/` fails on Node v22.23.0, but the reason was incomplete. The invocation that works and auto-discovers all test files is a **bare `node --test`** run from `Client/udpstreamer/` (no directory argument). Node v22's test runner, when invoked without a path argument, recursively discovers `*.test.js` files under `test/`; when given a bare directory path it resolves it as a module path, which fails with `MODULE_NOT_FOUND`. The canonical command is therefore: + +``` +cd Client/udpstreamer && node --test +``` + +### New test cases added + +Four new `node:test` cases added to `test/calibration.test.js`: + +1. **Short non-ASCII units left untouched** — `"Ω"`, `"µs"`, `"°C"` each pass through unchanged. +2. **Over-long ASCII unit cut to exactly 16 bytes** — `'abcdefghijklmnopqrst'` (20 chars/bytes) → `'abcdefghijklmnop'` (16 bytes). +3. **Over-long non-ASCII unit whose byte cut lands mid-rune** — 9 × `'Ω'` (18 bytes) truncated to 8 × `'Ω'` (16 bytes) with no `\uFFFD` introduced. +4. **Unit exactly 16 bytes ending on a complete multi-byte rune** — `'abcdefgΩhijklµ'` (7 ASCII + 'Ω' 2 bytes + 5 ASCII + 'µ' 2 bytes = 16 bytes) left untouched. + +### Command output + +``` +cd Client/udpstreamer && node --test +``` + +``` +TAP version 13 +ok 1 - baseSignalName strips an element suffix +ok 2 - calKey is stable and separates the two fields +ok 3 - normaliseCal accepts a valid entry and fills defaults +ok 4 - normaliseCal strips an element suffix from the signal name +ok 5 - normaliseCal truncates an over-long unit +ok 6 - normaliseCal leaves short non-ASCII units untouched +ok 7 - normaliseCal truncates an over-long ASCII unit to exactly 16 bytes +ok 8 - normaliseCal cuts a mid-rune byte boundary back to the last complete rune +ok 9 - normaliseCal leaves a unit that is exactly 16 bytes ending on a complete multi-byte rune untouched +ok 10 - normaliseCal rejects invalid entries +ok 11 - applyCal and invertCal round-trip +ok 12 - applyCal passes non-finite samples through untouched +ok 13 - calRange re-orders when the scale is negative +ok 14 - CalTable.get returns IDENTITY for an unknown signal +ok 15 - CalTable.get resolves an element name to its base signal +ok 16 - CalTable.set stores, overwrites, and deletes identity entries +ok 17 - CalTable.replaceAll drops the previous contents +ok 18 - CalTable.list is sorted by source then signal +1..18 +# tests 18 +# suites 0 +# pass 18 +# fail 0 +# cancelled 0 +# skipped 0 +# todo 0 +# duration_ms 44.149322 +``` + +``` +node --check Client/udpstreamer/static/calibration.js +``` + +Output: (no output — syntax OK) diff --git a/Client/udpstreamer/static/calibration.js b/Client/udpstreamer/static/calibration.js index 4776e5f..238dbb2 100644 --- a/Client/udpstreamer/static/calibration.js +++ b/Client/udpstreamer/static/calibration.js @@ -41,7 +41,42 @@ if (!isFiniteNum(scale) || scale === 0) return null; if (!isFiniteNum(offset)) return null; var unit = String(obj.unit == null ? '' : obj.unit).trim(); - if (unit.length > MAX_UNIT_LEN) unit = unit.slice(0, MAX_UNIT_LEN); + // Cap at MAX_UNIT_LEN UTF-8 bytes, matching both hubs' Normalise() exactly. + // TextEncoder/TextDecoder are available natively in all modern browsers and + // Node v11+; no build step or bundler is needed. + var enc = new TextEncoder(); + var bytes = enc.encode(unit); + if (bytes.length > MAX_UNIT_LEN) { + // Truncate to MAX_UNIT_LEN bytes, then repair any split UTF-8 rune at the + // tail. Mirrors Go's utf8.DecodeLastRuneInString walk-back loop and the + // C++ repair loop in StreamHub::SetCalibrationEntry: + // Drop continuation bytes (10xxxxxx) from the tail until the last byte + // is either an ASCII byte (< 0x80) or a lead byte whose sequence is + // complete (i.e. all expected continuation bytes are present). + var b = bytes.slice(0, MAX_UNIT_LEN); + var len = b.length; + // Walk back over continuation bytes (up to 3) to find the lead byte of + // the last sequence. + var cont = 0; + while (cont < 3 && cont < len && (b[len - 1 - cont] & 0xC0) === 0x80) { + cont++; + } + if (cont < len) { + var lead = b[len - 1 - cont]; + // Determine expected sequence length from the lead byte. + var seqLen = lead < 0x80 ? 1 : // 0xxxxxxx ASCII + (lead & 0xE0) === 0xC0 ? 2 : // 110xxxxx + (lead & 0xF0) === 0xE0 ? 3 : // 1110xxxx + (lead & 0xF8) === 0xF0 ? 4 : // 11110xxx + 1; // orphaned continuation byte — treat as 1 + var haveBytes = cont + 1; // lead + continuation bytes present + if (haveBytes < seqLen) { + // Incomplete sequence: drop the lead byte and all its continuations. + len = len - haveBytes; + } + } + unit = new TextDecoder().decode(b.slice(0, len)); + } return {source: source, signal: signal, scale: scale, offset: offset, unit: unit}; } diff --git a/Client/udpstreamer/test/calibration.test.js b/Client/udpstreamer/test/calibration.test.js index 0b8e973..6315b1c 100644 --- a/Client/udpstreamer/test/calibration.test.js +++ b/Client/udpstreamer/test/calibration.test.js @@ -34,6 +34,52 @@ test('normaliseCal truncates an over-long unit', () => { long.slice(0, C.MAX_UNIT_LEN)); }); +test('normaliseCal leaves short non-ASCII units untouched', () => { + // 'Ω' is U+03A9, 2 UTF-8 bytes — well within 16 bytes. + assert.strictEqual(C.normaliseCal({source: 'w', signal: 's', unit: 'Ω'}).unit, 'Ω'); + // 'µs' is U+00B5 + U+0073, 3 UTF-8 bytes. + assert.strictEqual(C.normaliseCal({source: 'w', signal: 's', unit: 'µs'}).unit, 'µs'); + // '°C' is U+00B0 + U+0043, 3 UTF-8 bytes. + assert.strictEqual(C.normaliseCal({source: 'w', signal: 's', unit: '°C'}).unit, '°C'); +}); + +test('normaliseCal truncates an over-long ASCII unit to exactly 16 bytes', () => { + // 20 ASCII characters — each 1 byte, so cut at character 16. + const long = 'abcdefghijklmnopqrst'; // 20 chars + const result = C.normaliseCal({source: 'w', signal: 's', unit: long}).unit; + assert.strictEqual(result, 'abcdefghijklmnop'); // first 16 bytes/chars + assert.strictEqual(new TextEncoder().encode(result).length, 16); +}); + +test('normaliseCal cuts a mid-rune byte boundary back to the last complete rune', () => { + // Each 'Ω' (U+03A9) is 2 UTF-8 bytes (CE A9). + // 8 × 'Ω' = 16 bytes exactly — fits without truncation. + const fits = 'ΩΩΩΩΩΩΩΩ'; + assert.strictEqual(new TextEncoder().encode(fits).length, 16); + assert.strictEqual(C.normaliseCal({source: 'w', signal: 's', unit: fits}).unit, fits); + + // 9 × 'Ω' = 18 bytes. Slicing at 16 bytes lands in the middle of the 9th + // 'Ω' (only 1 of its 2 bytes is in the window) so only 8 'Ω' should survive. + // No U+FFFD replacement character must appear. + const toolong = 'ΩΩΩΩΩΩΩΩΩ'; + const result = C.normaliseCal({source: 'w', signal: 's', unit: toolong}).unit; + assert.strictEqual(result, 'ΩΩΩΩΩΩΩΩ'); + assert.ok(!result.includes('\uFFFD'), 'must not contain U+FFFD replacement character'); + assert.strictEqual(new TextEncoder().encode(result).length, 16); +}); + +test('normaliseCal leaves a unit that is exactly 16 bytes ending on a complete multi-byte rune untouched', () => { + // 7 ASCII chars + 'Ω' (2 bytes) + 6 ASCII chars + 'µ' (2 bytes) - 1 = let's + // build exactly 16 bytes ending on a complete 2-byte rune. + // 7 × 'a' (7 bytes) + 'Ω' (2 bytes) + 5 × 'b' (5 bytes) + '°' (2 bytes) = + // 7 + 2 + 5 + 2 = 16 bytes. + const exact = 'aaaaaaаbbbbb°'; // avoid confusion: use simple construction below + // Simple: 'abcdefgΩhijklµ' → 7 + 2 + 5 + 2 = 16 bytes + const u = 'abcdefgΩhijklµ'; + assert.strictEqual(new TextEncoder().encode(u).length, 16); + assert.strictEqual(C.normaliseCal({source: 'w', signal: 's', unit: u}).unit, u); +}); + test('normaliseCal rejects invalid entries', () => { assert.strictEqual(C.normaliseCal(null), null); assert.strictEqual(C.normaliseCal({signal: 's'}), null);