feat(review): git-native config, keybindings, help, and base16 themes

Makefile

@@ -1,4 +1,4 @@
-.PHONY: install install-dev install-man install-hooks build test fmt clippy
+.PHONY: install install-dev install-man install-hooks build test smoke fmt clippy
 
 PREFIX ?= /usr/local
 
@@ -25,6 +25,12 @@ build:
 test:
 	cargo test --workspace
 
+# PTY smoke tests (ignored by default: wall-clock-bound and load-sensitive).
+# Spawns the review binary under a pseudo-terminal and plays the terminal's
+# side of the theme=auto probe conversation; see tests/pty_smoke.rs.
+smoke:
+	cargo test -p git-workon-review --test pty_smoke -- --ignored
+
 fmt:
 	cargo fmt
 

docs/adr/034-review-git-native-config-schema.md

@@ -0,0 +1,96 @@
+# 034 — Review TUI Config: Git-Native Per-View Namespaces
+
+## Context
+
+The review TUI (`git-workon-review`) grew its keybindings and colors as hardcoded
+values during M3–M5: a `match` in `tui.rs` for keys, a block of `const … Color::Rgb(…)`
+atop `render.rs` for theming. Making either user-configurable needs a config home, and
+the review binary reads no config today (`struct Cli {}` is empty).
+
+[ADR-006](006-git-native-config.md) already commits the tool to git-native config under
+the `workon.*` namespace — no bespoke file format, git's layered precedence
+(local → global → system), multivar for lists. The open question was whether a *keymap*
+fits that model, since a keymap is many key→action entries. Three shapes were considered:
+
+1. A dedicated `review.toml` (nested keymap syntax, in-tree shareable) — but a second
+   config system, against ADR-006's one-config-system principle, needs a new loader and
+   precedence layer.
+2. Value-side multivar `workon.review.bind = "key=action"` — git-native, but multivar
+   *accumulates* across layers, forcing us to reimplement override precedence and
+   last-wins dedup by hand and invent an unbind sentinel.
+3. Action-as-key, per-view namespaces (chosen).
+
+The keymap is also context-dependent: the same key differs by view (`j` is cursor-down in
+the diff pane, outline-move-down in the outline), and some bindings are two-key chords
+(`]f`). Whitespace and special keys (Tab, Enter, Esc, arrows, **space**) have no safe
+literal form.
+
+## Decision
+
+All review config lives under `workon.review.*` in git config, extending ADR-006. Config
+is stored **action-as-key** in **per-view subsections**:
+
+```
+workon.review.theme                    = dark            ; global, non-view
+workon.review.<view>.bind.<action>     = "<key tokens>"  ; a keymap entry
+workon.review.<view>.<setting>         = <value>         ; view config
+```
+
+- **View** ∈ `diff`, `outline`; a bare `workon.review.bind.<action>` is the **global**
+  keymap (active in every view). Git parses `workon.review.diff.bind.stage-hunk` as
+  section `workon`, subsection `review.diff.bind`, name `stage-hunk` — dotted subsections
+  are legal and case-sensitive (always lowercase here).
+- **The action is the config variable; the keys are the value.** Each binding is therefore
+  an ordinary *single-valued* variable, so git's native precedence does all override work:
+  setting it replaces (local beats global beats system via `config.get_string()`), and an
+  empty value unbinds. No custom layering, no sentinel. Defaults live in code; a git entry
+  overrides that action's default. Action names qualify as git variable names (alphanumeric
+  + `-`, alpha-initial): `stage-hunk`, `next-file`, `toggle-outline`, …
+- **Value = space-separated key tokens** (an action may have several keys, e.g.
+  `cursor-down = "j down"`). Replace, not append: setting a binding states exactly what
+  triggers it. Token grammar:
+  - **Reserved symbolic names (win over literals):** `space tab enter esc up down left
+    right home end pageup pagedown backspace delete backtab f1`–`f12`.
+  - **Modifier prefix:** `ctrl-`, `alt-`, `shift-` on any token (`ctrl-d`, `ctrl-space`).
+  - **Literal:** otherwise printable chars — length 1 is one key (`s`, `=`), length >1 is a
+    chord (`]f`). A token is matched against reserved words and the modifier grammar first,
+    literal only if neither matches, so `space` is always the spacebar.
+- **View config** (non-binding) shares the view namespace: `workon.review.outline.width`,
+  `workon.review.outline.mode`, `workon.review.diff.layout`, `workon.review.diff.zoom`.
+  The `.bind.` marker is what distinguishes a keymap entry from a view setting.
+- **Load-time inversion:** on startup, walk every `workon.review.*.bind.*` variable, split
+  values into key tokens, and build the per-view key→action dispatch maps. This pass
+  validates (unknown `bind.<action>` → warning; the action set is enumerable) and detects
+  collisions (one key claimed by two actions in a view → footer warning + deterministic
+  winner; defaults never collide, so this only fires on user config).
+- **Not rebindable:** the confirm modal (`y`/`n`/`Esc`) and the whole `Esc` precedence
+  cascade (confirm > outline-unfocus > selection-cancel > quit) stay hardcoded — they are
+  conventional, safety-sensitive, and the Esc cascade's documented precedence would break
+  if rebound.
+
+## Consequences
+
+- One config system across the whole tool; users already know `git config`. Global
+  preferences in `~/.gitconfig`, per-repo in `.git/config`, standard layering — inherited
+  from ADR-006 for free.
+- Override and unbind require **no resolver logic** — they are native git-config semantics.
+  This is the primary reason action-as-key beat value-side `key=action`.
+- Key names and action names become a **compatibility surface**: once users write
+  `workon.review.diff.bind.stage-hunk`, renaming that action or restructuring the namespace
+  breaks their config. Action names are therefore part of the stable API, and the help
+  overlay renders from the same enumerable action set.
+- Like all git-native config (ADR-006), review config is **not checked into the repo**, so a
+  team cannot ship a shared review keymap/theme in-tree. Accepted: this is a
+  personal-productivity TUI.
+- The per-view namespace gives previously-hardcoded view settings (outline width — M5
+  deferred narrow-terminal handling — outline mode, diff layout/zoom defaults) a natural
+  home without a second design pass.
+- Adding a rebindable action = adding it to the enumerable action set (code default +
+  dispatch + help entry); it is automatically configurable, validated, and documented.
+
+## References
+
+- [ADR-006](006-git-native-config.md) — git-native config under `workon.*` this extends
+- `docs/rfc/workon-review.md` — RFC; this is the everyday-usability pass inserted ahead of M7
+- `git-workon-review/src/tui.rs` — current hardcoded keymap (`map_key`) being replaced
+- `git-workon-review/src/render.rs` — current hardcoded palette (`const … Color::Rgb`) — see the theming decision

docs/adr/035-review-theming-base16-hybrid.md

@@ -0,0 +1,141 @@
+# 035 — Review TUI Theming: Hybrid base16, Render-Time Resolution, Terminal-Derived `auto`
+
+## Context
+
+The review TUI's colors were hardcoded during M3–M5: a `const … Color::Rgb(…)` block
+atop `render.rs` (dark-only) and a parallel `HIGHLIGHT_NAMES`/`HIGHLIGHT_COLORS` pair in
+`highlight.rs`. The everyday-usability pass (ahead of M7, see [ADR-034](034-review-git-native-config-schema.md))
+adds built-in light/dark theming and terminal adaptivity. Four things had to be resolved:
+the color *philosophy* (respect the terminal's 16 ANSI colors vs. ship tuned truecolor),
+the theme *primitive*, the *mechanism* by which a theme reaches syntax highlighting, and
+what "adapt to the terminal" concretely means.
+
+Key constraint: diff readability depends on a **truecolor gradient** — `BG_*_SUBTLE` vs
+`BG_*_STRONG` and their staged variants sit a few RGB shades apart, and that gradient is
+how word-level emphasis and staged-vs-unstaged attribution read at a glance. The 16-color
+ANSI palette has no equivalent, so pure "inherit the terminal's ANSI colors" (which would
+self-adapt for free) was rejected — it regresses the readability that is the tool's point.
+
+A second discovery shaped the primitive: `highlight.rs` is *already* a base16 template.
+Its comment says the palette is "in the same family as base16-eighties.dark," and the
+`C_RED/ORANGE/YELLOW/GREEN/CYAN/BLUE/PURPLE` consts are base08–base0E, mapped to captures
+per the base16 spec's role conventions (`keyword → base0E`, `string → base0B`,
+`function → base0D`, `comment → base03`, …). The capture→slot template already exists and
+is spec-conformant.
+
+## Decision
+
+**Philosophy — hybrid, split on "does this color sit on a tinted background?"**
+- **On a tint → base16 truecolor (theme-controlled):** diff add/del subtle/strong + staged
+  variants, cursor, selection, and **syntax**. Contrast is guaranteed because foreground and
+  background come from the *same* scheme.
+- **Chrome (default text, dim labels, gutter/dividers) + the canvas background →
+  base16-ramp-controlled (revised post-CS6):** originally these were ANSI-named
+  (`Color::Gray`/`DarkGray`) and the canvas was never painted, on the theory that inheriting
+  the terminal's own bg/fg would self-adapt for free. In practice this broke explicit
+  `light`/`dark` selections outright — the terminal's own (often dark) bg/fg bled straight
+  through a "light" theme, since nothing ever painted over it. Fixed: `Palette::background`
+  (base00)/`foreground` (base05)/`dim` (base03)/`gutter` (base04) are now real palette
+  fields, and `render()` paints the whole frame with `background` first when
+  `Palette::paint_canvas` is set. `dark()`/`light()` set `paint_canvas: true` — a curated
+  theme now fully controls the look, canvas included. `from_terminal` (`auto`) still derives
+  these four straight from the probed terminal colors — so it matches the terminal exactly,
+  as before — but sets `paint_canvas: false`, since `auto`'s base00 *is* the terminal's own
+  background; painting over it would flatten terminal transparency/background images for no
+  gain. The probe-failure fallback (`dark()`/`light()`) paints normally. Chrome that is
+  never a theme knob (error/warn/current-marker) stays ANSI/const in `render.rs`, unchanged.
+
+**Primitive — the theme is a base16 scheme.** A `Palette` holds the 16 slots
+(base00–07 mono ramp + base08–0F accents). Syntax uses the accents via the existing
+capture→slot template.
+
+Diff-bg tints ideally come from base08 (red / spec "Diff Deleted") and base0B (green / spec
+"Diff Inserted") and the scheme background, so syntax and tints stay coordinated. **But the
+derivation is luminance-dependent, not a single "blend toward base00" (corrected in CS4):**
+- **Dark (base00 dark):** the shipped M3–M5 tints are more saturated/darker than *any* convex
+  blend of an accent toward a dark base00 can produce (their green/blue channels sit *below*
+  base00's). A blend toward a dark base00 also yields muddy mid-tones, not punchy washes. So
+  the **dark tints are held explicit** in `Palette::dark()` (byte-identical to M3–M5, per the
+  pixel-identity gate). Deriving them would require scaling the accent toward *black* plus a
+  desaturation step, not a base00 blend — not worth reverse-engineering the hand-tuned values.
+- **Light (base00 light) and terminal-derived:** blending an accent toward a *light* base00
+  gives the correct pale tint, so the `tint_toward` derivation applies there (CS5/CS6). A
+  terminal-derived theme on a *dark* background hits the same problem as dark and needs the
+  toward-black+desaturate construction — a CS6 concern.
+
+Net: the scheme-coordinated derivation is real but must branch on background luminance; dark
+stays authored.
+
+**Mechanism — resolve color at render time, not in the highlight phase.**
+- `HIGHLIGHT_NAMES` stays global/const: it defines the capture *index space* bound by
+  `config.configure()` and is theme-invariant.
+- `FgSpan` carries the **capture index** (semantic role), not a resolved `Color`. The
+  highlight phase (`highlight.rs:283`) records the index instead of looking up a color.
+- Render resolves `index → Color` against the active `Palette` (`palette.slot[idx]`), in the
+  same place it resolves diff tints and cursor/selection. One theme-application site;
+  syntax and background contrast are reasoned about together.
+- Consequence: the expensive tree-sitter pass is theme-free and cacheable — a theme switch
+  recolors by re-rendering, without re-parsing.
+
+**Selection — `workon.review.theme = auto | dark | light`** (git config, per ADR-034;
+`auto` is the default).
+- **`auto` = terminal-derived.** Probe the terminal for its palette (`OSC 4;n;?` for
+  n=0–15, `OSC 10/11` for fg/bg), populate the 16 slots from the real RGB, and derive tints
+  from the probed base00/08/0B. `auto` *means* terminal-derivation and nothing else — it is
+  not a placeholder for a curated pick (an earlier `COLORFGBG`-picks-curated design was
+  rejected precisely because it would change `auto`'s meaning once the probe landed).
+- **`dark` / `light` = curated base16 schemes** — explicit overrides and the probe-failure
+  fallback. `dark` is the current eighties.dark values; `light` is a published base16 light
+  scheme's 16 hexes (pasted, not hand-invented).
+
+**Terminal derivation specifics.**
+- ANSI-16 cannot fill 6 base16 slots (base01, base02, base04, base06, base09, base0F), so
+  those are **synthesized**: ramp intermediates by interpolation (base01/02 from base00→03,
+  base04/06 from base03→05→07), base09 (orange) by blending base08+base0A, base0F from
+  base09/base08. The diff-critical slots (base00/08/0B) are always real, so tint quality is
+  preserved; the loss is secondary accents.
+- The probe runs at startup on the controlling `/dev/tty` (the TUI already renders there —
+  see `tui.rs`), in raw mode, reading replies with a short timeout. **Failure degrades
+  gracefully:** per-slot fallback to the curated scheme's slot; total failure falls back to
+  the curated scheme chosen by background luminance if `OSC 11` answered, else `dark`.
+  tmux/screen/ssh non-response is handled by the timeout, never a hang.
+
+**CS6 refinement — the diff-bg tints stay curated, only the scheme is derived.** In
+implementation, `auto` derives the base16 **scheme** (the 16 slots → syntax + monochrome ramp)
+from the terminal, but the **diff/cursor/selection tints stay curated by luminance** rather than
+derived from the probed accents (`Palette::from_terminal`: syntax = `SYNTAX_SLOTS` over the probed
+`Base16`; tints = `Palette::dark()`'s or `Palette::light()`'s tint fields, chosen by the luminance
+of the probed `base00`). Two reasons the earlier "derive tints from `base08`/`base0B`" plan was
+narrowed: (1) dark-tint derivation is unsolved (see the corrected Primitive section — a convex
+blend toward a dark `base00` can't reproduce the hand-tuned washes, and a probed *dark* terminal
+hits exactly that), and (2) deriving washes from an arbitrary terminal's accent is unpredictable
+across the range of real terminal palettes. The value of `auto` — **code colors matching the
+terminal** — is fully delivered by the probed syntax slots, which curated tints don't compromise;
+the diff washes were already hand-tuned per luminance, so borrowing them loses nothing. The six
+ANSI-less slots are still synthesized as above; `parse` → `build_base16` → `from_terminal` → the
+`palette_for_auto` fallback decision are all pure and unit-tested, with only the timed `/dev/tty`
+read left untested (see `terminal_query.rs`).
+
+## Consequences
+
+- Light/dark ships as curated base16 schemes now; **terminal-derivation is first-class from
+  the start**, not deferred. `auto` never has to change meaning later.
+- Because color resolves late as `palette.slot[idx]`, the slot *source* is pluggable — a future
+  user-supplied base16 scheme (`theme = <name>` / a scheme file, the deferred
+  "user-configurable colors" tier) is additive, no renderer change.
+- The OSC probe is the single most terminal-fragile component; its blast radius is contained
+  by the timeout + curated fallback, so a hostile terminal yields a correct curated theme,
+  never a hang or a broken palette.
+- Adding a syntax capture = adding it to `HIGHLIGHT_NAMES` + the capture→slot template; it is
+  automatically themed by every scheme.
+- `render.rs` and `highlight.rs` both change: the `const` palette becomes a `Palette` threaded
+  to render; `FgSpan` loses its `Color` field in favor of a capture index. Existing render
+  tests that assert concrete colors must resolve through a fixed test `Palette`.
+
+## References
+
+- [ADR-034](034-review-git-native-config-schema.md) — `workon.review.theme` config key
+- [ADR-006](006-git-native-config.md) — git-native config this builds on
+- `git-workon-review/src/highlight.rs` — existing base16-conformant capture→slot template
+- `git-workon-review/src/render.rs` — `const` palette + `tint_toward` blend helper being generalized
+- base16 styling spec — slot role conventions (base08 red/Diff-Deleted, base0B green/Diff-Inserted, base0E keywords, …)