Approving a scheme only promotes `community` to `user`; AI and hand-edited schemes keep their original source #99

New issue

Closed

opened 2026-09-01 02:58:43 +00:00 by claude-bot · 0 comments

claude-bot commented

2026-09-01 02:58:43 +00:00

Contributor

Found during #7's operational dry run.

What happens

approve_scheme promotes source for exactly one case — backend/app/routers/schemes.py:102:

if scheme.source == "community":
    scheme.source = "user"

And PUT /schemes/{scheme_id} never touches source at all. So a scheme whose colours, effect, speed, brightness and label the user has entirely rewritten still reports the source of the template it started from.

Concretely, tonight I authored late_summer_moon_2026_mid from scratch through PUT — every colour, fx, sx and the label replaced, nothing of the tag-fallback template surviving — then approved it. It still reports source: "builtin". Same for labor_day_2026_mid.

Why it matters

One of the three is a straight spec violation; two are arguable.

1. AI → user is specified and not implemented. Spec §3.3, source layer 4:

…becomes a candidate the user can approve — after which it's promoted to source 1 and reused automatically going forward.

Only community is handled, so an approved AI scheme stays source: "ai" permanently.

The visible consequence is in the review workflow: the calendar's "AI-sourced" filter matches s.source === "ai" (CalendarPage.tsx:39, YearCalendar.tsx:63). A scheme the user has reviewed and adopted keeps appearing there forever — so the filter never drains and stops working as a review queue, which is the only thing it's for.

2. The provenance badge is wrong. SidePanel.tsx:50 shows the "success" badge on source.includes("user"). An approved, hand-edited scheme shows as builtin/AI instead.

3. Editing doesn't mark a scheme user-authored. Arguably the sharpest: after a complete rewrite, source still names a template that contributed nothing to the result.

What is not affected

Cross-year reuse is fine — I checked before filing so this isn't over-scoped. _find_approved_for_key (scheme_generator.py:65) filters on approved == True only, not on source, so an approved builtin/AI scheme is carried forward to the next occurrence of its event_key. This is a provenance and UI-labelling defect, not a scheme-reuse one.

Suggested fix

On approve, promote community, ai and fallback → user.
On PUT /schemes/{id}, when segments or label change, set source = "user".
Make a deliberate decision about plain builtin. Spec source 1 is "schemes the user has explicitly saved or approved", which reads as covering it — but keeping builtin legible as "came from the curated library and was accepted unchanged" is also defensible. Whichever way it goes, code and spec should agree; today they don't.

Acceptance criteria

Approving an AI-sourced scheme sets source: user and it leaves the "AI-sourced" filter
Editing a scheme's content marks it user
A recorded decision, with spec and code agreeing, on whether approving an untouched builtin promotes
Tests covering the promotion matrix (builtin / community / ai / fallback × approve / edit)

Found during #7's operational dry run. ## What happens `approve_scheme` promotes `source` for exactly one case — [`backend/app/routers/schemes.py:102`](backend/app/routers/schemes.py#L102): ```python if scheme.source == "community": scheme.source = "user" ``` And `PUT /schemes/{scheme_id}` never touches `source` at all. So a scheme whose colours, effect, speed, brightness and label the user has *entirely rewritten* still reports the source of the template it started from. Concretely, tonight I authored `late_summer_moon_2026_mid` from scratch through `PUT` — every colour, `fx`, `sx` and the label replaced, nothing of the tag-fallback template surviving — then approved it. It still reports `source: "builtin"`. Same for `labor_day_2026_mid`. ## Why it matters One of the three is a straight spec violation; two are arguable. **1. AI → user is specified and not implemented.** Spec §3.3, source layer 4: > …becomes a candidate the user can approve — after which it's **promoted to source 1** and reused automatically going forward. Only `community` is handled, so an approved AI scheme stays `source: "ai"` permanently. The visible consequence is in the review workflow: the calendar's **"AI-sourced" filter** matches `s.source === "ai"` ([`CalendarPage.tsx:39`](frontend/src/pages/CalendarPage.tsx#L39), [`YearCalendar.tsx:63`](frontend/src/components/calendar/YearCalendar.tsx#L63)). A scheme the user has reviewed and adopted keeps appearing there forever — so the filter never drains and stops working as a review queue, which is the only thing it's for. **2. The provenance badge is wrong.** [`SidePanel.tsx:50`](frontend/src/components/panel/SidePanel.tsx#L50) shows the "success" badge on `source.includes("user")`. An approved, hand-edited scheme shows as builtin/AI instead. **3. Editing doesn't mark a scheme user-authored.** Arguably the sharpest: after a complete rewrite, `source` still names a template that contributed nothing to the result. ## What is *not* affected Cross-year reuse is fine — I checked before filing so this isn't over-scoped. `_find_approved_for_key` ([`scheme_generator.py:65`](backend/app/services/scheme_generator.py#L65)) filters on `approved == True` only, not on source, so an approved builtin/AI scheme **is** carried forward to the next occurrence of its `event_key`. This is a provenance and UI-labelling defect, not a scheme-reuse one. ## Suggested fix - On approve, promote `community`, `ai` and `fallback` → `user`. - On `PUT /schemes/{id}`, when segments or label change, set `source = "user"`. - Make a deliberate decision about plain `builtin`. Spec source 1 is *"schemes the user has explicitly saved **or approved**"*, which reads as covering it — but keeping `builtin` legible as "came from the curated library and was accepted unchanged" is also defensible. Whichever way it goes, code and spec should agree; today they don't. ## Acceptance criteria - [ ] Approving an AI-sourced scheme sets `source: user` and it leaves the "AI-sourced" filter - [ ] Editing a scheme's content marks it `user` - [ ] A recorded decision, with spec and code agreeing, on whether approving an untouched `builtin` promotes - [ ] Tests covering the promotion matrix (`builtin` / `community` / `ai` / `fallback` × approve / edit)

claude-bot added the

labels

2026-09-01 02:58:52 +00:00

claude-bot added this to the v1.1.0 milestone

2026-09-01 21:54:09 +00:00