fix: stop remote form entries overwriting each other

1 files changed, 128 insertions, 51 deletions

diff --git a/app/tests/test_hub.py b/app/tests/test_hub.py
index f350072..9f72ad9 100644
--- a/app/tests/test_hub.py
+++ b/app/tests/test_hub.py
@@ -649,12 +649,23 @@ class ConvertFlowTests(unittest.TestCase):
             patcher.start()
             self.addCleanup(patcher.stop)
 
+    # Keys shared by every backend entry; a "-remote" backend's other
+    # option keys are namespaced under "<entry>." in the form dict
+    # (mirroring hub.py), so _form_values maps them automatically.
+    _COMMON_KEYS = frozenset(("backend", "output_format", "speed",
+                              "single_file", "debug", "stop_and_exit"))
+
     def _form_values(self, **overrides):
         """A fully-populated form result, with sensible defaults."""
         values = {"output_format": "m4b", "speed": "1.0",
                   "single_file": False, "debug": False,
                   "stop_and_exit": True}
         values.update(overrides)
+        backend = values.get("backend") or ""
+        if backend.endswith("-remote"):
+            prefix = f"{backend}."
+            values = {(prefix + key if key not in self._COMMON_KEYS else key):
+                      value for key, value in values.items()}
         return values
 
     def _answer_form(self, **overrides):
@@ -662,7 +673,8 @@ class ConvertFlowTests(unittest.TestCase):
 
     def _field(self, key, form_index=-1):
         _, fields, _ = self.tui.forms_seen[form_index]
-        return next(f for f in fields if f["key"] == key)
+        return next(f for f in fields if f["key"] == key
+                    or f["key"].endswith("." + key))
 
     def _ready(self, key, label):
         """A backend status that is ready to convert with."""
@@ -748,9 +760,10 @@ class ConvertFlowTests(unittest.TestCase):
         title, fields, form_kwargs = self.tui.forms_seen[0]
         self.assertEqual(title, "Generate audiobooks")
         self.assertEqual([f["key"] for f in fields],
-                         ["backend", "model_id", "audiocpp_voice",
-                          "instructions", "output_format", "speed",
-                          "single_file", "debug", "stop_and_exit"])
+                         ["backend", "audiocpp-remote.model_id",
+                          "audiocpp-remote.audiocpp_voice",
+                          "audiocpp-remote.instructions", "output_format",
+                          "speed", "single_file", "debug", "stop_and_exit"])
         self.assertEqual(form_kwargs["buttons"], ("Generate!", "Cancel"))
         self.assertTrue(form_kwargs["start_on_buttons"])
         # The stop-and-exit toggle ships on by default.
@@ -1002,6 +1015,59 @@ class ConvertFlowTests(unittest.TestCase):
         self.assertEqual(fields[0]["choices"],
                          [("audio.cpp", "audiocpp"),
                           ("audio.cpp [remote]", "audiocpp-remote")])
+        # The two entries' fields are namespaced, so both carry their own
+        # values and picking one never leaks the other's into the run.
+        keys = [f["key"] for f in fields]
+        self.assertIn("model_id", keys)
+        self.assertIn("audiocpp-remote.model_id", keys)
+
+    def test_managed_and_remote_entries_do_not_overwrite_each_other(self):
+        # Regression: the form returns one flat {key: value} dict. When
+        # managed and remote entries shared field keys, the remote entry's
+        # hidden defaults silently won over the user's edits on whichever
+        # entry was selected.
+        self._patch_remote(
+            [{"id": "higgs", "family": "higgs_audio_tts", "task": "tts"}],
+            voices=["narrator"])
+        statuses = [self._ready("audiocpp", "audio.cpp"),
+                    self._remote("audiocpp", "audio.cpp")]
+        common = {"output_format": "m4b", "speed": "1.0",
+                  "single_file": False, "debug": False,
+                  "stop_and_exit": True}
+        with tempfile.TemporaryDirectory() as td:
+            root = Path(td)
+            (root / "server.json").write_text(json.dumps({
+                "models": [{"id": "qwen", "family": "qwen3_tts",
+                            "task": "tts"}],
+            }), encoding="utf-8")
+            with patch.object(hub.audiocpp_backend, "find_local_checkout",
+                              return_value=root), \
+                    patch.object(hub.config, "AUDIOCPP_INSTRUCTIONS", ""):
+                # Managed selected: its picks must survive next to the
+                # remote entry's same-shaped fields.
+                self.tui.form_script.append({
+                    "backend": "audiocpp", "model_id": "qwen",
+                    "audiocpp_voice": "", "instructions": "",
+                    "audiocpp-remote.model_id": "higgs",
+                    "audiocpp-remote.audiocpp_voice": "narrator",
+                    **common})
+                managed_cmd = self._convert(None, statuses)
+                self.assertIsNotNone(managed_cmd)
+                self.assertEqual(managed_cmd[2]["model_id"], "qwen")
+                # The remote entry's hidden "narrator" voice must not leak
+                # into the managed run (the old duplicate-key behavior).
+                self.assertIsNone(managed_cmd[2]["voice"])
+                # Remote selected: its picks win instead.
+                self.tui.form_script.append({
+                    "backend": "audiocpp-remote",
+                    "audiocpp-remote.model_id": "higgs",
+                    "audiocpp-remote.audiocpp_voice": "narrator",
+                    "model_id": "qwen",
+                    **common})
+                remote_cmd = self._convert(None, statuses)
+        self.assertIsNotNone(remote_cmd)
+        self.assertEqual(remote_cmd[2]["model_id"], "higgs")
+        self.assertEqual(remote_cmd[2]["voice"], "narrator")
 
     def test_audiocpp_remote_mapper_adds_api_url(self):
         self._patch_remote(
@@ -1038,15 +1104,20 @@ class ConvertFlowTests(unittest.TestCase):
                 patch.object(hub.config, "SPEAKER", "Vivian"):
             self._answer_form(backend="qwen", mode="custom", speaker="Serena",
                               clone="")
-            with patch.object(hub.common, "update_config_value") as mk_update:
+            # The fake mirrors the real update_config_value contract:
+            # persisting a value also lands it on the imported module.
+            def fake_update(key, value, config_path=None):
+                setattr(hub.config, key, value)
+                return True
+
+            with patch.object(hub.common, "update_config_value",
+                              fake_update):
                 cmd = self._convert(None,
                                         [self._ready("qwen", "qwen-tts")])
             speaker_in_memory = hub.config.SPEAKER
         self.assertEqual(cmd[0], "convert")
         self.assertEqual(cmd[1], hub.BACKEND_QWEN)
         self.assertIsNone(cmd[2]["clone"])
-        # The speaker choice is persisted for future runs too.
-        mk_update.assert_called_once_with("SPEAKER", "Serena")
         self.assertEqual(speaker_in_memory, "Serena")
         fields = self.tui.forms_seen[0][1]
         self.assertEqual([f["key"] for f in fields],
@@ -1512,8 +1583,25 @@ class AddAutostartTests(unittest.TestCase):
 class SettingsTests(unittest.TestCase):
     """Settings menu: field collection, validation, config.py writing."""
 
-    def test_write_config_preserves_comments_and_other_lines(self):
+    # Keys _apply_settings persists; every test that triggers a real or
+    # fake config write restores these afterwards.
+    _SETTING_KEYS = ("AUDIO_FORMAT", "AUDIO_BITRATE", "LANGUAGE",
+                     "CHUNK_SIZE", "STOP_SERVER_AND_EXIT",
+                     "AUDIOCPP_UNLOAD_MODELS",
+                     "QWEN_API_URL", "CLONE_API_URL",
+                     "FASTER_API_URL", "AUDIOCPP_API_URL",
+                     "QWEN_REMOTE_URL", "CLONE_REMOTE_URL",
+                     "FASTER_REMOTE_URL", "AUDIOCPP_REMOTE_URL")
+
+    def _snapshot_settings(self):
+        original = {name: getattr(hub.config, name) for name in
+                    self._SETTING_KEYS}
+        self.addCleanup(lambda: [setattr(hub.config, name, value)
+                                 for name, value in original.items()])
+
+    def test_update_config_value_preserves_comments_and_other_lines(self):
         import tempfile
+        self._snapshot_settings()
         with tempfile.TemporaryDirectory() as td:
             path = Path(td) / "config.py"
             path.write_text(
@@ -1524,11 +1612,12 @@ class SettingsTests(unittest.TestCase):
                 "\n"
                 "CHUNK_SIZE = 250  # words per request\n",
                 encoding="utf-8")
-            with patch.object(hub.config, "__file__", str(path)):
-                hub._write_config({"AUDIO_FORMAT": "mp3",
-                                   "AUDIO_BITRATE": "192k",
-                                   "LANGUAGE": "Japanese",
-                                   "CHUNK_SIZE": 300})
+            for key, value in (("AUDIO_FORMAT", "mp3"),
+                               ("AUDIO_BITRATE", "192k"),
+                               ("LANGUAGE", "Japanese"),
+                               ("CHUNK_SIZE", 300)):
+                self.assertTrue(hub.common.update_config_value(
+                    key, value, config_path=path))
             text = path.read_text(encoding="utf-8")
         self.assertEqual(
             text,
@@ -1538,32 +1627,28 @@ class SettingsTests(unittest.TestCase):
             'LANGUAGE = "Japanese"\n'
             "\n"
             "CHUNK_SIZE = 300  # words per request\n")
+        # The imported module mirrors the file immediately.
+        self.assertEqual(hub.config.AUDIO_FORMAT, "mp3")
+        self.assertEqual(hub.config.CHUNK_SIZE, 300)
 
-    def test_write_config_missing_key_raises(self):
+    def test_update_config_value_missing_key_returns_false(self):
         import tempfile
+        self._snapshot_settings()
         with tempfile.TemporaryDirectory() as td:
             path = Path(td) / "config.py"
             path.write_text("X = 1\n", encoding="utf-8")
-            with patch.object(hub.config, "__file__", str(path)):
-                with self.assertRaises(ValueError):
-                    hub._write_config({"AUDIO_FORMAT": "mp3"})
+            self.assertFalse(hub.common.update_config_value(
+                "AUDIO_FORMAT", "mp3", config_path=path))
 
     def test_apply_settings_writes_and_reloads_in_memory(self):
         written = {}
 
-        def fake_write(updates):
-            written.update(updates)
+        def fake_update(key, value, config_path=None):
+            written[key] = value
+            setattr(hub.config, key, value)
+            return True
 
-        original = {name: getattr(hub.config, name) for name in
-                    ("AUDIO_FORMAT", "AUDIO_BITRATE", "LANGUAGE",
-                     "CHUNK_SIZE", "STOP_SERVER_AND_EXIT",
-                     "AUDIOCPP_UNLOAD_MODELS",
-                     "QWEN_API_URL", "CLONE_API_URL",
-                     "FASTER_API_URL", "AUDIOCPP_API_URL",
-                     "QWEN_REMOTE_URL", "CLONE_REMOTE_URL",
-                     "FASTER_REMOTE_URL", "AUDIOCPP_REMOTE_URL")}
-        self.addCleanup(lambda: [setattr(hub.config, name, value)
-                                 for name, value in original.items()])
+        self._snapshot_settings()
         values = {"audio_format": "ogg", "audio_bitrate": " 192k ",
                   "language": "en", "chunk_size": "300",
                   "stop_and_exit": False,
@@ -1574,7 +1659,8 @@ class SettingsTests(unittest.TestCase):
                   "faster_remote_url": "http://10.0.0.6:8000",
                   "qwen_custom_remote_url": "",
                   "qwen_clone_remote_url": ""}
-        with patch.object(hub, "_write_config", fake_write), \
+        with patch.object(hub.common, "update_config_value",
+                          fake_update), \
                 patch.object(hub, "_sync_audiocpp_server_port"):
             hub._apply_settings(values)
         # Values are trimmed and language normalized to a display name;
@@ -1609,23 +1695,14 @@ class SettingsTests(unittest.TestCase):
                          "http://10.0.0.5:8080")
 
     def test_apply_settings_rejects_bad_values(self):
-        original = {name: getattr(hub.config, name) for name in
-                    ("AUDIO_FORMAT", "AUDIO_BITRATE", "LANGUAGE",
-                     "CHUNK_SIZE", "STOP_SERVER_AND_EXIT",
-                     "AUDIOCPP_UNLOAD_MODELS",
-                     "QWEN_API_URL", "CLONE_API_URL",
-                     "FASTER_API_URL", "AUDIOCPP_API_URL",
-                     "QWEN_REMOTE_URL", "CLONE_REMOTE_URL",
-                     "FASTER_REMOTE_URL", "AUDIOCPP_REMOTE_URL")}
-        self.addCleanup(lambda: [setattr(hub.config, name, value)
-                                 for name, value in original.items()])
+        self._snapshot_settings()
         base = {"audio_format": "m4b", "audio_bitrate": "128k",
                 "language": "English", "chunk_size": "250",
                 "stop_and_exit": True,
                 "unload_models": True,
                 "qwen_custom_port": "7860", "qwen_clone_port": "7861",
                 "faster_port": "8000", "audiocpp_port": "8080"}
-        with patch.object(hub, "_write_config") as mk_write:
+        with patch.object(hub.common, "update_config_value") as mk_update:
             with self.assertRaises(ValueError):
                 hub._apply_settings({**base, "language": "Klingon"})
             with self.assertRaises(ValueError):
@@ -1635,7 +1712,7 @@ class SettingsTests(unittest.TestCase):
             with self.assertRaises(ValueError):
                 hub._apply_settings({**base,
                                      "audiocpp_remote_url": "not a url"})
-            mk_write.assert_not_called()
+            mk_update.assert_not_called()
 
     def test_field_validators(self):
         self.assertIsNone(hub._validate_bitrate("128k"))
@@ -1831,7 +1908,8 @@ class SettingsTests(unittest.TestCase):
                 'FASTER_REMOTE_URL = "http://127.0.0.1:8000"\n'
                 'AUDIOCPP_REMOTE_URL = "http://127.0.0.1:8080"\n',
                 encoding="utf-8")
-            with patch.object(hub.config, "__file__", str(path)):
+            with patch.object(hub.common, "CONFIG_PATH", path), \
+                    patch.object(hub, "_sync_audiocpp_server_port"):
                 # Down to Chunk size, Enter -> editor, Ctrl-U + '300',
                 # Enter; Tab -> Save, Enter; a key dismisses the flash.
                 screen = FakeScreen(keys=[
@@ -1860,14 +1938,13 @@ class SettingsTests(unittest.TestCase):
                 'FASTER_API_URL = "http://127.0.0.1:8000"\n'
                 'AUDIOCPP_API_URL = "http://127.0.0.1:8080"\n',
                 encoding="utf-8")
-            with patch.object(hub.config, "__file__", str(path)), \
-                    patch.object(hub, "_sync_audiocpp_server_port"):
-                hub._write_config({
-                    "QWEN_API_URL": "http://127.0.0.1:7862",
-                    "CLONE_API_URL": "http://127.0.0.1:7863",
-                    "FASTER_API_URL": "http://127.0.0.1:8001",
-                    "AUDIOCPP_API_URL": "http://127.0.0.1:8081",
-                })
+            self._snapshot_settings()
+            for key, value in (
+                    ("QWEN_API_URL", "http://127.0.0.1:7862"),
+                    ("CLONE_API_URL", "http://127.0.0.1:7863"),
+                    ("FASTER_API_URL", "http://127.0.0.1:8001"),
+                    ("AUDIOCPP_API_URL", "http://127.0.0.1:8081")):
+                hub.common.update_config_value(key, value, config_path=path)
             text = path.read_text(encoding="utf-8")
         self.assertIn('QWEN_API_URL = "http://127.0.0.1:7862"', text)
         self.assertIn('CLONE_API_URL = "http://127.0.0.1:7863"', text)