From 1817ebe819deeeb204fb38ea3c97f93e9fcc4c12 Mon Sep 17 00:00:00 2001 From: dovvnloading Date: Wed, 29 Jul 2026 06:06:36 -0400 Subject: [PATCH] R8a: remove the dead Theme setting (finding #9) Settings' General page had a Theme select (Dark/Muted/Monochromatic) that persisted a real setTheme intent, but nothing in the SPA ever applied the value: no data-theme attribute, no alternate --gl-surface-* token set, the app rendered identically regardless of selection. Per user decision, removed rather than wired up - building two real alternate palettes is a design investment, not a bug fix, and a persisted preference that provably does nothing is worse than no preference. Removed end to end: the frontend select and its options, the setTheme intent and its handler, get_theme/set_theme on SettingsManager (including the now-pointless in-memory backfill for legacy state files missing the key), the theme field from the app-settings wire contract (regenerated), and the now-stale "Appearance" Help entry that described this exact control. Co-Authored-By: Claude Opus 5 --- backend/settings.py | 6 ---- backend/tests/test_settings.py | 23 +++------------ contracts/graphlink_app_settings_payload.py | 1 - graphlink_licensing.py | 10 ------- web_ui/src/app/chrome/SettingsDialog.test.tsx | 1 - web_ui/src/app/chrome/SettingsDialog.tsx | 29 +++++-------------- web_ui/src/app/chrome/help-data/sections.ts | 4 --- .../generated/app-settings-state.schema.json | 4 --- .../generated/app-settings-state.ts | 6 ---- 9 files changed, 11 insertions(+), 73 deletions(-) diff --git a/backend/settings.py b/backend/settings.py index 8ea5d40..f973faf 100644 --- a/backend/settings.py +++ b/backend/settings.py @@ -349,7 +349,6 @@ def _llama_cpp_scan_summary(manager: SettingsManager) -> str: def settings_payload(manager: SettingsManager) -> dict[str, Any]: return { - "theme": manager.get_theme(), "showTokenCounter": manager.get_show_token_counter(), "enableSystemPrompt": manager.get_enable_system_prompt(), "notificationPreferences": manager.get_notification_preferences(), @@ -482,10 +481,6 @@ async def set_active_section(section: str): # are GIL-atomic, and every mutation republishes on completion, so a # snapshot that races a write is immediately superseded by a settled one. - async def set_theme(theme: str): - await asyncio.to_thread(_apply, manager.set_theme, str(theme)) - await bus.publish("app-settings") - async def set_show_token_counter(enabled: bool): await asyncio.to_thread(_apply, manager.set_show_token_counter, bool(enabled)) await bus.publish("app-settings") @@ -1236,7 +1231,6 @@ def _persist() -> None: await bus.publish("app-settings") bus.register_intent("app-settings", "setActiveSection", set_active_section) - bus.register_intent("app-settings", "setTheme", set_theme) bus.register_intent("app-settings", "setShowTokenCounter", set_show_token_counter) bus.register_intent("app-settings", "setEnableSystemPrompt", set_enable_system_prompt) bus.register_intent("app-settings", "setNotificationPreference", set_notification_preference) diff --git a/backend/tests/test_settings.py b/backend/tests/test_settings.py index f2522e2..cb2ede0 100644 --- a/backend/tests/test_settings.py +++ b/backend/tests/test_settings.py @@ -32,7 +32,6 @@ async def send_json(self, data): def test_settings_payload_shape_matches_generated_validator_shape(manager): payload = settings_payload(manager) assert set(payload) == { - "theme", "showTokenCounter", "enableSystemPrompt", "notificationPreferences", @@ -42,7 +41,6 @@ def test_settings_payload_shape_matches_generated_validator_shape(manager): def test_settings_payload_reflects_real_manager_defaults(manager): payload = settings_payload(manager) - assert payload["theme"] == "dark" # R8a: the token counter overlay is off by default - opt-in, not opt-out. assert payload["showTokenCounter"] is False assert payload["enableSystemPrompt"] is True @@ -80,7 +78,6 @@ def test_register_settings_publishes_active_section_alongside_manager_state(mana asyncio.run(bus.publish("app-settings")) payload = recorder.messages[0]["payload"] assert payload["activeSection"] == "general" - assert payload["theme"] == "dark" def test_set_active_section_intent_updates_only_local_ui_state(manager): @@ -94,18 +91,6 @@ def test_set_active_section_intent_updates_only_local_ui_state(manager): assert payload["activeSection"] == "integrations" -def test_set_theme_intent_persists_to_the_real_settings_manager(manager): - bus = SessionBus("settings-theme-test") - register_settings(bus, manager) - - asyncio.run(bus.dispatch_intent("app-settings", "setTheme", ["light"])) - assert manager.get_theme() == "light" - # A fresh manager reading the same file proves it was actually persisted, - # not just mutated in memory. - reloaded = SettingsManager(manager.state_file) - assert reloaded.get_theme() == "light" - - def test_set_show_token_counter_intent(manager): bus = SessionBus("settings-token-counter-test") register_settings(bus, manager) @@ -178,17 +163,17 @@ def test_an_old_settings_file_without_schema_version_is_backfilled(tmp_path): import json state_file = tmp_path / "session.dat" - state_file.write_text(json.dumps({"theme": "mono"}), encoding="utf-8") + state_file.write_text(json.dumps({"show_token_counter": True}), encoding="utf-8") old_manager = SettingsManager(state_file) # Backfilled in memory immediately, the same as every other pre-existing - # key here (theme, show_token_counter, ...) - none of those write back to - # disk until the next explicit set_*() call either, so schema_version + # key here (show_token_counter, ...) - none of those write back to disk + # until the next explicit set_*() call either, so schema_version # matching that existing pattern is correct, not a gap. assert old_manager.get_schema_version() == SettingsManager.CURRENT_SCHEMA_VERSION - old_manager.set_theme("mono") # any setter call persists the whole (now-backfilled) state + old_manager.set_show_token_counter(False) # any setter call persists the whole (now-backfilled) state assert json.loads(state_file.read_text(encoding="utf-8"))["schema_version"] == SettingsManager.CURRENT_SCHEMA_VERSION diff --git a/contracts/graphlink_app_settings_payload.py b/contracts/graphlink_app_settings_payload.py index 66b19dc..6bcb696 100644 --- a/contracts/graphlink_app_settings_payload.py +++ b/contracts/graphlink_app_settings_payload.py @@ -35,7 +35,6 @@ class AppSettingsStatePayload: schemaVersion: int revision: int activeSection: str - theme: str showTokenCounter: bool enableSystemPrompt: bool notificationPreferences: dict[str, bool] diff --git a/graphlink_licensing.py b/graphlink_licensing.py index 6683364..80f24a3 100644 --- a/graphlink_licensing.py +++ b/graphlink_licensing.py @@ -85,8 +85,6 @@ def _load_state(self): try: with open(self.state_file, 'r') as f: state = json.load(f) - if 'theme' not in state: - state['theme'] = 'dark' if 'show_token_counter' not in state: state['show_token_counter'] = False state_changed = False @@ -232,7 +230,6 @@ def _backup_corrupt_state_file(self, error): def _create_initial_state(self): state = { "schema_version": self.CURRENT_SCHEMA_VERSION, - "theme": "dark", "show_token_counter": False, "ollama_chat_model": "", "ollama_title_model": "", @@ -370,13 +367,6 @@ def _save_state(self, state_data=None): def get_schema_version(self): return self.state.get("schema_version", self.CURRENT_SCHEMA_VERSION) - def get_theme(self): - return self.state.get("theme", "dark") - - def set_theme(self, theme_name): - self.state['theme'] = theme_name - self._save_state() - def get_show_token_counter(self): # R8a: off by default - the overlay is opt-in now, not opt-out. return self.state.get("show_token_counter", False) diff --git a/web_ui/src/app/chrome/SettingsDialog.test.tsx b/web_ui/src/app/chrome/SettingsDialog.test.tsx index 89e22ad..990355f 100644 --- a/web_ui/src/app/chrome/SettingsDialog.test.tsx +++ b/web_ui/src/app/chrome/SettingsDialog.test.tsx @@ -17,7 +17,6 @@ const snapshot = { minCompatibleSchemaVersion: 1, revision: 1, activeSection: "general", - theme: "dark", showTokenCounter: true, enableSystemPrompt: true, notificationPreferences: { info: true, success: true, warning: true, error: true }, diff --git a/web_ui/src/app/chrome/SettingsDialog.tsx b/web_ui/src/app/chrome/SettingsDialog.tsx index 9887c1e..3e689b1 100644 --- a/web_ui/src/app/chrome/SettingsDialog.tsx +++ b/web_ui/src/app/chrome/SettingsDialog.tsx @@ -49,12 +49,6 @@ function basename(path: string): string { return path.split(/[\\/]/).pop() || path; } -const THEME_OPTIONS = [ - { value: "dark", label: "Dark" }, - { value: "muted", label: "Muted" }, - { value: "mono", label: "Monochromatic" }, -]; - const NOTIFICATION_TYPE_LABELS: Record = { info: "Info", success: "Success", @@ -95,7 +89,6 @@ const initialState: AppSettingsState = { minCompatibleSchemaVersion: 1, revision: 0, activeSection: "General", - theme: "dark", showTokenCounter: true, enableSystemPrompt: true, notificationPreferences: {}, @@ -146,23 +139,15 @@ function GeneralPage({ state: AppSettingsState; transport: WsTransport; }) { + // R8a (UI/UX issue list finding #9): a Theme select (Dark/Muted/ + // Monochromatic) used to live here. It persisted a real setTheme intent, + // but nothing in the SPA ever applied the value - no data-theme attribute, + // no alternate token set, the app rendered identically no matter what was + // selected. Removed rather than wired up: building two real alternate + // palettes is a design decision, not a bug fix, and a persisted preference + // that provably does nothing is worse than no preference at all. return (
- -