conductor(plan): prior_session_sepia_20260610 spec + design + metadata
New track for prior-session sepia tint: - 3 new theme slots (prior_session_bg, prior_session_tint, prior_session_amount) - per-palette state dict mirroring _brightness/_contrast/_gamma - apply_prior_tint helper (float-only math per user requirement) - 6 prior-session render sites wrapped (2 bubble_vendor swaps + 4 tint wraps) - Theme Settings panel slider with persistence Code-block tonemap fix is OUT OF SCOPE (upstream imgui_bundle 1.92.5 API only exposes 4-value PaletteId enum, no per-instance struct). See spec §1.1.1 and design doc 'Honest constraint' section.
This commit is contained in:
@@ -0,0 +1,148 @@
|
||||
# Prior-Session Sepia Tint — Design
|
||||
|
||||
**Date:** 2026-06-10
|
||||
**Status:** Approved (pending user spec review)
|
||||
**Track:** `prior_session_sepia_20260610`
|
||||
**Spec:** [../../../conductor/tracks/prior_session_sepia_20260610/spec.md](../../../conductor/tracks/prior_session_sepia_20260610/spec.md)
|
||||
|
||||
## Problem Statement
|
||||
|
||||
The current prior-session (Historical Replay) mode uses `theme.get_color("bubble_vendor")` as a
|
||||
background tint at three call sites in `src/gui_2.py` (lines 1028, 3960, plus the "HISTORICAL
|
||||
VIEW" banners at 5142 and 5440). This is a **semantic overload** — `bubble_vendor` is the
|
||||
"Vendor API" role's bubble color, not a dedicated prior-session slot. Theme authors cannot tune
|
||||
the prior-session feel without changing the Vendor API bubble.
|
||||
|
||||
Beyond the missing dedicated slot, the **content** (text, markdown) renders at full
|
||||
palette saturation. The "obviously looking at the past" cue is carried only by the bg, not by
|
||||
the prose.
|
||||
|
||||
**Pre-existing related bug** (surfaced by the user during brainstorming): code blocks rendered
|
||||
by `imgui_color_text_edit` are not tonemap-aware. The library has 4 hardcoded palettes (`dark`,
|
||||
`light`, `mariana`, `retro_blue`) and their colors are never passed through `theme._tone_map()`.
|
||||
The user has called this "disappointing" because it defeats the primary purpose of the tonemapper:
|
||||
allowing a light theme to be usable on a bright monitor without searing the user's retinas.
|
||||
**This bug is NOT fixable in this track** — see "Honest constraint" below.
|
||||
|
||||
## Brainstorm Q&A
|
||||
|
||||
### Q1. Tint scope (data only vs. data+chrome vs. whole window)
|
||||
|
||||
**A1 confirmed sepia. A2 chose "anything that is actually affected by the prior session (is
|
||||
hosting old information, old state)" → scope (ii) data + chrome inside prior-session views, with
|
||||
fallback to (iii) whole window if scope (ii) doesn't look "obviously old" in manual review.**
|
||||
|
||||
### Q2. Two independent effects (distinct bg + tint) or single transform
|
||||
|
||||
**A — Two independent effects, slider only controls the content tint (recommended and chosen).**
|
||||
New theme slots: `prior_session_bg` (flat warm color), `prior_session_tint` (sepia color),
|
||||
`prior_session_amount` (per-palette float 0.0-1.0). Slider controls only the amount; bg is
|
||||
whatever the theme says.
|
||||
|
||||
### Q3. (a) Slider location, (b) tint scope, (c) default per-theme, (d) code blocks
|
||||
|
||||
**(a) Theme Settings panel** (under the existing Tone Mapping section). Mirrors the
|
||||
tonemap pattern exactly.
|
||||
|
||||
**(b) Scope (ii) data + chrome inside prior-session views**, with fallback to (iii) whole
|
||||
window only if (ii) doesn't look obviously old.
|
||||
|
||||
**(c) 0.3 (subtle)** — the user can slide up if they want more aggressive. The default is
|
||||
intentionally subtle because the user said "obviously looking at the past" but the
|
||||
prior-session feature is a niche mode, not a primary view.
|
||||
|
||||
**(d) Originally proposed: bundle the code-block tonemap fix into this track.** The user
|
||||
revealed the pre-existing disappointment and said: *"if you can somehow tint the code blocks
|
||||
lmk cause right now they are not affected by tonemapping."* The fix was proposed as
|
||||
"mutate the `Palette` struct's color slots via `editor.get_palette()` and re-apply."
|
||||
|
||||
### Q4. Float-only math (HARD CONSTRAINT)
|
||||
|
||||
> "Make sure that all math you do is not integer based. I want to have as much accuracy as
|
||||
> possible for smooth calculations."
|
||||
|
||||
Applied to: `apply_prior_tint`, the slider (`slider_float` not `slider_int`), the per-palette
|
||||
state dict (stores `float` not `int`), the TOML key (`prior_session_amount: float`), the
|
||||
code-block palette mutation. The transform pipeline never truncates to int.
|
||||
|
||||
## Approach: A1 — Per-render explicit transform
|
||||
|
||||
Add `apply_prior_tint(rgba, palette) -> rgba` helper in `src/theme_2.py`. Per-palette state
|
||||
lives in `_prior_session_amount: dict[str, float]` mirroring `_brightness` exactly. Each
|
||||
prior-session rendering site in `src/gui_2.py` wraps its `theme.get_color()` call with
|
||||
`apply_prior_tint(...)` (one-line wrap).
|
||||
|
||||
### Honest constraint surfaced during self-review
|
||||
|
||||
I verified the upstream `imgui_bundle 1.92.5` API before writing the plan:
|
||||
|
||||
```
|
||||
$ python -c "from imgui_bundle import imgui_color_text_edit as ed; help(ed.TextEditor.get_palette)"
|
||||
get_palette(self) -> imgui_bundle._imgui_bundle.imgui_color_text_edit.TextEditor.PaletteId
|
||||
$ python -c "from imgui_bundle import imgui_color_text_edit as ed; help(ed.TextEditor.set_palette)"
|
||||
set_palette(self, a_value: imgui_bundle._imgui_bundle.imgui_color_text_edit.TextEditor.PaletteId) -> None
|
||||
```
|
||||
|
||||
**`PaletteId` is a 4-value enum** (`dark`, `light`, `mariana`, `retro_blue`). There is
|
||||
no `Palette` struct with mutable per-color slots. The original brainstorm proposed
|
||||
`apply_color_grades_to_editor_palette(editor, palette)` that would mutate the struct
|
||||
in-place — but the struct doesn't exist in this API surface.
|
||||
|
||||
**This means the code-block tonemap-awareness is NOT fixable in this track** (or any
|
||||
track that doesn't fork the library). The user's disappointment is real and the
|
||||
pre-existing behavior persists. The same constraint forced the `multi_themes_20260604`
|
||||
track to ship a `syntax_palette` enum field rather than custom token colors. The
|
||||
honest answer is in the spec's §1.1.1.
|
||||
|
||||
### Why A1 over A2 (transparent via get_color) and A3 (context manager)
|
||||
|
||||
- **A2** would auto-apply sepia inside `theme.get_color()` when `is_viewing_prior_session` is
|
||||
True. Rejected: violates the data-oriented "view composes" principle; risks accidentally
|
||||
tinting status indicators that must stay saturated.
|
||||
- **A3** would require a `with prior_session_view():` wrapper at every prior-session site.
|
||||
Rejected: same number of wraps as A1 but uglier.
|
||||
|
||||
### Float-only math contract
|
||||
|
||||
```
|
||||
result = lerp(desaturate(input), tint_color, amount)
|
||||
```
|
||||
|
||||
- `desaturate(rgba)`: BT.709 luma `0.2126*r + 0.7152*g + 0.0722*b` (all float, all 0.0-1.0).
|
||||
- `lerp(a, b, t)`: `a + (b - a) * t` per channel, `t` clamped to [0.0, 1.0].
|
||||
- `apply_prior_tint(rgba, palette)`: identity at amount=0.0; pure tint at amount=1.0; alpha
|
||||
passed through unchanged; output is `tuple[float, float, float, float]`.
|
||||
|
||||
## File changes (high-level)
|
||||
|
||||
| File | Action | Purpose |
|
||||
|---|---|---|
|
||||
| `src/theme_2.py` | Modify | Add `_prior_session_amount` dict + 3 accessors; add `_desaturate`, `_lerp_rgba`, `_imvec4_to_rgba`, `_rgba_to_imvec4`, `apply_prior_tint`; add 3 keys to fallback dict; persist to config |
|
||||
| `src/theme_models.py` | Modify | Add 3 fields to `ThemePalette`; update `from_dict` / `to_dict` / validator |
|
||||
| `src/gui_2.py` | Modify | 2 `bubble_vendor` → `prior_session_bg` swaps; 4 `apply_prior_tint` wraps at the 2 banners + 2 render functions; 1 new Theme panel section |
|
||||
| `themes/*.toml` (8 files) | Modify | Add 3 new keys with per-theme defaults |
|
||||
| `tests/test_prior_session_amount.py` | Create | per-palette dict semantics |
|
||||
| `tests/test_prior_session_tint.py` | Create | math contract: identity/pure/monotonic/alpha |
|
||||
| `tests/test_prior_session_toml.py` | Create | round-trip + validation |
|
||||
| `tests/test_prior_session_render.py` | Create | ImVec4 ↔ tuple round-trip |
|
||||
| `tests/test_prior_session_persistence.py` | Create | slider → save → restart round-trip |
|
||||
|
||||
## Risk assessment
|
||||
|
||||
| Risk | Likelihood | Impact | Mitigation |
|
||||
|---|---|---|---|
|
||||
| Float math accumulates rounding error over many frames | Low | Low | Each `apply_prior_tint` call is independent; no accumulation; output clamped to [0.0, 1.0] |
|
||||
| The 8 themes don't all have sensible defaults for the new keys | Low | Low | Defaults in the fallback dict cover missing keys |
|
||||
| Light themes need a different default for `prior_session_bg` (cream vs. dark brown) | Med | Low | Defaults table in the spec sets per-light-theme `prior_session_bg = (235, 220, 190)` |
|
||||
| Live-gui tests regress because of the new `apply_prior_tint` wraps | Low | Med | Phase 4 batch-verifies the full suite; per `live_gui_test_hardening_20260605` rule, batch is the only verification that matters |
|
||||
| The scope (ii) "data + chrome inside prior-session views" doesn't look "obviously old" at the 0.3 default | Med | Low | Phase 4 manual smoke escalates to scope (iii) if needed; the wrap is local to 6 sites, easy to expand |
|
||||
| **Code-block tonemap-awareness disappoints the user again** | High | Low | Explicit §1.1.1 in the spec; the pre-existing bug persists; user was told upfront in the design doc that the API doesn't support per-instance Palette |
|
||||
|
||||
## Out of scope (matches spec §9)
|
||||
|
||||
- Per-language syntax color customization (upstream limitation)
|
||||
- Film grain / vignette / scanline post-effect
|
||||
- Per-theme user overrides via `config.toml` (TOML key is factory default only)
|
||||
- Modifying the upstream `imgui_color_text_edit` library
|
||||
- Process-isolation of the pure helper
|
||||
- Reorganizing the existing tonemap state dicts
|
||||
Reference in New Issue
Block a user