normaliseCal was using String.length/.slice() (UTF-16 code units), so multi-byte characters like °, Ω, µ could slip through oversized. Now uses TextEncoder to slice at 16 bytes, then repairs any incomplete trailing UTF-8 sequence by walking back over continuation bytes to find the lead byte and dropping the incomplete rune — exactly mirroring Go's utf8.DecodeLastRuneInString loop and the C++ walk-back in StreamHub::SetCalibrationEntry. Adds 4 new test cases covering the non-ASCII/boundary scenarios, and corrects the canonical test command in the task-6 report to 'cd Client/udpstreamer && node --test'. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
171 lines
9.5 KiB
Markdown
171 lines
9.5 KiB
Markdown
# 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 `<script src="/calibration.js"></script>` immediately before the existing `<script src="/app.js"></script>`.
|
||
|
||
## 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)
|