Repository navigation
feat(tuitheme): read the TUI theme file every terminal UI draws from - #2
Conversation
The 16 terminal colors hold no shade near the background, so a TUI drawn in them can only mark a selected row by reversing it into a slab. A theme generator can write the theme's own selection, panel and accent colors to one file per theme, and every program on this module reads that file. tuitheme.Load reads it: groups ui, markdown and syntax, each value a #rrggbb color, a palette slot "0" to "15", or a style object. A missing key falls back along its dots, then to an embedded default written in palette slots, so a program with no file draws in the terminal's own colors. Theme.Code gives a role as one SGR sequence that gotermstyle.Palette.Paint takes. Notify relays SIGUSR2, which a generator sends after it moves current.json, so a running program loads it again. tuitheme/schema/v1.json documents every key and is the $schema URL a theme file names. Agent-Name: claude-code-unattended Agent-Session: 4773265d-bd77-424c-b9db-836de857b797 Agent-Run: 01a1b9b4-580a-78e4-993a-0b0cbf56760f Worklog: 2555.1
…e does A theme generator writes ui.match as one color, which is a foreground. The built-in default drew it as slot 0 on slot 11, so a program read a match as a background with no theme file and a foreground with one. A foreground match also stays readable on the selected row, as fzf and Telescope draw theirs. Agent-Name: claude-code-unattended Agent-Session: 4773265d-bd77-424c-b9db-836de857b797 Agent-Run: 01a1b9b4-580a-78e4-993a-0b0cbf56760f Worklog: 2555.1
![datapoints-fleet-automation[bot]](https://avatars.githubusercontent.com/u/49924492?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR review — system-design
Used: Read the PR diff and tuitheme/*.go at head. Ran a small Go program that sends SIGUSR2 to itself without calling Notify.
Standards: No rules cited. I ran no fleet standards search, because no finding needed one.
Findings: 0 correctness · 0 design · 0 prose
No findings.
Findings as JSON
{
"used": "Read the PR diff and tuitheme/*.go at head. Ran a small Go program that sends SIGUSR2 to itself without calling Notify.",
"standards": "No rules cited. I ran no `fleet standards search`, because no finding needed one.",
"findings": []
}This review does not gate merge.
There was a problem hiding this comment.
PR review — failure-paths
Used: Checked out the PR head and read tuitheme in full. Ran a throwaway test (since deleted) for null values and Load(""). Ran go test ./tuitheme, which passed.
Standards: Searched "malformed config value silently becomes default". Cited configuration.md § "An anchored path refuses the empty value, and its spelling is a check verbs fault". Cited python.md § "Fail fast instead of defaulting" (a Python rule, applied here by analogy).
Findings: 2 correctness · 0 design · 0 prose
Correctness
failure-paths.1 tuitheme/theme.go:176 · tuitheme/theme.go:178
Reach: A theme file containing "selection": null or "accent": "" reaches it. I ran parse on {"ui":{"selection":null,"accent":""}}. It returned no error and stored an empty Style for both roles.
A null or empty-string role value loads as a valid, empty style, and nothing reports it.
parseValue unmarshals null into a string without error, because JSON null is a no-op for a Go string. The result goes through parseColor(""), which returns unset. The role is stored as Style{}.
The empty style masks both the file's own dotted fallback and the built-in default. The user's ui.selection stops drawing, Code returns "", and the load reports success.
This breaks the documented contract: "A value of neither form fails the load, and the error names its key." Null and "" are neither a color nor a style object. A theme generator that emits null for an unset color is a plausible trigger.
Fix: reject null and "" in parseValue, naming the key. Alternatively, skip the key so it falls back. Add a test for each.
failure-paths.2 tuitheme/theme.go:62 · tuitheme/theme.go:85
Reach: A machine where os.UserHomeDir fails (HOME unset) and XDG_CONFIG_HOME is empty or relative reaches it. DefaultPath returns "". I ran Load(""). It returned the built-in default with a nil error.
DefaultPath swallows the home-directory failure into "". The Load call tuitheme.Load(tuitheme.DefaultPath()), which the README shows, treats "" as a missing file. os.ReadFile("") gives ENOENT, which is fs.ErrNotExist. The program silently draws the default and never learns that the theme location was unresolvable. Every later SIGUSR2 reload repeats the silent default.
This is the same failure class as configuration.md § "An anchored path refuses the empty value".
Fix: have DefaultPath return (string, error). Alternatively, make Load return an error for an empty path.
Findings as JSON
{
"used": "Checked out the PR head and read tuitheme in full. Ran a throwaway test (since deleted) for null values and Load(\"\"). Ran go test ./tuitheme, which passed.",
"standards": "Searched \"malformed config value silently becomes default\". Cited configuration.md § \"An anchored path refuses the empty value, and its spelling is a check verbs fault\". Cited python.md § \"Fail fast instead of defaulting\" (a Python rule, applied here by analogy).",
"findings": [
{
"number": "failure-paths.1",
"section": "correctness",
"sites": [
"tuitheme/theme.go:176",
"tuitheme/theme.go:178"
],
"reach": "A theme file containing `\"selection\": null` or `\"accent\": \"\"` reaches it. I ran `parse` on `{\"ui\":{\"selection\":null,\"accent\":\"\"}}`. It returned no error and stored an empty Style for both roles.",
"body": "A null or empty-string role value loads as a valid, empty style, and nothing reports it.\n\n`parseValue` unmarshals `null` into a string without error, because JSON null is a no-op for a Go string. The result goes through `parseColor(\"\")`, which returns unset. The role is stored as `Style{}`.\n\nThe empty style masks both the file's own dotted fallback and the built-in default. The user's `ui.selection` stops drawing, `Code` returns \"\", and the load reports success.\n\nThis breaks the documented contract: \"A value of neither form fails the load, and the error names its key.\" Null and \"\" are neither a color nor a style object. A theme generator that emits null for an unset color is a plausible trigger.\n\nFix: reject `null` and `\"\"` in `parseValue`, naming the key. Alternatively, skip the key so it falls back. Add a test for each."
},
{
"number": "failure-paths.2",
"section": "correctness",
"sites": [
"tuitheme/theme.go:62",
"tuitheme/theme.go:85"
],
"reach": "A machine where `os.UserHomeDir` fails (HOME unset) and XDG_CONFIG_HOME is empty or relative reaches it. `DefaultPath` returns \"\". I ran `Load(\"\")`. It returned the built-in default with a nil error.",
"body": "`DefaultPath` swallows the home-directory failure into \"\". The `Load` call `tuitheme.Load(tuitheme.DefaultPath())`, which the README shows, treats \"\" as a missing file. `os.ReadFile(\"\")` gives ENOENT, which is `fs.ErrNotExist`. The program silently draws the default and never learns that the theme location was unresolvable. Every later SIGUSR2 reload repeats the silent default.\n\nThis is the same failure class as configuration.md § \"An anchored path refuses the empty value\".\n\nFix: have `DefaultPath` return `(string, error)`. Alternatively, make `Load` return an error for an empty path."
}
]
}This review does not gate merge.
There was a problem hiding this comment.
PR review — stale-prose
Used: I cloned the repo and checked out PR 2 at 8eef34c. I read every added or rewritten sentence beside the code it describes, in these files: CLAUDE.md, README.md, doc.go, tuitheme/theme.go, tuitheme/style.go, tuitheme/reload_unix.go, tuitheme/reload_other.go and the descriptions in tuitheme/schema/v1.json. I went through both commits, so a later commit could not have left an earlier sentence false. I ran a Go program that sends itself SIGUSR2 without calling Notify. It printed "survived" and exited 0, which confirms what README.md and reload_unix.go say.
Standards: pull-requests.md/Sentences the diff adds are checked for what goes stale, and the finding carries the rewrite. documentation.md/A rename, move or behavior change sweeps every doc and comment that describes it, from above every repo.
Findings: 0 correctness · 0 design · 1 prose
Prose
stale-prose.1 doc.go:30 · doc.go:31
False: the inserted tuitheme sentence makes "names cobra for it" refer to tuitheme.
The diff puts a new sentence between the cobrahelp sentence and the go.mod sentence, which was left unchanged:
// that imports it. The subpackage tuitheme reads the theme file a terminal UI
// draws from, and imports only the standard library too. The module's go.mod names cobra for it, so an importer's
"For it" now refers back to tuitheme, the nearest subpackage named. The sentence therefore says go.mod requires cobra for tuitheme. That is false: tuitheme imports only embed, encoding/json, errors, fmt, io/fs, os, os/signal, path/filepath, strconv, strings and syscall. Line 31 is also no longer wrapped like the rest of the comment.
Rewrite: move the tuitheme sentence after the go.mod sentence, and name cobrahelp outright:
// so a tool importing this package builds no other module. The subpackage
// cobrahelp is the cobra frontend, kept apart so cobra reaches only a tool
// that imports it. The module's go.mod names cobra for cobrahelp, so an
// importer's module graph lists cobra while its build compiles none of it.
// The subpackage tuitheme reads the theme file a terminal UI draws from, and
// imports only the standard library.Findings as JSON
{
"used": "I cloned the repo and checked out PR 2 at 8eef34c. I read every added or rewritten sentence beside the code it describes, in these files: CLAUDE.md, README.md, doc.go, tuitheme/theme.go, tuitheme/style.go, tuitheme/reload_unix.go, tuitheme/reload_other.go and the descriptions in tuitheme/schema/v1.json. I went through both commits, so a later commit could not have left an earlier sentence false. I ran a Go program that sends itself SIGUSR2 without calling Notify. It printed \"survived\" and exited 0, which confirms what README.md and reload_unix.go say.",
"standards": "pull-requests.md/Sentences the diff adds are checked for what goes stale, and the finding carries the rewrite. documentation.md/A rename, move or behavior change sweeps every doc and comment that describes it, from above every repo.",
"findings": [
{
"number": "stale-prose.1",
"section": "prose",
"sites": [
"doc.go:30",
"doc.go:31"
],
"reach": "",
"body": "**False: the inserted tuitheme sentence makes \"names cobra for it\" refer to tuitheme.**\n\nThe diff puts a new sentence between the cobrahelp sentence and the go.mod sentence, which was left unchanged:\n\n> `// that imports it. The subpackage tuitheme reads the theme file a terminal UI`\n> `// draws from, and imports only the standard library too. The module's go.mod names cobra for it, so an importer's`\n\n\"For it\" now refers back to tuitheme, the nearest subpackage named. The sentence therefore says go.mod requires cobra for tuitheme. That is false: `tuitheme` imports only `embed`, `encoding/json`, `errors`, `fmt`, `io/fs`, `os`, `os/signal`, `path/filepath`, `strconv`, `strings` and `syscall`. Line 31 is also no longer wrapped like the rest of the comment.\n\nRewrite: move the tuitheme sentence after the go.mod sentence, and name cobrahelp outright:\n\n```go\n// so a tool importing this package builds no other module. The subpackage\n// cobrahelp is the cobra frontend, kept apart so cobra reaches only a tool\n// that imports it. The module's go.mod names cobra for cobrahelp, so an\n// importer's module graph lists cobra while its build compiles none of it.\n// The subpackage tuitheme reads the theme file a terminal UI draws from, and\n// imports only the standard library.\n```"
}
]
}This review does not gate merge.
There was a problem hiding this comment.
PR review — useless-test
Used: I cloned the repo and checked out PR 2 at 8eef34c, and go test ./... passed. I read both new tests beside theme.go, style.go and reload_unix.go. I changed Notify's signal to SIGHUP, and TestSIGUSR2AsksForTheFileAgain failed after 5s. That shows its relay half pins the signal choice. I then restored the file. The diff changes no root-package or cobrahelp code, only a doc.go comment, so no existing test covers changed code.
Standards: testing.md § "Test behavior the user cares about, not what this commit happened to change"; pull-requests.md § "Tests the diff adds are checked for whether they test the program or the change"; testing.md § "Never assert on rendered output — assert the value it was built from".
Findings: 0 correctness · 1 design · 0 prose
Design and architecture
useless-test.1 tuitheme/reload_unix_test.go:28
Reach: Every go test on a unix platform runs it. Every later change to Load or Style.Code must also keep this copy of the ui.selection expectation in step with theme_test.go.
The Load-and-Code tail of TestSIGUSR2AsksForTheFileAgain tests nothing about reloading. Delete it.
Keep the relay half of TestSIGUSR2AsksForTheFileAgain in tuitheme/reload_unix_test.go. It pins SIGUSR2 as the signal, and a theme generator relies on that signal across programs. I changed Notify to listen for SIGHUP and the test failed. No other test catches that.
Delete lines 28–34: the Load(path) call and the ui.selection "\033[0;48;2;45;79;103m" assertion. Also delete the writeTheme call at line 19 and the path variable that only those lines use. The tail checks that Load reads a file written before the signal was sent. Load returns the same theme whether or not a signal ever arrived, so this half cannot fail on any reload bug. It is the scaffolding that showed the change working end to end, kept because it passed. That is the shape named in testing.md § "Test behavior the user cares about, not what this commit happened to change".
The value it asserts is already pinned. TestRoleFallsBackAlongItsDotsThenToTheTerminalsSixteenColors checks an rgb bg reaching 48;2;r;g;b through Load and Code in its ui.selection row. No replacement test is needed.
TestRoleFallsBackAlongItsDotsThenToTheTerminalsSixteenColors stays as it is. Each of its rows asserts a computed value, the SGR code Palette.Paint receives. The rows cover four behaviors:
- the fallback along dots stops at the group
- a role the file omits falls back per role to the default
- each slot and rgb form maps to its SGR parameters
- an empty style yields ""
None of those values is a literal copied out of the code under test.
Findings as JSON
{
"used": "I cloned the repo and checked out PR 2 at 8eef34c, and `go test ./...` passed. I read both new tests beside theme.go, style.go and reload_unix.go. I changed Notify's signal to SIGHUP, and TestSIGUSR2AsksForTheFileAgain failed after 5s. That shows its relay half pins the signal choice. I then restored the file. The diff changes no root-package or cobrahelp code, only a doc.go comment, so no existing test covers changed code.",
"standards": "testing.md § \"Test behavior the user cares about, not what this commit happened to change\"; pull-requests.md § \"Tests the diff adds are checked for whether they test the program or the change\"; testing.md § \"Never assert on rendered output — assert the value it was built from\".",
"findings": [
{
"number": "useless-test.1",
"section": "design",
"sites": [
"tuitheme/reload_unix_test.go:28"
],
"reach": "Every `go test` on a unix platform runs it. Every later change to Load or Style.Code must also keep this copy of the ui.selection expectation in step with theme_test.go.",
"body": "**The Load-and-Code tail of TestSIGUSR2AsksForTheFileAgain tests nothing about reloading. Delete it.**\n\nKeep the relay half of `TestSIGUSR2AsksForTheFileAgain` in `tuitheme/reload_unix_test.go`. It pins SIGUSR2 as the signal, and a theme generator relies on that signal across programs. I changed `Notify` to listen for SIGHUP and the test failed. No other test catches that.\n\nDelete lines 28–34: the `Load(path)` call and the `ui.selection` `\"\\033[0;48;2;45;79;103m\"` assertion. Also delete the `writeTheme` call at line 19 and the `path` variable that only those lines use. The tail checks that `Load` reads a file written before the signal was sent. `Load` returns the same theme whether or not a signal ever arrived, so this half cannot fail on any reload bug. It is the scaffolding that showed the change working end to end, kept because it passed. That is the shape named in `testing.md` § \"Test behavior the user cares about, not what this commit happened to change\".\n\nThe value it asserts is already pinned. `TestRoleFallsBackAlongItsDotsThenToTheTerminalsSixteenColors` checks an rgb `bg` reaching `48;2;r;g;b` through `Load` and `Code` in its `ui.selection` row. No replacement test is needed.\n\n`TestRoleFallsBackAlongItsDotsThenToTheTerminalsSixteenColors` stays as it is. Each of its rows asserts a computed value, the SGR code `Palette.Paint` receives. The rows cover four behaviors:\n- the fallback along dots stops at the group\n- a role the file omits falls back per role to the default\n- each slot and rgb form maps to its SGR parameters\n- an empty style yields \"\"\n\nNone of those values is a literal copied out of the code under test."
}
]
}This review does not gate merge.
There was a problem hiding this comment.
PR review — contracts
Used: I cloned PR head 8eef34c and ran go test ./..., which passed. I loaded six probe theme files through tuitheme.Load from a temporary test file and then removed it. I read the generator side in datapointchris/theme lib/lib.sh _signal_reload, which gates SIGUSR2 on the process's SigCgt bit 11. I read the emboldened caller in datapointchris/gotermboard frame.go:109. I searched the owner's repos for tui-theme, tuitheme, SIGUSR2 and emboldened. Nothing yet reads the new path or package.
Standards: I cited data.md § "A shared file is named in config; only the tool's own default is compiled in", configuration.md § "Every path a tool writes is an XDG base directory" and api-design.md § "An update schema refuses a null for a column that cannot hold one". I also checked help.md § "A fault about how a value is written prints the value as written", and the PR's error text meets it.
Findings: 0 correctness · 2 design · 0 prose
Design and architecture
contracts.1 tuitheme/theme.go:62 · tuitheme/theme.go:7 · README.md:262
Reach: Every full-screen program that calls tuitheme.DefaultPath() inherits the compiled-in path, starting with gotermboard as the PR's first caller. The next change this costs is the first theme test or second theme location. Each reader must then grow its own flag or env var, and each picks a different name.
DefaultPath compiles in a shared file's path and gives no variable to override it.
DefaultPath returns $XDG_CONFIG_HOME/tui-theme/current.json, or ~/.config/tui-theme/current.json. No reader owns that file. A theme generator writes it, and every TUI on the module reads it. data.md § "A shared file is named in config; only the tool's own default is compiled in" asks for one order: an environment variable, then config, then a default. It also bars a compiled-in default that names a path outside the tool's own XDG directories. The library has no variable layer. So there is no one name a user can export to point every reader at a test theme or a second file. Each program would invent its own name, such as --theme, GOTERMBOARD_THEME or TUI_THEME_FILE. The name tui-theme would then gain a second spelling in every reader.
The library is the right place for this, because it already owns the platform split for Notify.
Fix: have DefaultPath read one documented variable first, such as TUI_THEME, and fall back to the XDG path. Name that variable in the package doc and the README bullet. Do it before the first reader ships, because adding it later changes what a reader with that variable set loads.
contracts.2 tuitheme/theme.go:168 · tuitheme/style.go:27 · tuitheme/schema/v1.json:118
Reach: No generator writes this file yet. The next one built for it is the caller. A generator that fills a role from a missing palette entry writes null or "" for it. The datapointchris/theme generators are shell scripts, and a jq lookup of an absent key yields null. That file loads with no error, and the role loses its default.
The reader accepts null and "" as values, though the schema and the README say they fail the load.
The published contract has two parts. The README says "A value of neither form fails the load, and the error names its key." The schema's color pattern ^(#[0-9a-fA-F]{6}|[0-9]|1[0-5])$ rejects both "" and null. parseValue unmarshals null into a string as "", and parseColor("") returns an unset color. The same happens for "fg": "" inside a style object.
I loaded {"ui":{"selection":null}} through Load. It returned no error. theme.Code("ui.selection") was "", while Default().Code("ui.selection") is "\x1b[0;30;47m". The empty value shadows the built-in default, so the selected row loses its highlight with no sign. That is the failure the PR's "Decisions" section says failing the load prevents. The idea of an explicit null being refused rather than read as a value matches api-design.md § "An update schema refuses a null for a column that cannot hold one".
Fix: in parseValue, reject a raw null, and reject a bare-string value of "". Keep "" as unset only for an fg or bg the object leaves out. Detect that case by decoding those two fields as *string, not by the empty string. Then the reader fails exactly the files the schema rejects.
Findings as JSON
{
"used": "I cloned PR head 8eef34c and ran `go test ./...`, which passed. I loaded six probe theme files through `tuitheme.Load` from a temporary test file and then removed it. I read the generator side in datapointchris/theme `lib/lib.sh` `_signal_reload`, which gates SIGUSR2 on the process's SigCgt bit 11. I read the `emboldened` caller in datapointchris/gotermboard `frame.go:109`. I searched the owner's repos for `tui-theme`, `tuitheme`, `SIGUSR2` and `emboldened`. Nothing yet reads the new path or package.",
"standards": "I cited `data.md` § \"A shared file is named in config; only the tool's own default is compiled in\", `configuration.md` § \"Every path a tool writes is an XDG base directory\" and `api-design.md` § \"An update schema refuses a null for a column that cannot hold one\". I also checked `help.md` § \"A fault about how a value is written prints the value as written\", and the PR's error text meets it.",
"findings": [
{
"number": "contracts.1",
"section": "design",
"sites": [
"tuitheme/theme.go:62",
"tuitheme/theme.go:7",
"README.md:262"
],
"reach": "Every full-screen program that calls `tuitheme.DefaultPath()` inherits the compiled-in path, starting with gotermboard as the PR's first caller. The next change this costs is the first theme test or second theme location. Each reader must then grow its own flag or env var, and each picks a different name.",
"body": "**`DefaultPath` compiles in a shared file's path and gives no variable to override it.**\n\n`DefaultPath` returns `$XDG_CONFIG_HOME/tui-theme/current.json`, or `~/.config/tui-theme/current.json`. No reader owns that file. A theme generator writes it, and every TUI on the module reads it. `data.md` § \"A shared file is named in config; only the tool's own default is compiled in\" asks for one order: an environment variable, then config, then a default. It also bars a compiled-in default that names a path outside the tool's own XDG directories. The library has no variable layer. So there is no one name a user can export to point every reader at a test theme or a second file. Each program would invent its own name, such as `--theme`, `GOTERMBOARD_THEME` or `TUI_THEME_FILE`. The name `tui-theme` would then gain a second spelling in every reader.\n\nThe library is the right place for this, because it already owns the platform split for `Notify`.\n\n**Fix:** have `DefaultPath` read one documented variable first, such as `TUI_THEME`, and fall back to the XDG path. Name that variable in the package doc and the README bullet. Do it before the first reader ships, because adding it later changes what a reader with that variable set loads."
},
{
"number": "contracts.2",
"section": "design",
"sites": [
"tuitheme/theme.go:168",
"tuitheme/style.go:27",
"tuitheme/schema/v1.json:118"
],
"reach": "No generator writes this file yet. The next one built for it is the caller. A generator that fills a role from a missing palette entry writes `null` or `\"\"` for it. The datapointchris/theme generators are shell scripts, and a jq lookup of an absent key yields `null`. That file loads with no error, and the role loses its default.",
"body": "**The reader accepts `null` and `\"\"` as values, though the schema and the README say they fail the load.**\n\nThe published contract has two parts. The README says \"A value of neither form fails the load, and the error names its key.\" The schema's `color` pattern `^(#[0-9a-fA-F]{6}|[0-9]|1[0-5])$` rejects both `\"\"` and `null`. `parseValue` unmarshals `null` into a string as `\"\"`, and `parseColor(\"\")` returns an unset color. The same happens for `\"fg\": \"\"` inside a style object.\n\nI loaded `{\"ui\":{\"selection\":null}}` through `Load`. It returned no error. `theme.Code(\"ui.selection\")` was `\"\"`, while `Default().Code(\"ui.selection\")` is `\"\\x1b[0;30;47m\"`. The empty value shadows the built-in default, so the selected row loses its highlight with no sign. That is the failure the PR's \"Decisions\" section says failing the load prevents. The idea of an explicit null being refused rather than read as a value matches `api-design.md` § \"An update schema refuses a null for a column that cannot hold one\".\n\n**Fix:** in `parseValue`, reject a raw `null`, and reject a bare-string value of `\"\"`. Keep `\"\"` as unset only for an `fg` or `bg` the object leaves out. Detect that case by decoding those two fields as `*string`, not by the empty string. Then the reader fails exactly the files the schema rejects."
}
]
}This review does not gate merge.
There was a problem hiding this comment.
PR review — code-architecture
Used: I cloned the repo and checked out PR 2 at 8eef34c. I read every file in the repo, not only the diff. go test ./tuitheme/ passes. A Python script compared the schema's group and key set against default.json, and they agree today. I searched with rg and found no hook or CI step that validates default.json or the schema.
Standards: The finding cites code-quality.md § "A guarantee rests on something that runs, never on prose or a docstring". I searched for "schema and default file kept in step only by prose rule" and for "single source of truth one owner for a list of names". Neither search turned up a nearer rule.
Findings: 0 correctness · 1 design · 0 prose
Design and architecture
code-architecture.1 tuitheme/theme.go:51 · tuitheme/schema/v1.json:12 · tuitheme/schema/v1.json:63 · tuitheme/schema/v1.json:91 · tuitheme/schema/v1.json:118 · tuitheme/default.json:3 · tuitheme/style.go:26 · CLAUDE.md:53
Reach: The next role added to the format costs this, for example a ui.cursor or a markdown.heading.7. CLAUDE.md:49-55 expects these additions: it says the format "only grows" and that a key added to the schema gets its value in default.json in the same change. The schema already lists markdown.heading.4 to markdown.heading.6, and default.json leaves them out. A new group (a fourth beside ui, markdown and syntax) or a new value form costs the same.
Three files decide the theme format's roles, and only a CLAUDE.md sentence keeps them in step
Each fact below has more than one owner:
- The group set (
ui,markdown,syntax) is in two places. One is thegroupsslice intheme.go:51. The other is the schema's top-levelproperties, withadditionalProperties: false. - The role set is in two places. One is the schema's per-group
properties(v1.json:12,:63,:91). The other is the keys indefault.json. CLAUDE.md:53 says every schema key needs a value indefault.jsonor a dotted ancestor there. - The color grammar is in two places. One is the schema's
color.pattern(v1.json:118). The other isparseColor(style.go:26).
All three agree today. I checked: every schema key resolves to a key or ancestor in default.json, and the regex and parseColor accept the same strings. Nothing that runs holds that agreement:
go testnever opens the schema..pre-commit-config.yamland.github/workflows/validate.ymlhave nocheck-jsonschemastep. The PR body says that check was run by hand.mustParseonly checks thatdefault.jsonparses. It does not check coverage.
Adding a role means editing the schema and default.json together. Adding a group means editing theme.go, the schema and default.json together. In both cases a missed edit passes the suite. code-quality.md § "A guarantee rests on something that runs, never on prose or a docstring" covers this: a pairing between two declarations needs a check that runs. CLAUDE.md prose does not count.
Fix: add one Go test in tuitheme. It reads schema/v1.json and asserts three things:
groupsequals the schema's top-level properties minus$schema.- Every property in each group resolves through
lookup(builtIn, …)to a keydefault.jsonactually holds. parseColoraccepts every one of0–15plus a#rrggbbvalue, and rejects16,07and#fff, matching the regex.
Also add a check-jsonschema --schemafile tuitheme/schema/v1.json tuitheme/default.json hook, plus --check-metaschema on the schema, to pre-commit. After that, CLAUDE.md:53 describes a check instead of being the only thing that holds the rule.
Findings as JSON
{
"used": "I cloned the repo and checked out PR 2 at 8eef34c. I read every file in the repo, not only the diff. `go test ./tuitheme/` passes. A Python script compared the schema's group and key set against `default.json`, and they agree today. I searched with rg and found no hook or CI step that validates `default.json` or the schema.",
"standards": "The finding cites `code-quality.md` § \"A guarantee rests on something that runs, never on prose or a docstring\". I searched for \"schema and default file kept in step only by prose rule\" and for \"single source of truth one owner for a list of names\". Neither search turned up a nearer rule.",
"findings": [
{
"number": "code-architecture.1",
"section": "design",
"sites": [
"tuitheme/theme.go:51",
"tuitheme/schema/v1.json:12",
"tuitheme/schema/v1.json:63",
"tuitheme/schema/v1.json:91",
"tuitheme/schema/v1.json:118",
"tuitheme/default.json:3",
"tuitheme/style.go:26",
"CLAUDE.md:53"
],
"reach": "The next role added to the format costs this, for example a `ui.cursor` or a `markdown.heading.7`. CLAUDE.md:49-55 expects these additions: it says the format \"only grows\" and that a key added to the schema gets its value in `default.json` in the same change. The schema already lists `markdown.heading.4` to `markdown.heading.6`, and `default.json` leaves them out. A new group (a fourth beside `ui`, `markdown` and `syntax`) or a new value form costs the same.",
"body": "### Three files decide the theme format's roles, and only a CLAUDE.md sentence keeps them in step\n\nEach fact below has more than one owner:\n\n- **The group set** (`ui`, `markdown`, `syntax`) is in two places. One is the `groups` slice in `theme.go:51`. The other is the schema's top-level `properties`, with `additionalProperties: false`.\n- **The role set** is in two places. One is the schema's per-group `properties` (`v1.json:12`, `:63`, `:91`). The other is the keys in `default.json`. CLAUDE.md:53 says every schema key needs a value in `default.json` or a dotted ancestor there.\n- **The color grammar** is in two places. One is the schema's `color.pattern` (`v1.json:118`). The other is `parseColor` (`style.go:26`).\n\nAll three agree today. I checked: every schema key resolves to a key or ancestor in `default.json`, and the regex and `parseColor` accept the same strings. Nothing that runs holds that agreement:\n\n- `go test` never opens the schema.\n- `.pre-commit-config.yaml` and `.github/workflows/validate.yml` have no `check-jsonschema` step. The PR body says that check was run by hand.\n- `mustParse` only checks that `default.json` parses. It does not check coverage.\n\nAdding a role means editing the schema and `default.json` together. Adding a group means editing `theme.go`, the schema and `default.json` together. In both cases a missed edit passes the suite. `code-quality.md` § \"A guarantee rests on something that runs, never on prose or a docstring\" covers this: a pairing between two declarations needs a check that runs. CLAUDE.md prose does not count.\n\n**Fix:** add one Go test in `tuitheme`. It reads `schema/v1.json` and asserts three things:\n\n1. `groups` equals the schema's top-level properties minus `$schema`.\n2. Every property in each group resolves through `lookup(builtIn, …)` to a key `default.json` actually holds.\n3. `parseColor` accepts every one of `0`–`15` plus a `#rrggbb` value, and rejects `16`, `07` and `#fff`, matching the regex.\n\nAlso add a `check-jsonschema --schemafile tuitheme/schema/v1.json tuitheme/default.json` hook, plus `--check-metaschema` on the schema, to pre-commit. After that, CLAUDE.md:53 describes a check instead of being the only thing that holds the rule."
}
]
}This review does not gate merge.
There was a problem hiding this comment.
PR review — fleet-patterns
Used: I checked out PR 2 and read the whole tuitheme package. I shallow-cloned these sibling repos: theme, gotermboard, msg, prs, projects, helpnav, pytermstyle, dotfiles and tmux-tree-editor. I read theme PR 24's diff, which is the writer of this file, and cloned its tui-theme-file branch. I ran uvx check-jsonschema --schemafile tuitheme/schema/v1.json on all 20 generated themes/*/tui-theme.json, and all 20 passed. I compared default.json with gotermboard's 16-color code and heading table.
Standards: These rules apply: data.md § "A shared file is named in config; only the tool's own default is compiled in", tui.md § "TUI colors follow the terminal's theme", api-design.md § "A value the server owns reaches the client from the server", system-design.md § "A file format's first version is checked against the writers already planned for it before it ships" (met: all 20 planned writer outputs validate) and configuration.md § "Every path a tool writes is an XDG base directory".
Findings: 0 correctness · 3 design · 0 prose
Design and architecture
fleet-patterns.1 tuitheme/theme.go:65 · README.md:265 · theme:lib/lib.sh:518 · theme:lib/lib.sh:2367 · theme:README.md:224
Reach: Every reader the theme repo names in TUI_THEME_READERS would compile this path in: fleet, slate, psyche and projects. The next machine that keeps its theme files elsewhere has to repoint all four by hand. Until then each one falls silently back to 16 colors, because Load returns the default with no error when the file is missing. A relative XDG_CONFIG_HOME already splits them: theme's ${XDG_CONFIG_HOME:-$HOME/.config} at lib.sh:518 writes under the relative path, and DefaultPath ignores it and reads ~/.config.
A compiled-in path to a file the reader does not own replaces the config-named palette file.
DefaultPath compiles in $XDG_CONFIG_HOME/tui-theme/current.json. That directory belongs to the theme generator, not to any program that imports tuitheme. Theme PR 24 states the intent: "so no reader's config needs a line for it".
The fleet already solves this problem the other way:
- tui.md § "TUI colors follow the terminal's theme" says a palette file is "written by a theme generator for each theme and named in the user's config".
- data.md § "A shared file is named in config; only the tool's own default is compiled in" says: "No default may name a path outside the tool's own XDG directories." It resolves the path in this order: environment variable, then config key, then the tool's own default.
- The two existing SIGUSR2 readers in theme work that way. abtop takes
theme = "current"in its ownconfig.toml(theme README.md:224). btop names its theme in its own config.
The PR body gives no reason for the divergence. The existing solution is better because the arrangement stays in each machine's config, where the rule puts it. It does not live in four binaries.
Fix: pick one of two.
- Drop
DefaultPath. Each reader resolves the path from its own environment variable, then its own config key. It callsLoad(path)only when one is set, and reports which layer supplied the path. - Alternatively, rename
DefaultPathto make clear it is only a suggestion the reader's config can override, and add a sanctioned exception to data.md that names this directory.
Either way, the reader's XDG fallback and theme's bash fallback should resolve the same directory.
fleet-patterns.2 tuitheme/schema/v1.json:12 · tuitheme/schema/v1.json:63 · tuitheme/schema/v1.json:91 · theme:lib/generators/tui-theme.py:33 · theme:lib/generators/tui-theme.py:35 · theme:lib/generators/tui-theme.py:44 · theme:lib/generators/tui-theme.py:125
Reach: The next key added to ui or markdown in v1.json gets a value in default.json here, as CLAUDE.md now requires. Nothing makes the generator write it. Every theme then draws that role in the 16-color default, and both suites stay green. A key added only in theme would fail editor validation, because ui and markdown set additionalProperties: false. Theme's tests never validate against the schema, so they would not catch it.
The schema's key vocabulary is hand-copied into the one writer, and no test holds the copy level.
This PR's schema/v1.json owns the format's vocabulary:
- the 11
uiroles: text, muted, accent, selection, match, border, panel, error, warning, info and success - the 21
markdownkeys - the 13 base
syntaxroles
Theme PR 24 types each list out again in tui-theme.py:
- the
ui_groupdict literal MARKDOWN_KEYSSYNTAX_BASESCHEMA_URL
The two copies agree today. I ran uvx check-jsonschema --schemafile tuitheme/schema/v1.json on all 20 generated themes/*/tui-theme.json and got ok -- validation done. Theme's tests/generators.bats runs no such check.
api-design.md § "A value the server owns reaches the client from the server" settles this. The writer reads the owner's document, or its suite checks agreement against it. It never keeps a hand-typed copy.
This PR's CLAUDE.md adds "A key added to the schema gets its value in tuitheme/default.json in the same change". That binds the repo's own default and says nothing of the generator.
Fix: two parts, in theme and here.
- Theme's generator suite validates every generated
tui-theme.jsonagainst the published v1.json, pinned or fetched. - Theme derives
MARKDOWN_KEYSand theuikey set from the schema'sproperties. - Here, the CLAUDE.md bullet names the generator as a second place a new key has to reach.
fleet-patterns.3 tuitheme/default.json:36 · tuitheme/default.json:21 · gotermboard:markdown/code.go:107 · gotermboard:markdown/render.go:164
Reach: gotermboard's markdown renderer is the fleet's default for any Go TUI that draws markdown (tui.md § "A TUI that draws markdown starts from gotermboard's renderer"). A recolor made in tokenCode alone gets no theme file. A recolor made in default.json alone leaves gotermboard's code blocks unchanged. Two copies now carry the 16-color table that tui.md says a theme file should replace. This is weak evidence: only one other copy exists, and today it agrees.
default.json restates gotermboard's 16-color syntax and heading table instead of becoming its source.
The syntax group in default.json is gotermboard's tokenCode written out again, slot for slot:
- comment: 8 italic
- string: 10
- number: 11
- keyword: 13
- type, attribute and function.builtin: 14
- function and tag: 12
The gotermboard side uses its bright Code* constants, which are SGR 90–97. In the same way, heading.1/.2/.3 at 12/12/14 bold copy layout.heading. The only deliberate divergence is ui.match. The latest commit makes it a foreground. gotermboard's Reader draws a match in reverse.
The PR body cites gotermboard for ui.accent but gives no plan for this table. Nothing makes one copy follow the other.
Fix: make tuitheme the owner. gotermboard's tokenCode and heading read theme.Style("syntax.<capture>") and theme.Style("markdown.heading.N") from a *tuitheme.Theme, with tuitheme.Default() when no file is configured. That deletes the second table. Until that change lands, record the plan in the PR body. Without it, the next recolor reaches one table and misses the other.
Findings as JSON
{
"used": "I checked out PR 2 and read the whole `tuitheme` package. I shallow-cloned these sibling repos: theme, gotermboard, msg, prs, projects, helpnav, pytermstyle, dotfiles and tmux-tree-editor. I read theme PR 24's diff, which is the writer of this file, and cloned its `tui-theme-file` branch. I ran `uvx check-jsonschema --schemafile tuitheme/schema/v1.json` on all 20 generated `themes/*/tui-theme.json`, and all 20 passed. I compared `default.json` with gotermboard's 16-color code and heading table.",
"standards": "These rules apply: data.md § \"A shared file is named in config; only the tool's own default is compiled in\", tui.md § \"TUI colors follow the terminal's theme\", api-design.md § \"A value the server owns reaches the client from the server\", system-design.md § \"A file format's first version is checked against the writers already planned for it before it ships\" (met: all 20 planned writer outputs validate) and configuration.md § \"Every path a tool writes is an XDG base directory\".",
"findings": [
{
"number": "fleet-patterns.1",
"section": "design",
"sites": [
"tuitheme/theme.go:65",
"README.md:265",
"theme:lib/lib.sh:518",
"theme:lib/lib.sh:2367",
"theme:README.md:224"
],
"reach": "Every reader the theme repo names in `TUI_THEME_READERS` would compile this path in: fleet, slate, psyche and projects. The next machine that keeps its theme files elsewhere has to repoint all four by hand. Until then each one falls silently back to 16 colors, because `Load` returns the default with no error when the file is missing. A relative `XDG_CONFIG_HOME` already splits them: theme's `${XDG_CONFIG_HOME:-$HOME/.config}` at lib.sh:518 writes under the relative path, and `DefaultPath` ignores it and reads `~/.config`.",
"body": "**A compiled-in path to a file the reader does not own replaces the config-named palette file.**\n\n`DefaultPath` compiles in `$XDG_CONFIG_HOME/tui-theme/current.json`. That directory belongs to the theme generator, not to any program that imports `tuitheme`. Theme PR 24 states the intent: \"so no reader's config needs a line for it\".\n\nThe fleet already solves this problem the other way:\n- tui.md § \"TUI colors follow the terminal's theme\" says a palette file is \"written by a theme generator for each theme and named in the user's config\".\n- data.md § \"A shared file is named in config; only the tool's own default is compiled in\" says: \"No default may name a path outside the tool's own XDG directories.\" It resolves the path in this order: environment variable, then config key, then the tool's own default.\n- The two existing SIGUSR2 readers in theme work that way. abtop takes `theme = \"current\"` in its own `config.toml` (theme README.md:224). btop names its theme in its own config.\n\nThe PR body gives no reason for the divergence. The existing solution is better because the arrangement stays in each machine's config, where the rule puts it. It does not live in four binaries.\n\n**Fix:** pick one of two.\n- Drop `DefaultPath`. Each reader resolves the path from its own environment variable, then its own config key. It calls `Load(path)` only when one is set, and reports which layer supplied the path.\n- Alternatively, rename `DefaultPath` to make clear it is only a suggestion the reader's config can override, and add a sanctioned exception to data.md that names this directory.\n\nEither way, the reader's XDG fallback and theme's bash fallback should resolve the same directory."
},
{
"number": "fleet-patterns.2",
"section": "design",
"sites": [
"tuitheme/schema/v1.json:12",
"tuitheme/schema/v1.json:63",
"tuitheme/schema/v1.json:91",
"theme:lib/generators/tui-theme.py:33",
"theme:lib/generators/tui-theme.py:35",
"theme:lib/generators/tui-theme.py:44",
"theme:lib/generators/tui-theme.py:125"
],
"reach": "The next key added to `ui` or `markdown` in v1.json gets a value in `default.json` here, as CLAUDE.md now requires. Nothing makes the generator write it. Every theme then draws that role in the 16-color default, and both suites stay green. A key added only in theme would fail editor validation, because `ui` and `markdown` set `additionalProperties: false`. Theme's tests never validate against the schema, so they would not catch it.",
"body": "**The schema's key vocabulary is hand-copied into the one writer, and no test holds the copy level.**\n\nThis PR's `schema/v1.json` owns the format's vocabulary:\n- the 11 `ui` roles: text, muted, accent, selection, match, border, panel, error, warning, info and success\n- the 21 `markdown` keys\n- the 13 base `syntax` roles\n\nTheme PR 24 types each list out again in `tui-theme.py`:\n- the `ui_group` dict literal\n- `MARKDOWN_KEYS`\n- `SYNTAX_BASE`\n- `SCHEMA_URL`\n\nThe two copies agree today. I ran `uvx check-jsonschema --schemafile tuitheme/schema/v1.json` on all 20 generated `themes/*/tui-theme.json` and got `ok -- validation done`. Theme's `tests/generators.bats` runs no such check.\n\napi-design.md § \"A value the server owns reaches the client from the server\" settles this. The writer reads the owner's document, or its suite checks agreement against it. It never keeps a hand-typed copy.\n\nThis PR's CLAUDE.md adds \"A key added to the schema gets its value in `tuitheme/default.json` in the same change\". That binds the repo's own default and says nothing of the generator.\n\n**Fix:** two parts, in theme and here.\n- Theme's generator suite validates every generated `tui-theme.json` against the published v1.json, pinned or fetched.\n- Theme derives `MARKDOWN_KEYS` and the `ui` key set from the schema's `properties`.\n- Here, the CLAUDE.md bullet names the generator as a second place a new key has to reach."
},
{
"number": "fleet-patterns.3",
"section": "design",
"sites": [
"tuitheme/default.json:36",
"tuitheme/default.json:21",
"gotermboard:markdown/code.go:107",
"gotermboard:markdown/render.go:164"
],
"reach": "gotermboard's markdown renderer is the fleet's default for any Go TUI that draws markdown (tui.md § \"A TUI that draws markdown starts from gotermboard's renderer\"). A recolor made in `tokenCode` alone gets no theme file. A recolor made in `default.json` alone leaves gotermboard's code blocks unchanged. Two copies now carry the 16-color table that tui.md says a theme file should replace. This is weak evidence: only one other copy exists, and today it agrees.",
"body": "**`default.json` restates gotermboard's 16-color syntax and heading table instead of becoming its source.**\n\nThe `syntax` group in `default.json` is gotermboard's `tokenCode` written out again, slot for slot:\n- comment: 8 italic\n- string: 10\n- number: 11\n- keyword: 13\n- type, attribute and function.builtin: 14\n- function and tag: 12\n\nThe gotermboard side uses its bright `Code*` constants, which are SGR 90–97. In the same way, `heading.1`/`.2`/`.3` at 12/12/14 bold copy `layout.heading`. The only deliberate divergence is `ui.match`. The latest commit makes it a foreground. gotermboard's Reader draws a match in reverse.\n\nThe PR body cites gotermboard for `ui.accent` but gives no plan for this table. Nothing makes one copy follow the other.\n\n**Fix:** make `tuitheme` the owner. gotermboard's `tokenCode` and `heading` read `theme.Style(\"syntax.<capture>\")` and `theme.Style(\"markdown.heading.N\")` from a `*tuitheme.Theme`, with `tuitheme.Default()` when no file is configured. That deletes the second table. Until that change lands, record the plan in the PR body. Without it, the next recolor reaches one table and misses the other."
}
]
}This review does not gate merge.
There was a problem hiding this comment.
PR review — G1
Used: I checked out PR 2 and ran go test ./... (pass), golangci-lint run ./tuitheme/... (0 issues) and gofmt -l (clean). I loaded all 20 tui-theme.json files from the companion PR datapointchris/theme#24 through Load (all load). I loaded 12 malformed or edge-case files from a scratch test. I ran the README's own example as a scratch module with XDG_CONFIG_HOME pointed at a directory of mine, against a malformed file and against a file in a future value form.
Standards: Cited: system-design.md § "A file format's first version is checked against the writers already planned for it before it ships", testing.md § "Test behavior the user cares about, not what this commit happened to change", and pull-requests.md § "Tests the diff adds are checked for whether they test the program or the change". Searches on a nil result returned with an error and on README examples that ignore an error found no rule.
Findings: 1 correctness · 2 design · 1 prose
Correctness
G1.1 tuitheme/theme.go:87 · tuitheme/theme.go:91 · tuitheme/theme.go:98 · tuitheme/theme.go:22 · README.md:225
Reach: Every reader's first Load at startup when current.json holds a value it cannot parse. That file can be hand-edited or written by a newer generator. The README and package-doc examples are the code a reader copies, and they reach the panic directly.
A theme file the reader cannot parse crashes the documented startup flow. Load returns nil, err on a read or parse error. Theme.Style then dereferences the nil *Theme. The README example and the package doc both read theme, err := tuitheme.Load(path) and then call theme.Code(...) straight away.
I ran that README snippet as a scratch module with XDG_CONFIG_HOME pointed at my own directory. I gave it a current.json of {"ui": {"selection": {"bg": "#2d4f6"}}}. It printed the error and then panic: runtime error: invalid memory address or nil pointer dereference. A file in a future value form, "border": {"fg":"#555555","underline":"curly"}, panicked the same way.
The docstring says "the caller keeps what it was drawing". At startup the caller has drawn nothing yet. Each reader on the theme PR's list (fleet, slate, psyche, projects) would have to write its own if err != nil { theme = tuitheme.Default() } branch. That is the same code written identically in several tools, which is exactly what this module exists to hold.
Fix: have Load return Default() along with the error on every failure path. Callers then always get a theme they can draw. On a reload a caller can still ignore the result and keep its old theme. Change the docstring to say that. Also make the README and doc examples check err, or say on the Load line that theme is usable even when err is set.
Design and architecture
G1.2 tuitheme/theme.go:157 · tuitheme/theme.go:81 · CLAUDE.md:49 · README.md:253
Reach: The planned v2 change. The new CLAUDE.md rule "The theme file format only grows" and the README both say a change older readers cannot read becomes schema/v2.json. The generator in datapointchris/theme#24 writes one current.json that every installed reader loads.
The schema version is invisible to readers, so v2 will break every reader built before it. Load never reads $schema, and DefaultPath names one unversioned file. parse fails the whole load on the first value it cannot read (theme.go:157). Once a generator writes a v2 value form into current.json, every reader built before v2 loses the entire theme. I ran a file with "underline":"curly" to check: the load error was ui.border: {...} is neither a color nor a style object, and none of the file's other roles were used.
CLAUDE.md says "Programs released long ago read files written today". The versioned schema URL only keeps editors working. Readers are what the rule promises, and nothing keeps them working. The cheap time to fix this is before a v1 file is installed anywhere, which system-design.md § "A file format's first version is checked against the writers already planned for it before it ships" asks for. Its Why ends with "An older reader still refuses a new enum member."
Fix: put the version where a reader looks for the file. For example, DefaultPath returns tui-theme/v1/current.json. A v2 generator then writes both versions, and older readers keep their colors. Alternatively, Load reads $schema and returns the default plus an error for a version it does not know. That keeps old readers running, but untinted.
G1.3 tuitheme/reload_unix_test.go:13
Reach: pull-requests.md § "Tests the diff adds are checked for whether they test the program or the change". Every later edit to reload_unix.go or Load has to read this test and keep it passing.
TestSIGUSR2AsksForTheFileAgain tests os/signal, not this package. Notify is one call to signal.Notify(c, syscall.SIGUSR2). The test sends SIGUSR2 to its own process and asserts the channel receives it. That is os/signal's documented behavior, one of the shapes testing.md § "Test behavior the user cares about, not what this commit happened to change" names.
The test's second half does not test a reload either. It writes the file once, before any load, and then loads it once. So nothing checks that a second Load picks up a changed file. TestRoleFallsBackAlongItsDotsThenToTheTerminalsSixteenColors already checks that Load reads a #rrggbb selection.
The behavior this package actually owns is the build split. Non-unix targets get a Notify that relays nothing. The PR checked that with GOOS builds, and this test does not cover it.
Fix: delete the test. If the build split needs a guard, add a CI step running GOOS=windows go vet ./tuitheme/.
Prose
G1.4 doc.go:31
An inserted sentence makes "for it" point at the wrong package. The new sentence went between cobrahelp and the next one, so the line now reads: "The subpackage tuitheme reads the theme file a terminal UI draws from, and imports only the standard library too. The module's go.mod names cobra for it". The nearest "it" is now tuitheme. go.mod names cobra for cobrahelp. The line also runs past the wrap width of the comment around it.
Rewrite: keep cobrahelp's two sentences together, then add tuitheme's after them, and rewrap:
"The subpackage cobrahelp is the cobra frontend, kept apart so cobra reaches only a tool that imports it. The module's go.mod names cobra for cobrahelp, so an importer's module graph lists cobra while its build compiles none of it. The subpackage tuitheme imports only the standard library."
Findings as JSON
{
"used": "I checked out PR 2 and ran `go test ./...` (pass), `golangci-lint run ./tuitheme/...` (0 issues) and `gofmt -l` (clean). I loaded all 20 `tui-theme.json` files from the companion PR datapointchris/theme#24 through `Load` (all load). I loaded 12 malformed or edge-case files from a scratch test. I ran the README's own example as a scratch module with `XDG_CONFIG_HOME` pointed at a directory of mine, against a malformed file and against a file in a future value form.",
"standards": "Cited: `system-design.md` § \"A file format's first version is checked against the writers already planned for it before it ships\", `testing.md` § \"Test behavior the user cares about, not what this commit happened to change\", and `pull-requests.md` § \"Tests the diff adds are checked for whether they test the program or the change\". Searches on a nil result returned with an error and on README examples that ignore an error found no rule.",
"findings": [
{
"number": "G1.1",
"section": "correctness",
"sites": [
"tuitheme/theme.go:87",
"tuitheme/theme.go:91",
"tuitheme/theme.go:98",
"tuitheme/theme.go:22",
"README.md:225"
],
"reach": "Every reader's first `Load` at startup when `current.json` holds a value it cannot parse. That file can be hand-edited or written by a newer generator. The README and package-doc examples are the code a reader copies, and they reach the panic directly.",
"body": "**A theme file the reader cannot parse crashes the documented startup flow.** `Load` returns `nil, err` on a read or parse error. `Theme.Style` then dereferences the nil `*Theme`. The README example and the package doc both read `theme, err := tuitheme.Load(path)` and then call `theme.Code(...)` straight away.\n\nI ran that README snippet as a scratch module with `XDG_CONFIG_HOME` pointed at my own directory. I gave it a `current.json` of `{\"ui\": {\"selection\": {\"bg\": \"#2d4f6\"}}}`. It printed the error and then `panic: runtime error: invalid memory address or nil pointer dereference`. A file in a future value form, `\"border\": {\"fg\":\"#555555\",\"underline\":\"curly\"}`, panicked the same way.\n\nThe docstring says \"the caller keeps what it was drawing\". At startup the caller has drawn nothing yet. Each reader on the theme PR's list (`fleet`, `slate`, `psyche`, `projects`) would have to write its own `if err != nil { theme = tuitheme.Default() }` branch. That is the same code written identically in several tools, which is exactly what this module exists to hold.\n\nFix: have `Load` return `Default()` along with the error on every failure path. Callers then always get a theme they can draw. On a reload a caller can still ignore the result and keep its old theme. Change the docstring to say that. Also make the README and doc examples check `err`, or say on the `Load` line that `theme` is usable even when `err` is set."
},
{
"number": "G1.2",
"section": "design",
"sites": [
"tuitheme/theme.go:157",
"tuitheme/theme.go:81",
"CLAUDE.md:49",
"README.md:253"
],
"reach": "The planned v2 change. The new `CLAUDE.md` rule \"The theme file format only grows\" and the README both say a change older readers cannot read becomes `schema/v2.json`. The generator in datapointchris/theme#24 writes one `current.json` that every installed reader loads.",
"body": "**The schema version is invisible to readers, so v2 will break every reader built before it.** `Load` never reads `$schema`, and `DefaultPath` names one unversioned file. `parse` fails the whole load on the first value it cannot read (`theme.go:157`). Once a generator writes a v2 value form into `current.json`, every reader built before v2 loses the entire theme. I ran a file with `\"underline\":\"curly\"` to check: the load error was `ui.border: {...} is neither a color nor a style object`, and none of the file's other roles were used.\n\n`CLAUDE.md` says \"Programs released long ago read files written today\". The versioned schema URL only keeps editors working. Readers are what the rule promises, and nothing keeps them working. The cheap time to fix this is before a v1 file is installed anywhere, which `system-design.md` § \"A file format's first version is checked against the writers already planned for it before it ships\" asks for. Its Why ends with \"An older reader still refuses a new enum member.\"\n\nFix: put the version where a reader looks for the file. For example, `DefaultPath` returns `tui-theme/v1/current.json`. A v2 generator then writes both versions, and older readers keep their colors. Alternatively, `Load` reads `$schema` and returns the default plus an error for a version it does not know. That keeps old readers running, but untinted."
},
{
"number": "G1.3",
"section": "design",
"sites": [
"tuitheme/reload_unix_test.go:13"
],
"reach": "`pull-requests.md` § \"Tests the diff adds are checked for whether they test the program or the change\". Every later edit to `reload_unix.go` or `Load` has to read this test and keep it passing.",
"body": "**`TestSIGUSR2AsksForTheFileAgain` tests `os/signal`, not this package.** `Notify` is one call to `signal.Notify(c, syscall.SIGUSR2)`. The test sends SIGUSR2 to its own process and asserts the channel receives it. That is `os/signal`'s documented behavior, one of the shapes `testing.md` § \"Test behavior the user cares about, not what this commit happened to change\" names.\n\nThe test's second half does not test a reload either. It writes the file once, before any load, and then loads it once. So nothing checks that a second `Load` picks up a changed file. `TestRoleFallsBackAlongItsDotsThenToTheTerminalsSixteenColors` already checks that `Load` reads a `#rrggbb` selection.\n\nThe behavior this package actually owns is the build split. Non-unix targets get a `Notify` that relays nothing. The PR checked that with `GOOS` builds, and this test does not cover it.\n\nFix: delete the test. If the build split needs a guard, add a CI step running `GOOS=windows go vet ./tuitheme/`."
},
{
"number": "G1.4",
"section": "prose",
"sites": [
"doc.go:31"
],
"reach": "",
"body": "**An inserted sentence makes \"for it\" point at the wrong package.** The new sentence went between cobrahelp and the next one, so the line now reads: \"The subpackage tuitheme reads the theme file a terminal UI draws from, and imports only the standard library too. The module's go.mod names cobra for it\". The nearest \"it\" is now tuitheme. go.mod names cobra for cobrahelp. The line also runs past the wrap width of the comment around it.\n\nRewrite: keep cobrahelp's two sentences together, then add tuitheme's after them, and rewrap:\n\n\"The subpackage cobrahelp is the cobra frontend, kept apart so cobra reaches only a tool that imports it. The module's go.mod names cobra for cobrahelp, so an importer's module graph lists cobra while its build compiles none of it. The subpackage tuitheme imports only the standard library.\""
}
]
}This review does not gate merge.
There was a problem hiding this comment.
PR review — synthesis
Used: the 14 findings of round 1, from 8 review(s), and nothing else
Standards: none, because this step reads only the findings
Findings: 0 correctness · 2 design · 0 prose
Design and architecture
synthesis.1 explains code-architecture.1 · fleet-patterns.2 · fleet-patterns.3
The format's vocabulary (groups, roles, color grammar) is held in several copies: the schema, default.json, theme.go and style.go here, and the generator in the theme repo. Only prose keeps them level, and no running check does. Findings: code-architecture.1 for this repo's three files, fleet-patterns.2 for the generator copy, fleet-patterns.3 for the gotermboard table.
synthesis.2 explains fleet-patterns.1 · contracts.1 · failure-paths.2
DefaultPath compiles in the path of a file the reader does not own. It has no env or config layer and no error on an unresolvable home. Readers get a silent default or a path they cannot override.
One defect, raised more than once
G1.4 · stale-prose.1: The doc.go comment has the new tuitheme sentence inserted before the go.mod sentence. "For it" now points at tuitheme instead of cobrahelp, and the line is not rewrapped.
failure-paths.1 · contracts.2: parseValue in theme.go accepts null and "" as a valid empty style. This masks the default and contradicts the documented failure of the load.
G1.3 · useless-test.1: The Load-and-Code half of TestSIGUSR2AsksForTheFileAgain in reload_unix_test.go does not test reloading.
Shapes as JSON
{
"shapes": [
{
"number": "synthesis.1",
"section": "design",
"explains": [
"code-architecture.1",
"fleet-patterns.2",
"fleet-patterns.3"
],
"body": "The format's vocabulary (groups, roles, color grammar) is held in several copies: the schema, default.json, theme.go and style.go here, and the generator in the theme repo. Only prose keeps them level, and no running check does. Findings: code-architecture.1 for this repo's three files, fleet-patterns.2 for the generator copy, fleet-patterns.3 for the gotermboard table."
},
{
"number": "synthesis.2",
"section": "design",
"explains": [
"fleet-patterns.1",
"contracts.1",
"failure-paths.2"
],
"body": "DefaultPath compiles in the path of a file the reader does not own. It has no env or config layer and no error on an unresolvable home. Readers get a silent default or a path they cannot override."
}
],
"duplicates": [
{
"defect": "The doc.go comment has the new tuitheme sentence inserted before the go.mod sentence. \"For it\" now points at tuitheme instead of cobrahelp, and the line is not rewrapped.",
"findings": [
"G1.4",
"stale-prose.1"
]
},
{
"defect": "parseValue in theme.go accepts null and \"\" as a valid empty style. This masks the default and contradicts the documented failure of the load.",
"findings": [
"failure-paths.1",
"contracts.2"
]
},
{
"defect": "The Load-and-Code half of TestSIGUSR2AsksForTheFileAgain in reload_unix_test.go does not test reloading.",
"findings": [
"G1.3",
"useless-test.1"
]
}
]
}This review does not gate merge.
PR review round 1Ran: G1, useless-test, code-architecture, fleet-patterns, system-design, stale-prose, failure-paths, contracts, synthesis
Round record{
"grammar": 6,
"round": 1,
"repo": "datapointchris/gotermstyle",
"pr": 2,
"head": "8eef34cde8a0b89eee974bfac135fe6764fa8214",
"experiment": "none",
"dispatch": "20261108T061926Z-pr-review",
"roster": "2325da4b952f7856fff4ad1cf53e6b09ef9c6940",
"harness": "5960178486928371110328f9ccf459b298d7237c",
"machine": "queue",
"started": "2026-11-08T06:20:37Z",
"finished": "2026-11-08T06:29:06Z",
"reviewers": [
{
"label": "G1",
"agent": "pr-reviewer",
"model": "opus",
"effort": "high",
"experiment": "none",
"arm": "default",
"reason": "STATIC",
"prompt_profile": "pr-reviewer-prompts/general.md",
"prompt_sha": "a0f85e3b82b4",
"status": "ran",
"why": "",
"attempts": 1,
"seconds": 312,
"tokens": {
"input": 50,
"cache_write": 62046,
"cache_read": 1169191,
"output": 18430
},
"tokens_from": "result",
"tokens_by_model": {
"claude-opus-5-5": {
"input": 50,
"cache_write": 62046,
"cache_read": 1169191,
"output": 18430
}
},
"session": "01a1b9d2-2de8-75a6-b936-5b5f5fbfac20",
"worklog": "",
"worklog_refused": "error: API request failed (503 Service Unavailable)",
"claude_code": "2.1.319",
"review_url": "https://github.com/datapointchris/gotermstyle/pull/2#pullrequestreview-5816900486",
"review_id": 5816900486,
"findings": {
"correctness": 1,
"design": 2,
"prose": 1
},
"numbers": [
"G1.1",
"G1.2",
"G1.3",
"G1.4"
],
"rules": {
"proposed": 2,
"keys": [
"rule/code-quality/a-loader-for-an-optional-file-returns-a-usable-value-beside-its-error",
"rule/system-design/a-formats-version-lives-where-its-readers-select-on-it-not-only-in-an-editors-schema-url"
],
"filed": true,
"why": "",
"path": "/home/fleet/.cache/run-pr-reviewers/datapointchris-gotermstyle-pr2-round1-20261108T061926Z/records/G1.rules.json"
},
"head": "8eef34cde8a0b89eee974bfac135fe6764fa8214",
"kept": false,
"records": "/home/fleet/.cache/run-pr-reviewers/datapointchris-gotermstyle-pr2-round1-20261108T061926Z/records",
"started": "2026-11-08T06:22:43Z",
"finished": "2026-11-08T06:28:11Z",
"account": "fleetizen",
"api_key_source": "none",
"machine": "queue"
},
{
"label": "useless-test",
"agent": "pr-useless-test-reviewer",
"model": "opus",
"effort": "high",
"experiment": "none",
"arm": "default",
"reason": "STATIC",
"prompt_profile": "pr-reviewer-prompts/useless-test.md",
"prompt_sha": "3700d72236e3",
"status": "ran",
"why": "",
"attempts": 1,
"seconds": 107,
"tokens": {
"input": 10,
"cache_write": 19860,
"cache_read": 85915,
"output": 3738
},
"tokens_from": "result",
"tokens_by_model": {
"claude-opus-5-5": {
"input": 10,
"cache_write": 19860,
"cache_read": 85915,
"output": 3738
}
},
"session": "01a1b9d1-55d8-7f49-affa-ef0bf4d7b47b",
"worklog": "2555.1",
"worklog_refused": "",
"claude_code": "2.1.319",
"review_url": "https://github.com/datapointchris/gotermstyle/pull/2#pullrequestreview-5816889667",
"review_id": 5816889667,
"findings": {
"correctness": 0,
"design": 1,
"prose": 0
},
"numbers": [
"useless-test.1"
],
"rules": {},
"head": "8eef34cde8a0b89eee974bfac135fe6764fa8214",
"kept": false,
"records": "/home/fleet/.cache/run-pr-reviewers/datapointchris-gotermstyle-pr2-round1-20261108T061926Z/records",
"started": "2026-11-08T06:21:33Z",
"finished": "2026-11-08T06:23:30Z",
"account": "fleetizen",
"api_key_source": "none",
"machine": "queue"
},
{
"label": "code-architecture",
"agent": "pr-code-architecture-reviewer",
"model": "opus",
"effort": "medium",
"experiment": "none",
"arm": "default",
"reason": "STATIC",
"prompt_profile": "pr-reviewer-prompts/persona.md",
"prompt_sha": "d9be86eb4324",
"status": "ran",
"why": "",
"attempts": 1,
"seconds": 116,
"tokens": {
"input": 20,
"cache_write": 28941,
"cache_read": 253606,
"output": 5453
},
"tokens_from": "result",
"tokens_by_model": {
"claude-opus-5-5": {
"input": 20,
"cache_write": 28941,
"cache_read": 253606,
"output": 5453
}
},
"session": "01a1b9d2-3193-7969-a178-d3122a1c3e1c",
"worklog": "",
"worklog_refused": "error: API request failed (503 Service Unavailable)",
"claude_code": "2.1.319",
"review_url": "https://github.com/datapointchris/gotermstyle/pull/2#pullrequestreview-5816893090",
"review_id": 5816893090,
"findings": {
"correctness": 0,
"design": 1,
"prose": 0
},
"numbers": [
"code-architecture.1"
],
"rules": {},
"head": "8eef34cde8a0b89eee974bfac135fe6764fa8214",
"kept": false,
"records": "/home/fleet/.cache/run-pr-reviewers/datapointchris-gotermstyle-pr2-round1-20261108T061926Z/records",
"started": "2026-11-08T06:22:44Z",
"finished": "2026-11-08T06:25:00Z",
"account": "fleetizen",
"api_key_source": "none",
"machine": "queue"
},
{
"label": "fleet-patterns",
"agent": "pr-fleet-patterns-reviewer",
"model": "opus",
"effort": "medium",
"experiment": "none",
"arm": "default",
"reason": "STATIC",
"prompt_profile": "pr-reviewer-prompts/persona.md",
"prompt_sha": "eae343d1d8d5",
"status": "ran",
"why": "",
"attempts": 1,
"seconds": 258,
"tokens": {
"input": 44,
"cache_write": 59598,
"cache_read": 971511,
"output": 12342
},
"tokens_from": "result",
"tokens_by_model": {
"claude-opus-5-5": {
"input": 44,
"cache_write": 59598,
"cache_read": 971511,
"output": 12342
}
},
"session": "01a1b9d0-653a-72ed-ba33-ed3eac71508c",
"worklog": "2555.1",
"worklog_refused": "",
"claude_code": "2.1.319",
"review_url": "https://github.com/datapointchris/gotermstyle/pull/2#pullrequestreview-5816893529",
"review_id": 5816893529,
"findings": {
"correctness": 0,
"design": 3,
"prose": 0
},
"numbers": [
"fleet-patterns.1",
"fleet-patterns.2",
"fleet-patterns.3"
],
"rules": {},
"head": "8eef34cde8a0b89eee974bfac135fe6764fa8214",
"kept": false,
"records": "/home/fleet/.cache/run-pr-reviewers/datapointchris-gotermstyle-pr2-round1-20261108T061926Z/records",
"started": "2026-11-08T06:20:37Z",
"finished": "2026-11-08T06:25:11Z",
"account": "fleetizen",
"api_key_source": "none",
"machine": "queue"
},
{
"label": "system-design",
"agent": "pr-system-design-reviewer",
"model": "sonnet",
"effort": "medium",
"experiment": "none",
"arm": "default",
"reason": "STATIC",
"prompt_profile": "pr-reviewer-prompts/persona.md",
"prompt_sha": "c284e39c894a",
"status": "ran",
"why": "",
"attempts": 1,
"seconds": 54,
"tokens": {
"input": 12,
"cache_write": 11698,
"cache_read": 87421,
"output": 1949
},
"tokens_from": "result",
"tokens_by_model": {
"claude-sonnet-5-5": {
"input": 12,
"cache_write": 11698,
"cache_read": 87421,
"output": 1949
}
},
"session": "01a1b9d0-7446-7210-bdfd-fff235185c56",
"worklog": "2555.1",
"worklog_refused": "",
"claude_code": "2.1.319",
"review_url": "https://github.com/datapointchris/gotermstyle/pull/2#pullrequestreview-5816885187",
"review_id": 5816885187,
"findings": {
"correctness": 0,
"design": 0,
"prose": 0
},
"numbers": [],
"rules": {},
"head": "8eef34cde8a0b89eee974bfac135fe6764fa8214",
"kept": false,
"records": "/home/fleet/.cache/run-pr-reviewers/datapointchris-gotermstyle-pr2-round1-20261108T061926Z/records",
"started": "2026-11-08T06:20:37Z",
"finished": "2026-11-08T06:21:37Z",
"account": "fleetizen",
"api_key_source": "none",
"machine": "queue"
},
{
"label": "stale-prose",
"agent": "pr-stale-prose-reviewer",
"model": "opus",
"effort": "medium",
"experiment": "none",
"arm": "default",
"reason": "STATIC",
"prompt_profile": "pr-reviewer-prompts/persona.md",
"prompt_sha": "50506a05f936",
"status": "ran",
"why": "",
"attempts": 1,
"seconds": 100,
"tokens": {
"input": 14,
"cache_write": 21792,
"cache_read": 126806,
"output": 4025
},
"tokens_from": "result",
"tokens_by_model": {
"claude-opus-5-5": {
"input": 14,
"cache_write": 21792,
"cache_read": 126806,
"output": 4025
}
},
"session": "01a1b9d0-830d-7b7f-97f6-d81871ecba11",
"worklog": "2555.1",
"worklog_refused": "",
"claude_code": "2.1.319",
"review_url": "https://github.com/datapointchris/gotermstyle/pull/2#pullrequestreview-5816887511",
"review_id": 5816887511,
"findings": {
"correctness": 0,
"design": 0,
"prose": 1
},
"numbers": [
"stale-prose.1"
],
"rules": {},
"head": "8eef34cde8a0b89eee974bfac135fe6764fa8214",
"kept": false,
"records": "/home/fleet/.cache/run-pr-reviewers/datapointchris-gotermstyle-pr2-round1-20261108T061926Z/records",
"started": "2026-11-08T06:20:43Z",
"finished": "2026-11-08T06:22:37Z",
"account": "fleetizen",
"api_key_source": "none",
"machine": "queue"
},
{
"label": "failure-paths",
"agent": "pr-failure-paths-reviewer",
"model": "sonnet",
"effort": "medium",
"experiment": "none",
"arm": "default",
"reason": "STATIC",
"prompt_profile": "pr-reviewer-prompts/persona.md",
"prompt_sha": "56bdd640764b",
"status": "ran",
"why": "",
"attempts": 1,
"seconds": 78,
"tokens": {
"input": 12,
"cache_write": 17594,
"cache_read": 106195,
"output": 3925
},
"tokens_from": "result",
"tokens_by_model": {
"claude-sonnet-5-5": {
"input": 12,
"cache_write": 17594,
"cache_read": 106195,
"output": 3925
}
},
"session": "01a1b9d0-699c-7824-a288-c088eed23a3a",
"worklog": "2555.1",
"worklog_refused": "",
"claude_code": "2.1.319",
"review_url": "https://github.com/datapointchris/gotermstyle/pull/2#pullrequestreview-5816886212",
"review_id": 5816886212,
"findings": {
"correctness": 2,
"design": 0,
"prose": 0
},
"numbers": [
"failure-paths.1",
"failure-paths.2"
],
"rules": {},
"head": "8eef34cde8a0b89eee974bfac135fe6764fa8214",
"kept": false,
"records": "/home/fleet/.cache/run-pr-reviewers/datapointchris-gotermstyle-pr2-round1-20261108T061926Z/records",
"started": "2026-11-08T06:20:37Z",
"finished": "2026-11-08T06:22:03Z",
"account": "fleetizen",
"api_key_source": "none",
"machine": "queue"
},
{
"label": "contracts",
"agent": "pr-contracts-reviewer",
"model": "opus",
"effort": "medium",
"experiment": "none",
"arm": "default",
"reason": "STATIC",
"prompt_profile": "pr-reviewer-prompts/persona.md",
"prompt_sha": "88b65626bceb",
"status": "ran",
"why": "",
"attempts": 1,
"seconds": 207,
"tokens": {
"input": 28,
"cache_write": 31387,
"cache_read": 347494,
"output": 9495
},
"tokens_from": "result",
"tokens_by_model": {
"claude-opus-5-5": {
"input": 28,
"cache_write": 31387,
"cache_read": 347494,
"output": 9495
}
},
"session": "01a1b9d0-7c47-7cc4-a07f-8404cdefeaca",
"worklog": "2555.1",
"worklog_refused": "",
"claude_code": "2.1.319",
"review_url": "https://github.com/datapointchris/gotermstyle/pull/2#pullrequestreview-5816892017",
"review_id": 5816892017,
"findings": {
"correctness": 0,
"design": 2,
"prose": 0
},
"numbers": [
"contracts.1",
"contracts.2"
],
"rules": {},
"head": "8eef34cde8a0b89eee974bfac135fe6764fa8214",
"kept": false,
"records": "/home/fleet/.cache/run-pr-reviewers/datapointchris-gotermstyle-pr2-round1-20261108T061926Z/records",
"started": "2026-11-08T06:20:46Z",
"finished": "2026-11-08T06:24:31Z",
"account": "fleetizen",
"api_key_source": "none",
"machine": "queue"
},
{
"label": "synthesis",
"agent": null,
"model": "sonnet",
"effort": "medium",
"experiment": "none",
"arm": "default",
"reason": "STATIC",
"prompt_profile": "pr-reviewer-prompts/synthesis.md",
"prompt_sha": "40dcb5a50df0",
"status": "ran",
"why": "",
"attempts": 1,
"seconds": 7,
"tokens": {
"input": 2,
"cache_write": 10412,
"cache_read": 1433,
"output": 580
},
"tokens_from": "result",
"tokens_by_model": {
"claude-sonnet-5-5": {
"input": 2,
"cache_write": 10412,
"cache_read": 1433,
"output": 580
}
},
"session": "01a1b9d8-2fb4-779b-976b-89604f6d7ca4",
"worklog": "2555.1",
"worklog_refused": "",
"claude_code": "2.1.319",
"review_url": "https://github.com/datapointchris/gotermstyle/pull/2#pullrequestreview-5816902683",
"review_id": 5816902683,
"findings": {
"correctness": 0,
"design": 2,
"prose": 0
},
"numbers": [
"synthesis.1",
"synthesis.2"
],
"rules": {},
"head": "8eef34cde8a0b89eee974bfac135fe6764fa8214",
"kept": false,
"records": "/home/fleet/.cache/run-pr-reviewers/datapointchris-gotermstyle-pr2-round1-20261108T061926Z/records",
"started": "2026-11-08T06:28:54Z",
"finished": "2026-11-08T06:29:06Z",
"account": "fleetizen",
"api_key_source": "none",
"machine": "queue"
}
],
"rules": {
"proposed": 2,
"keys": [
"rule/code-quality/a-loader-for-an-optional-file-returns-a-usable-value-beside-its-error",
"rule/system-design/a-formats-version-lives-where-its-readers-select-on-it-not-only-in-an-editors-schema-url"
],
"unfiled": []
},
"kept": []
} |
The tuitheme sentence sat between cobrahelp and the go.mod sentence, so "names cobra for it" read as naming cobra for tuitheme, which imports the standard library alone. Agent-Name: claude-code-unattended Agent-Session: 4773265d-bd77-424c-b9db-836de857b797 Agent-Run: 01a1b9cf-f0a1-76ce-a000-cdf27a041ee5 Worklog: 2555.1
Its second half loaded a file written before the signal, so it passed whether or not a reload worked, and the fallback table already pins the same rgb background. The relay half stays: changing Notify to SIGHUP fails it, and a theme generator signals every reader with SIGUSR2. Agent-Name: claude-code-unattended Agent-Session: 4773265d-bd77-424c-b9db-836de857b797 Agent-Run: 01a1b9cf-f0a1-76ce-a000-cdf27a041ee5 Worklog: 2555.1
…ery error A role written as null or "" loaded as a style that sets nothing. It hid the role's fallback and the built-in default, so a generator with no color for the selection left the selected row unmarked and reported success. Both now fail the load and name the key, as the schema does. An fg or bg the object leaves out is still unset. Load returned nil beside an error, and the documented startup flow then dereferenced it. It now returns the built-in default beside every error, so a program starting up draws the terminal's 16 colors and reports the error, while a program reloading keeps the theme it has. An empty path is an error rather than a missing file. Agent-Name: claude-code-unattended Agent-Session: 4773265d-bd77-424c-b9db-836de857b797 Agent-Run: 01a1b9cf-f0a1-76ce-a000-cdf27a041ee5 Worklog: 2555.1
DefaultPath compiled in one location and offered no way to name another, so a theme under test or a second theme location would have grown a flag or variable of its own in each reader. TUI_THEME_FILE now wins over the XDG path in every reader at once. DefaultPath also returned "" when it could not resolve a home directory, which Load read as a missing file and drew the default with no error. It now returns (string, error) and names what it could not resolve. The README and package examples check both errors and show a reload that keeps the theme on screen when the new file fails to load. Agent-Name: claude-code-unattended Agent-Session: 4773265d-bd77-424c-b9db-836de857b797 Agent-Run: 01a1b9cf-f0a1-76ce-a000-cdf27a041ee5 Worklog: 2555.1
…cabulary The groups, roles, style fields and color grammar live in the schema, default.json, theme.go and style.go, and only a CLAUDE.md sentence kept them level. A new test reads schema/v1.json and fails when a group the schema declares is not read, a role it declares has no default along its dots, default.json sets a key the schema refuses, the style fields differ, or parseColor and the color pattern disagree on a probe. The format's growth rule now keeps readers working, not only editors. A reader fails the whole load on a value it cannot read, so a new value form goes in a new key or style field. A change that cannot be added that way is a new file beside current.json, which a generator writes alongside the old one. Agent-Name: claude-code-unattended Agent-Session: 4773265d-bd77-424c-b9db-836de857b797 Agent-Run: 01a1b9cf-f0a1-76ce-a000-cdf27a041ee5 Worklog: 2555.1
A TUI drawn in the 16 terminal colors has no shade near the background to mark a selected row with, so it reverses the row into a slab of the foreground color. On a dark theme that is a bright block over the one row the reader is looking at. This adds
tuitheme, which reads a theme file holding the theme's own selection, panel, accent, markdown and syntax colors. A theme generator writes one file per theme, and every program on this module reads the same one. A program with no file draws exactly the terminal's 16 colors, as it does today.Written by Claude Code session 4773265d-bd77-424c-b9db-836de857b797.
What to look at
tuitheme/theme.go—lookupis the fallback rule: the file's key, then the key with its last dotted part dropped, stopping at the group, then the same chain in the built-in default. Check thatmarkdown.heading.2never reaches a baremarkdown.tuitheme/style.go—Style.Code. Slots 0–7 map to SGR 30–37 and 40–47, slots 8–15 to 90–97 and 100–107, and#rrggbbto38;2and48;2. Every code opens with0;, asCodeCyanand its siblings do, so gotermboard'semboldened(code)keeps working on a theme code.tuitheme/default.json— the built-in values.ui.selectionis slot 0 on slot 7, andui.accentis slot 11, the yellow gotermboard's current tab draws in today.tuitheme/schema/v1.json— the published format.ui,markdownand the style object reject unknown keys, so an editor flags a typo.syntaxaccepts any dotted capture.tuitheme/reload_unix.goandreload_other.go— theunixbuild constraint. Windows, plan9, js and wasip1 get aNotifythat relays nothing.How it was verified
go test ./...passes. Thetuithemetests:TestRoleFallsBackAlongItsDotsThenToTheTerminalsSixteenColorsTestAGeneratorsNullOrEmptyColorFailsTheLoadAndLeavesTheDefaultToDrawTestTheSchemaAndTheReaderAgreeOnEveryGroupRoleAndColor, which readsschema/v1.jsonTestSIGUSR2AsksForTheFileAgain, which sends SIGUSR2 to its own process and fails ifNotifyrelays nothing within 5s""read as unset again.ui.cursoradded to the schema alone.parseColortaking16.GOOS/GOARCHbuilds of./tuitheme/pass for windows, plan9, js/wasm, wasip1/wasm, aix, solaris, illumos, darwin, freebsd and android.check-jsonschema --check-metaschemapasses on the schema, andcheck-jsonschema --schemafile tuitheme/schema/v1.jsonpasses ondefault.json.tuitheme: t.json: ui.selection: bg: "#zzzzzz" is neither #rrggbb nor a slot from 0 to 15. With no home directory,DefaultPathreturnstuitheme: no theme file: TUI_THEME_FILE is unset and $HOME is not defined.What changes
github.com/datapointchris/gotermstyle/tuitheme. Its API isLoad,Default,DefaultPath,PathEnv,Theme.Style,Theme.Code,Style.Code,Color.String,Color.Slot,Color.RGB,NotifyandStop.DefaultPath() (string, error)is$TUI_THEME_FILEwhen set. Otherwise it is$XDG_CONFIG_HOME/tui-theme/current.json, or~/.config/tui-theme/current.jsonwhen that variable is unset or relative. It errors when it cannot resolve a home directory.Loadalways returns a theme to draw. A missing file loads the built-in default with no error. Every other failure returns the built-in default beside the error: an unreadable file, a value of neither form,null,"", or an empty path.https://raw.githubusercontent.com/datapointchris/gotermstyle/main/tuitheme/schema/v1.json. It resolves once this merges.dimis drawn when a theme file sets it. The root package's palette still never draws dim, anddefault.jsonnever sets it.tuithemeimports only the standard library.Decisions, and what they rejected
tuitheme.Loadandtheme.Code.selectionwith onlybgdraws the terminal's own foreground on it, rather than mixing in the default'sfg. The format specifies a missing key falling back, and says nothing of a missing field.Notifytakes achan<- os.Signal, assignal.Notifydoes. A program can share one channel with its other signals. The library's part is the platform split, so no program needs build-constrained files of its own.schema/v2.jsonbeside this one, as Zed versions its theme schema URL. Its file sits besidecurrent.json, and a generator writes both, so a v1 reader never meets a v2 value. Rejected: a versioned directory such astui-theme/v1/current.json, which moves every reader's path before any v2 exists.Loadreturns the built-in default beside an error, never nil. A program starting up draws the terminal's 16 colors and reports the error, and a program reloading ignores the result. Rejected: nil beside the error, which made every reader write the same fallback branch.TUI_THEME_FILEis the one override, read insideDefaultPath. It points every reader at another file at once, such as a theme under test. Rejected: a config key in each reader, because the format exists so no reader's config needs a line for its theme.LoadandCodewould serve the first caller.Style,Color.SlotandColor.RGBare there because a syntax highlighter needs the slot or the RGB to build its own style, andCodecannot express that.What this does not do
#rrggbbvalue is emitted as 24-bit, and a file written in slots is the form for a terminal that cannot.reverseattribute, because the format has none. So a program with no theme file draws its selection in slot 0 on slot 7 rather than in reverse video.The review
Round 1: #2 (comment)
failure-paths.1: fixed — 2985c02. A role written as
nullor"", or anfgorbgof"", fails the load and names the key.TestAGeneratorsNullOrEmptyColorFailsTheLoadAndLeavesTheDefaultToDrawholds it.contracts.2: fixed — same as failure-paths.1.
fgandbgdecode as*string, so one the object leaves out is still unset, and the reader now refuses what the schema refuses.failure-paths.2: fixed — 81e571a.
DefaultPathreturns(string, error)and names what it could not resolve.Load("")is an error rather than a missing file (2985c02).contracts.1: fixed — 81e571a.
DefaultPathreadsTUI_THEME_FILEbefore the XDG path, and the package doc and README name it. It is exported asPathEnv, so a reader's help can print it.fleet-patterns.1: rejected — the shared default path is the design the owner settled for this file. A theme generator installs one file, and every reader finds it with no config line. A config key per reader would bring back the per-program setup the format replaces. Both gaps the finding names are fixed.
TUI_THEME_FILErepoints every reader at once (81e571a). theme's install now ignores a relativeXDG_CONFIG_HOMEas the reader does (datapointchris/theme@716e007).G1.1: fixed — 2985c02.
Loadreturns the built-in default beside every error, so the README flow draws the 16 colors and reports the error rather than panicking. The README and package examples check both errors, and their reload loop keeps the theme on screen when a reload fails (81e571a).G1.2: fixed — c2ffec6. The growth rule now keeps readers working, not only editors. A new value form goes in a new key or style field, which an older reader ignores. A change no adding can express is a new file beside
current.json, and a generator writes it alongside the v1 file. CLAUDE.md and the README state the rule.G1.3: rejected — the relay half stays. useless-test.1 changed
Notifyto SIGHUP and this test failed, and no other test catches that. A theme generator signals every reader with SIGUSR2. The Load tail is gone, same as useless-test.1. The non-unix build is compiled by the release workflow's windows leg before every tag.useless-test.1: fixed — 213f78c. The
Loadtail, itswriteThemecall andpathare gone, and the relay half stays.G1.4: fixed — fff61b7. cobrahelp's two sentences sit together, then tuitheme's, rewrapped.
stale-prose.1: fixed — same as G1.4.
code-architecture.1: fixed — c2ffec6.
TestTheSchemaAndTheReaderAgreeOnEveryGroupRoleAndColorreadsschema/v1.json. It fails when:default.jsonsets a key the schema refuses;parseColorand the color pattern disagree on a probe.The
check-jsonschemahook was not added. The hooks file is generated, so a hand-added hook does not survive its next regeneration, and the Go test holds the same agreements on every run.fleet-patterns.2: fixed — datapointchris/theme@65de5a7. theme's suite fetches the schema at each generated file's
$schemaURL. It fails when a file'suiormarkdownkeys differ from the schema's, or a basesyntaxrole is missing. It skips when the URL is unreachable, which it is until this merges. A schema withui.cursoradded fails it. CLAUDE.md names the generator as a second place a new key lands (c2ffec6). The generator still types its keys out, so generating a theme needs no network.fleet-patterns.3: deferred — gotermboard pins gotermstyle v0.1.0, so it cannot read
tuithemeuntil a tag carries it. ThentokenCodeand the heading styles readsyntax.<capture>andmarkdown.heading.Nfrom a*tuitheme.Theme, withDefault()when none is loaded, and gotermboard's own table goes. That is filed as gotermboard work.synthesis.1: fixed — same as code-architecture.1 for this repo's copies, and fleet-patterns.2 for the generator's. gotermboard's table is deferred, as fleet-patterns.3 says.
synthesis.2: fixed — same as contracts.1 and failure-paths.2. The compiled-in default stays, for the reason fleet-patterns.1 gives.