Repository navigation
fix(color): one canonical color rendering everywhere (#30) - #36
Merged
Merged
Conversation
A board reports a channel color two different ways — the ABGR-packed integer in a ZB/CK snapshot, the raw RGBA bytes in a live PC delta — and a config declares a third, hex. Nothing folded those together, so the two readers disagreed: `get` printed 8f96a3ff while `verify` compared the config's hex against the snapshot's 4288910991 and reported drift on every color, forever. Fix it at the source rather than at the comparison. internal/color is now the single codec: every form folds to 8 lowercase RGBA hex digits, and the client canonicalizes chars values on the way into the state tree, so Get, GetRaw, Snapshot, dump and verify all see the same string. - internal/color: Canonical (any form -> RGBA hex) and Parse (hex -> wire bytes), replacing four separate ad-hoc parsers. - client: canonicalize PC deltas and ZB/CK snapshots at ingest. - boardconfig: Compile pushes canonical hex for both wire and human, Diff compares canonical forms for KindChars, ToConfig emits color so `dump --as-config` -> `verify` round-trips clean on a colored board. - diff: compare []byte bytewise; two byte payloads reaching == panicked. - Document the canonical rendering in README, PROTOCOL, and the skill. Regression coverage that fails on master: Diff across all four color representations, the ToConfig/YAML round-trip, client Get/GetRaw/Snapshot agreement from both a snapshot and a delta, and four e2e cases. Closes #30 Claude-Session: https://claude.ai/code/session_01RGKiwtBjK7VU9xZHMZwMgo
Colors are canonicalized on the way into the state tree, so --raw prints the same 8 hex digits as a humanized read. Say so in the flag help rather than promising a raw wire value it no longer returns. Claude-Session: https://claude.ai/code/session_01RGKiwtBjK7VU9xZHMZwMgo
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #30.
Root cause
The two readers disagreed because a color exists in three shapes and nothing folded them together:
42889109918f 96 a3 ffsetargument8f96a3Client.GetroutedKindCharsthrough a humanizer;Client.Snapshotreturned the tree verbatim.verifyreadsSnapshot, so it compared the config's"8f96a3ff"against the snapshot's4288910991and drifted on every color, forever.Fix
Fix it at the source rather than at the comparison. New
internal/coloris the single codec — every form folds to 8 lowercase RGBA hex digits — and the client canonicalizes chars values on the way into the state tree, soGet,GetRaw,Snapshot,dump, andverifyall see the same string.internal/color—Canonical(any form → RGBA hex) andParse(hex → wire bytes), replacing four separate ad-hoc parsers (client.humanizeColor,client.toCharsVal,helpers.SetColor,cli.parseColor).client— canonicalize PC deltas and ZB/CK snapshots at ingest.boardconfig—Compilepushes canonical hex for both wire and human values,Diffcompares canonical forms forKindChars, andToConfignow emits color sodump --as-config→verifyround-trips.diff— compare[]bytebytewise. Two byte payloads reaching==panicked (comparing uncomparable type []uint8); a live PC delta landing mid-session was enough to hit it. Found while writing the regression tests.Contract changes a reviewer should see
dump --raw --jsonchanged the JSON type for colors:"line/ch19/color": 4288910991→"line/ch19/color": "8f96a3ff". Number to string.--rawno longer means "unnormalized" for color. Colors normalize at ingest, so raw and humanized reads agree. Flag help updated on bothgetanddump.dump --as-confignow emits acolor:line for every channel, including00000000for uncolored ones. Writing00000000to an already-uncolored channel is value-identical (confirmed on the real board), but a dumped config no longer has "absent color line = leave alone" semantics.Regression coverage
All fail on master:
TestDiffColorAcrossRepresentations— packed int / int / float / RGBA bytes / canonical hex / 6-digit hex / uppercase all Diff clean against one config; a different color still drifts, with both sides of the row printed canonically.TestDiffColorDoesNotPanicOnByteSnapshot— pins the==panic.TestToConfigEmitsCanonicalColorandTestToConfigColorSurvivesYAML— thedump --as-config→verifyround-trip, including an all-numeric color YAML would otherwise read back as an integer.TestColorIsCanonicalFromSnapshot/...FromDelta—Get,GetRaw, andSnapshotagree, from both ingest paths.test/apply_verify.bats, plus a color added to theboard-as-code.ymlfixture so the existing apply/verify/dump round-trip covers it.408 unit (was 373), 76 e2e (was 72).
just test,test-race,lint,vet,fmt-check,test-e2eall green.Confirmed on the real 32R
The fake was telling the truth. On the live board:
get line/ch19/colorandget line/ch19/color --rawboth render8f96a3ff(master:4288910991on--raw).8f96a3,8f96a3ff,#8f96a3,8F96A3FF— verify clean against the real color.ff0000still drifts, exit 1, printed aswant ff0000ff got 8f96a3ff.dump --as-config→verify: clean, 2089 settings, including all 32 channel colors.8f96a3, read back8f96a3ff, verified clean against a 6-digit config, restored to00000000. No other board state touched — noapply, noreset.https://claude.ai/code/session_01RGKiwtBjK7VU9xZHMZwMgo