Presets: fill missing built-in defaults on load
PresetManager.load already heals a forward-incompatible presets.json: the block just above repairs the legacy `custom` shape and re-saves the file. But if the file exists and is missing a whole built-in preset (e.g. an older install written before `reason` existed), load returned it as-is, so that built-in stayed permanently absent — silently missing from the picker that GET /api/presets feeds, with no way for the user to get it back. Extend the same self-heal: after the legacy migration, fill in any built-in presets the loaded file is missing, defaults-first so user edits win, and persist the result. This never clobbers an intentional removal — there is no delete path for the built-in keys (only user_templates entries can be deleted), and presets are hidden via an `enabled: False` flag, not removal. Checks: python -m pytest tests/test_preset_fill_missing_defaults.py (3 passed; 2 fail on the pre-fix code), the existing preset cases in tests/test_review_regressions.py still pass, python -m py_compile src/preset_manager.py, git diff --check.
This commit is contained in:
@@ -92,6 +92,18 @@ Use precise language. Show causal relationships explicitly. Quantify uncertainty
|
||||
custom.setdefault("inject_prefix", "")
|
||||
custom.setdefault("inject_suffix", "")
|
||||
self.save(presets)
|
||||
# Heal a forward-incompatible file the same way the legacy `custom`
|
||||
# migration above does: fill in any built-in presets an older or
|
||||
# partial presets.json is missing, so they reach existing installs
|
||||
# (a missing built-in is otherwise silently absent from the picker
|
||||
# served by GET /api/presets). There is no delete path for the
|
||||
# built-in keys, so this never clobbers an intentional removal.
|
||||
# Defaults first, loaded values win — user edits are preserved.
|
||||
if isinstance(presets, dict) and any(
|
||||
k not in presets for k in self.DEFAULT_PRESETS
|
||||
):
|
||||
presets = {**self.DEFAULT_PRESETS, **presets}
|
||||
self.save(presets)
|
||||
return presets
|
||||
except Exception as e:
|
||||
logger.error(f"Error loading presets: {e}")
|
||||
|
||||
78
tests/test_preset_fill_missing_defaults.py
Normal file
78
tests/test_preset_fill_missing_defaults.py
Normal file
@@ -0,0 +1,78 @@
|
||||
"""An older / partial presets.json must be healed forward on load: built-in
|
||||
presets that are missing get filled in, WITHOUT clobbering user edits.
|
||||
|
||||
This extends the adjacent legacy `custom`-shape migration in
|
||||
`PresetManager.load`, which already repairs forward-incompatible files and
|
||||
re-saves them. A missing built-in is never an intentional user action — there
|
||||
is no delete path for the built-in keys (only `user_templates` entries can be
|
||||
deleted), and presets are hidden via an `enabled: False` flag, not removal — so
|
||||
filling them back in is safe.
|
||||
"""
|
||||
import json
|
||||
import os
|
||||
import tempfile
|
||||
|
||||
from src.preset_manager import PresetManager
|
||||
|
||||
|
||||
def _write_presets(data: dict) -> str:
|
||||
d = tempfile.mkdtemp()
|
||||
with open(os.path.join(d, "presets.json"), "w", encoding="utf-8") as f:
|
||||
json.dump(data, f)
|
||||
return d
|
||||
|
||||
|
||||
def test_missing_builtin_presets_are_filled_in():
|
||||
# Partial file: has code_analyze + brainstorm, missing reason + custom.
|
||||
data_dir = _write_presets({
|
||||
"code_analyze": {"name": "Code Analyze", "temperature": 0.2,
|
||||
"max_tokens": 8000, "system_prompt": "analyze"},
|
||||
"brainstorm": {"name": "Brainstorm", "temperature": 0.9,
|
||||
"max_tokens": 4096, "system_prompt": "ideate"},
|
||||
})
|
||||
pm = PresetManager(data_dir)
|
||||
for key in PresetManager.DEFAULT_PRESETS:
|
||||
assert key in pm.presets, f"built-in preset {key!r} should be present"
|
||||
# The fill is persisted so the next load is already complete.
|
||||
with open(os.path.join(data_dir, "presets.json"), encoding="utf-8") as f:
|
||||
on_disk = json.load(f)
|
||||
assert "reason" in on_disk and "custom" in on_disk
|
||||
|
||||
|
||||
def test_fill_does_not_clobber_user_edits():
|
||||
# An edited `custom` (enabled, bespoke prompt) plus a missing `reason`.
|
||||
edited_custom = {
|
||||
"name": "My Persona",
|
||||
"character_name": "My Persona",
|
||||
"temperature": 0.55,
|
||||
"max_tokens": 1234,
|
||||
"system_prompt": "You are my bespoke assistant.",
|
||||
"inject_prefix": "PRE",
|
||||
"inject_suffix": "SUF",
|
||||
"enabled": True,
|
||||
}
|
||||
data_dir = _write_presets({
|
||||
"code_analyze": {"name": "Code Analyze", "temperature": 0.2,
|
||||
"max_tokens": 8000, "system_prompt": "analyze"},
|
||||
"brainstorm": {"name": "Brainstorm", "temperature": 0.9,
|
||||
"max_tokens": 4096, "system_prompt": "ideate"},
|
||||
"custom": edited_custom,
|
||||
"user_templates": [{"id": "t1", "name": "Tmpl"}],
|
||||
# missing: reason
|
||||
})
|
||||
pm = PresetManager(data_dir)
|
||||
# reason was filled...
|
||||
assert "reason" in pm.presets
|
||||
# ...but the user's edited custom + templates are untouched.
|
||||
assert pm.presets["custom"] == edited_custom
|
||||
assert pm.presets["user_templates"] == [{"id": "t1", "name": "Tmpl"}]
|
||||
|
||||
|
||||
def test_complete_file_is_not_rewritten_needlessly():
|
||||
# A file that already has every built-in must be returned unchanged.
|
||||
full = {k: dict(v) for k, v in PresetManager.DEFAULT_PRESETS.items()}
|
||||
full["custom"]["enabled"] = True # a user edit that must survive
|
||||
data_dir = _write_presets(full)
|
||||
pm = PresetManager(data_dir)
|
||||
assert pm.presets["custom"]["enabled"] is True
|
||||
assert set(PresetManager.DEFAULT_PRESETS) <= set(pm.presets)
|
||||
Reference in New Issue
Block a user