test(store): cover appearance-preferences patch normalization

[KrasimirKralev]

What Problem This Solves

internal/store/appearance.go holds the two validators that gate every appearance-preferences update: NormalizeAppearancePreferencesPatch (canonicalize + validate a caller-supplied patch) and AppearancePreferencesPatchEmpty (decide whether there is anything to persist). Both are invoked from both store backends — internal/store/sqlite/sqlite.go:294/:364 and internal/store/postgres/postgres.go:280/:350 — yet shipped with no direct test coverage.

The normalizer carries a subtle contract that is easy to regress silently:

• each field (color_mode, board_theme, message_layout, density) validates against its own allow-list;
• the per-field default alias (system / signal / standard / comfortable) canonicalizes to the empty string — the stored representation of "use the default" — while an explicit value (dark, iris, outlined, compact, …) passes through unchanged;
• an unknown value must return "<field> is invalid", never be persisted;
• a nil field pointer means "not being patched" and must round-trip as nil (the callers gate the DB write on AppearancePreferencesPatchEmpty, so a nil→empty slip would turn an untouched field into a write).

A regression in any of these — dropping the default→empty canonicalization, remapping an allow-list entry, or dropping a field from the Empty conjunction — would not be caught before merge.

Why This Change Was Made

Coverage-only. This adds one new file, internal/store/appearance_test.go (+141, no production code touched), pinning the two validators' current main behavior. The tests drive the real exported functions (same-package white-box, matching the harness/layout of the sibling setup_code_defaults_test.go), not a stub. Scenarios were chosen to bite on the real failure modes — the default-alias canonicalization, the invalid-value gate, the nil round-trip, and each field of the Empty conjunction — rather than to chase line count.

Scope boundary: no runtime/production changes, no new config, defaults, or dependencies; the diff is a single new _test.go under internal/store/.

User Impact

No user-visible or runtime change. For maintainers, the appearance-preferences validation gates now regress loudly instead of silently: a future edit that breaks the default→empty canonicalization, an allow-list, the invalid-value rejection, or the Empty check will fail this suite.

Evidence

Linux, Go 1.26.5, branched off current main (55da179). Run with the repo's standard go test.

6/6 pass on clean main:

$ go test ./apps/api/internal/store/ -run 'Appearance' -v
--- PASS: TestNormalizeAppearancePreferencesPatch_CanonicalizesDefaultsToEmpty
--- PASS: TestNormalizeAppearancePreferencesPatch_PassesThroughExplicitValues
--- PASS: TestNormalizeAppearancePreferencesPatch_NilFieldsStayNil
--- PASS: TestNormalizeAppearancePreferencesPatch_RejectsInvalidPerField (color_mode, board_theme, message_layout, density)
--- PASS: TestAppearancePreferencesPatchEmpty
PASS
ok  	github.com/openclaw/clickclack/apps/api/internal/store

Full package green (go test ./apps/api/internal/store/ → ok), go vet clean, gofmt -l clean.

Non-vacuous — the suite bites when the target is mutated (each mutation applied to appearance.go, suite re-run, then reverted; file restored byte-identical):

Mutation to appearance.go	Result
"system" default alias maps to "system" instead of ""	CanonicalizesDefaultsToEmpty fails
disable the !ok invalid-value guard (accept any value)	RejectsInvalidPerField fails
drop patch.Density == nil from the Empty conjunction	AppearancePreferencesPatchEmpty fails
(reverted — control)	all pass

What was not tested / limitations: the DB-persistence call sites in sqlite.go/postgres.go are out of scope — this pins the pure validator contract they depend on, at the same level as the merged sibling coverage (#104). No production code is touched, so there is no runtime behavior to exercise beyond the exported functions.

Related: none — coverage mined from the module itself; no linked issue.

---

AI-assisted contribution.

---

Generated by Claude Code

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed August 2, 2026, 3:13 AM ET / 07:13 UTC.

ClawSweeper review

What this changes

Adds 141 lines of Go tests for appearance-preference patch normalization, invalid-value rejection, nil preservation, and empty-patch detection in the shared store layer.

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

Keep open: this is a focused, clean test-only PR that adds direct characterization coverage still absent from current main for the appearance-patch validator contract. No actionable correctness or security defect was found; it needs ordinary maintainer review rather than automated repair.

Priority: P3
Reviewed head: 978281f1e1e90348b1e4eb4b9759d6fea5e22a9a

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused, well-explained test addition with credible live validation and no identified correctness blocker.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (live_output): The PR body provides after-fix terminal output for the focused Go tests, full package test, vet, formatting, and deliberate mutation checks; this is appropriate real validation for a test-only change.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (live_output): The PR body provides after-fix terminal output for the focused Go tests, full package test, vet, formatting, and deliberate mutation checks; this is appropriate real validation for a test-only change.
Evidence reviewed	5 items	Current implementation contract: Current main validates each of the four preference fields against its own allow-list, maps default aliases to empty strings, preserves nil pointers, and rejects unknown values. Backend integration points: Both SQLite and PostgreSQL call the shared normalizer and skip persistence when the normalized patch is empty, so direct tests of the shared functions protect both backends' common contract. Existing coverage boundary: Current main has backend lifecycle tests but no direct store-package tests for NormalizeAppearancePreferencesPatch or AppearancePreferencesPatchEmpty; the proposed file fills that focused gap without production changes.
Findings	None	None.
Security	None	None.

How this fits together

Appearance settings updates enter through the SQLite and PostgreSQL stores, which normalize and validate an optional patch before deciding whether to persist any fields. The shared validator determines both accepted preference values and whether an update becomes a database write.

flowchart LR
  A[Appearance settings request] --> B[SQLite or PostgreSQL store]
  B --> C[Shared patch normalizer]
  C --> D[Validation and default canonicalization]
  D --> E{Any field patched?}
  E -->|Yes| F[Persist preference fields]
  E -->|No| G[Return current preferences]

Loading

Before merge

• Complete next step (P2) - No mechanical repair is needed; this clean, test-only PR needs normal maintainer merge review.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Focused test surface	1 file added, 141 lines added, 0 production files changed	The branch is limited to direct regression coverage for the shared validator and does not alter runtime behavior.

Technical review

Best possible solution:

Merge the focused shared-store characterization tests if maintainers want to preserve the current normalization contract against future regressions.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this PR characterizes an existing validator contract rather than reporting a runtime failure; current source and existing backend tests establish the behavior being pinned.

Is this the best way to solve the issue?

Yes: direct tests of the shared pure functions are the narrowest maintainable way to protect behavior used by both store backends, without duplicating production logic or changing configuration.

AGENTS.md: found, but no applicable review policy affected this item.

Codex review notes: model internal, reasoning high; reviewed against b8293987baa5.

Labels

Label justifications:

• P3: This is low-risk regression-test coverage with no user-visible runtime change.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body provides after-fix terminal output for the focused Go tests, full package test, vet, formatting, and deliberate mutation checks; this is appropriate real validation for a test-only change.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides after-fix terminal output for the focused Go tests, full package test, vet, formatting, and deliberate mutation checks; this is appropriate real validation for a test-only change.

Evidence

What I checked:

• Current implementation contract: Current main validates each of the four preference fields against its own allow-list, maps default aliases to empty strings, preserves nil pointers, and rejects unknown values. (apps/api/internal/store/appearance.go:5, b8293987baa5)
• Backend integration points: Both SQLite and PostgreSQL call the shared normalizer and skip persistence when the normalized patch is empty, so direct tests of the shared functions protect both backends' common contract. (apps/api/internal/store/sqlite/appearance.go:29, b8293987baa5)
• Existing coverage boundary: Current main has backend lifecycle tests but no direct store-package tests for NormalizeAppearancePreferencesPatch or AppearancePreferencesPatchEmpty; the proposed file fills that focused gap without production changes. (apps/api/internal/store/appearance_test.go:1, 978281f1e1e9)
• Feature provenance: The current shared appearance validator and both backend helpers appear to date to the same source-introducing commit, with all lines in the shared validator attributed to that commit. (apps/api/internal/store/appearance.go:12, 5000b7ef4295)
• Related merged coverage pattern: The related merged PR established the same focused characterization-test approach for a different store validator; it does not duplicate this PR's appearance-preference coverage. (apps/api/internal/store/setup_code_defaults_test.go:1, 82a41403c931)

Likely related people:

• Shakker: Git history and blame attribute the shared validator and both backend appearance-update helpers to the same source-introducing commit. (role: current implementation author; confidence: high; commits: 5000b7ef4295; files: apps/api/internal/store/appearance.go, apps/api/internal/store/sqlite/appearance.go, apps/api/internal/store/postgres/appearance.go)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (15 earlier review cycles; latest 8 shown)

• reviewed 2026-07-31T20:45:07.802Z sha 978281f :: needs maintainer review before merge. :: none
• reviewed 2026-07-31T22:16:06.579Z sha 978281f :: needs maintainer review before merge. :: none
• reviewed 2026-08-01T09:34:25.882Z sha 978281f :: needs maintainer review before merge. :: none
• reviewed 2026-08-01T11:26:57.284Z sha 978281f :: needs maintainer review before merge. :: none
• reviewed 2026-08-01T14:52:35.326Z sha 978281f :: needs maintainer review before merge. :: none
• reviewed 2026-08-01T17:33:25.349Z sha 978281f :: needs maintainer review before merge. :: none
• reviewed 2026-08-01T21:08:37.430Z sha 978281f :: needs maintainer review before merge. :: none
• reviewed 2026-08-01T23:46:48.660Z sha 978281f :: needs maintainer review before merge. :: none