refactor(settings): drop persisted diff/patch helpers

Co-authored-by: openhands <openhands@all-hands.dev>

Author: openhands-agent

openhands-agent-server/openhands/agent_server/settings_router.py

@@ -18,7 +18,7 @@ def _get_conversation_settings_schema() -> SettingsSchema:
     return ConversationSettings.export_schema()
 
 
-@settings_router.get("/schema", response_model=SettingsSchema)
+@settings_router.get("/agent-schema", response_model=SettingsSchema)
 async def get_agent_settings_schema() -> SettingsSchema:
     """Return the schema used to render AgentSettings-based settings forms."""
     return _get_agent_settings_schema()

openhands-sdk/openhands/sdk/settings/model.py

@@ -225,11 +225,9 @@ def _default_llm_settings() -> LLM:
     return LLM(model=model)
 
 
-# Canonical persisted AgentSettings payload contract:
-# - v1 (legacy): flat dotted OpenHands mapping or unversioned nested payload
-# - v2 (current): standard AgentSettings JSON including `schema_version`
+# Persisted settings payloads currently use schema_version 1.
 _LEGACY_AGENT_SETTINGS_VERSION = 1
-_CURRENT_AGENT_SETTINGS_VERSION = 2
+_CURRENT_AGENT_SETTINGS_VERSION = 1
 _PERSISTED_AGENT_SETTINGS_VERSION_KEY = "schema_version"
 _LEGACY_WRAPPED_SETTINGS_VERSION_KEY = "version"
 _LEGACY_WRAPPED_SETTINGS_SETTINGS_KEY = "settings"
@@ -469,38 +467,6 @@ def export_schema(cls) -> SettingsSchema:
         """Export a structured schema describing configurable conversation settings."""
         return export_settings_schema(cls)
 
-    @classmethod
-    def migrate_persisted_payload(cls, payload: Mapping[str, Any]) -> dict[str, Any]:
-        """Return the latest canonical persisted ConversationSettings payload."""
-        return _migrate_persisted_conversation_settings_payload(payload)
-
-    @classmethod
-    def from_persisted(cls, payload: Mapping[str, Any]) -> ConversationSettings:
-        """Load persisted ConversationSettings after applying SDK-owned migrations."""
-        return cls.model_validate(cls.migrate_persisted_payload(payload))
-
-    def patch(self, payload: Mapping[str, Any]) -> ConversationSettings:
-        """Return a new settings object with a persisted patch applied."""
-        base_payload = self.model_dump(mode="json")
-        merged_payload = _merge_patch_payload(
-            base_payload, _normalize_patch_payload(payload)
-        )
-        merged_payload[_PERSISTED_AGENT_SETTINGS_VERSION_KEY] = (
-            self.CURRENT_PERSISTED_VERSION
-        )
-        return type(self).from_persisted(merged_payload)
-
-    def diff(self, target: ConversationSettings | Mapping[str, Any]) -> dict[str, Any]:
-        """Return the minimal persisted patch from these settings to ``target``."""
-        target_settings = (
-            target
-            if isinstance(target, ConversationSettings)
-            else type(self).from_persisted(target)
-        )
-        base_payload = self.model_dump(mode="json")
-        target_payload = target_settings.model_dump(mode="json")
-        return _diff_payload(base_payload, target_payload)
-
     def build_confirmation_policy(self):
         from openhands.sdk.security.confirmation_policy import (
             AlwaysConfirm,
@@ -620,44 +586,6 @@ def export_schema(cls) -> SettingsSchema:
         """Export a structured schema describing configurable agent settings."""
         return export_settings_schema(cls)
 
-    @classmethod
-    def migrate_persisted_payload(cls, payload: Mapping[str, Any]) -> dict[str, Any]:
-        """Return the latest canonical persisted AgentSettings payload."""
-        return _migrate_persisted_agent_settings_payload(payload)
-
-    @classmethod
-    def from_persisted(cls, payload: Mapping[str, Any]) -> AgentSettings:
-        """Load persisted AgentSettings after applying SDK-owned migrations."""
-        return cls.model_validate(cls.migrate_persisted_payload(payload))
-
-    def patch(self, payload: Mapping[str, Any]) -> AgentSettings:
-        """Return a new settings object with a persisted patch applied.
-
-        The patch payload may be sparse and may use either the current nested
-        representation or the legacy flat dotted representation.
-        """
-        base_payload = self.model_dump(mode="json", context={"expose_secrets": True})
-        merged_payload = _merge_patch_payload(
-            base_payload, _normalize_patch_payload(payload)
-        )
-        merged_payload[_PERSISTED_AGENT_SETTINGS_VERSION_KEY] = (
-            self.CURRENT_PERSISTED_VERSION
-        )
-        return type(self).from_persisted(merged_payload)
-
-    def diff(self, target: AgentSettings | Mapping[str, Any]) -> dict[str, Any]:
-        """Return the minimal persisted patch from these settings to ``target``."""
-        target_settings = (
-            target
-            if isinstance(target, AgentSettings)
-            else type(self).from_persisted(target)
-        )
-        base_payload = self.model_dump(mode="json", context={"expose_secrets": True})
-        target_payload = target_settings.model_dump(
-            mode="json", context={"expose_secrets": True}
-        )
-        return _diff_payload(base_payload, target_payload)
-
     def create_agent(self) -> Agent:
         """Build an :class:`Agent` purely from these settings.
 

tests/agent_server/test_settings_router.py

@@ -7,7 +7,7 @@
 def test_get_agent_settings_schema():
     client = TestClient(create_app(Config(static_files_path=None, session_api_keys=[])))
 
-    response = client.get("/api/settings/schema")
+    response = client.get("/api/settings/agent-schema")
 
     assert response.status_code == 200
     body = response.json()

tests/sdk/test_settings.py

@@ -114,7 +114,6 @@ def test_agent_settings_export_schema_groups_sections() -> None:
     assert "verification.security_analyzer" not in v_fields
 
 
-
 def test_conversation_settings_export_schema_groups_sections() -> None:
     schema = ConversationSettings.export_schema()
 
@@ -140,36 +139,25 @@ def test_conversation_settings_export_schema_groups_sections() -> None:
     )
     assert verification_fields["verification.security_analyzer"].default == "llm"
     assert (
-        verification_fields["verification.security_analyzer"].choices[0].value
-        == "llm"
+        verification_fields["verification.security_analyzer"].choices[0].value == "llm"
     )
     assert verification_fields["verification.security_analyzer"].depends_on == [
         "verification.confirmation_mode"
     ]
 
 
-def test_conversation_settings_patch_and_diff_roundtrip() -> None:
-    defaults = ConversationSettings()
-    updated = defaults.patch(
-        {
-            "max_iterations": 42,
-            "verification": {
-                "confirmation_mode": True,
-                "security_analyzer": "none",
-            },
-        }
+def test_conversation_settings_model_dump_roundtrip() -> None:
+    settings = ConversationSettings(
+        max_iterations=42,
+        verification=ConversationVerificationSettings(
+            confirmation_mode=True,
+            security_analyzer="none",
+        ),
     )
 
-    patch = defaults.diff(updated)
+    restored = ConversationSettings.model_validate(settings.model_dump(mode="json"))
 
-    assert patch == {
-        "max_iterations": 42,
-        "verification": {
-            "confirmation_mode": True,
-            "security_analyzer": "none",
-        },
-    }
-    assert defaults.patch(patch) == updated
+    assert restored == settings
 
 
 def test_conversation_settings_builds_runtime_helpers() -> None:
@@ -187,8 +175,12 @@ def test_conversation_settings_builds_runtime_helpers() -> None:
     assert isinstance(request_kwargs["confirmation_policy"], ConfirmRisky)
     assert isinstance(request_kwargs["security_analyzer"], LLMSecurityAnalyzer)
 
-    fallback_settings = settings.patch(
-        {"verification": {"security_analyzer": "none"}}
+    fallback_settings = ConversationSettings(
+        max_iterations=settings.max_iterations,
+        verification=ConversationVerificationSettings(
+            confirmation_mode=True,
+            security_analyzer="none",
+        ),
     )
     fallback_request_kwargs = fallback_settings.to_start_request_kwargs()
 
@@ -331,78 +323,44 @@ def test_roundtrip_preserves_llm_model() -> None:
     assert restored.llm.model == "test-model"
 
 
-def test_from_persisted_agent_settings_migrates_legacy_flat_payload() -> None:
-    legacy_payload = {
-        "schema_version": 1,
-        "llm.model": "legacy-model",
-        "llm.api_key": "secret-key",
-        "verification.critic_enabled": True,
-    }
-
-    settings = AgentSettings.from_persisted(legacy_payload)
+def test_agent_settings_model_validate_accepts_nested_payload() -> None:
+    settings = AgentSettings.model_validate(
+        {
+            "schema_version": AgentSettings.CURRENT_PERSISTED_VERSION,
+            "llm": {
+                "model": "test-model",
+                "api_key": "secret-key",
+            },
+            "verification": {"critic_enabled": True},
+        }
+    )
 
     assert settings.schema_version == AgentSettings.CURRENT_PERSISTED_VERSION
-    assert settings.llm.model == "legacy-model"
+    assert settings.llm.model == "test-model"
     assert isinstance(settings.llm.api_key, SecretStr)
     assert settings.llm.api_key.get_secret_value() == "secret-key"
     assert settings.verification.critic_enabled is True
 
 
-def test_from_persisted_agent_settings_accepts_current_payload() -> None:
-    current_payload = {
-        "schema_version": AgentSettings.CURRENT_PERSISTED_VERSION,
-        "llm": {"model": "test-model"},
-    }
-
-    settings = AgentSettings.from_persisted(current_payload)
-
-    assert settings.schema_version == AgentSettings.CURRENT_PERSISTED_VERSION
-    assert settings.llm.model == "test-model"
-
-
-def test_from_persisted_agent_settings_accepts_legacy_wrapped_payload() -> None:
-    wrapped_payload = {
-        "version": 2,
-        "settings": {
-            "llm": {"model": "wrapped-model"},
-        },
-    }
-
-    settings = AgentSettings.from_persisted(wrapped_payload)
-
-    assert settings.schema_version == AgentSettings.CURRENT_PERSISTED_VERSION
-    assert settings.llm.model == "wrapped-model"
-
-
-def test_agent_settings_patch_accepts_sparse_nested_payload() -> None:
-    settings = AgentSettings.from_persisted(
+def test_agent_settings_model_dump_roundtrip_preserves_sparse_updates() -> None:
+    settings = AgentSettings.model_validate(
         {
             "schema_version": AgentSettings.CURRENT_PERSISTED_VERSION,
             "llm": {"model": "base-model", "base_url": "https://example.com"},
         }
     )
 
-    patched = settings.patch({"llm": {"base_url": None}})
-
-    assert patched.llm.model == "base-model"
-    assert patched.llm.base_url is None
-    assert patched.schema_version == AgentSettings.CURRENT_PERSISTED_VERSION
-
-
-def test_agent_settings_diff_returns_sparse_persisted_patch() -> None:
-    org_settings = AgentSettings.from_persisted(
-        {
-            "schema_version": AgentSettings.CURRENT_PERSISTED_VERSION,
-            "llm": {"model": "base-model", "base_url": "https://example.com"},
+    updated = settings.model_copy(
+        update={
+            "llm": settings.llm.model_copy(
+                update={"model": "personal-model", "base_url": None}
+            )
         }
     )
-    personal_settings = org_settings.patch(
-        {"llm": {"model": "personal-model", "base_url": None}}
+    restored = AgentSettings.model_validate(
+        updated.model_dump(mode="json", context={"expose_secrets": True})
     )
 
-    member_patch = org_settings.diff(personal_settings)
-
-    assert member_patch == {"llm": {"base_url": None, "model": "personal-model"}}
-    assert org_settings.patch(member_patch).model_dump(
-        mode="json", context={"expose_secrets": True}
-    ) == personal_settings.model_dump(mode="json", context={"expose_secrets": True})
+    assert restored.llm.model == "personal-model"
+    assert restored.llm.base_url is None
+    assert restored.schema_version == AgentSettings.CURRENT_PERSISTED_VERSION