优化sync_system代码质量

tests/unit/test_api_client_rate_limit.py

@@ -1,9 +1,11 @@
 from __future__ import annotations
 
 import asyncio
+import logging
 import time
 
 import pytest
+import httpx
 
 from sync_state_machine.datasource.api.client import ApiClient
 
@@ -21,11 +23,28 @@ class _FakeResponse:
 class _FakeAsyncClient:
     def __init__(self) -> None:
         self.call_times: list[float] = []
+        self.closed = False
 
     async def request(self, **kwargs):
         self.call_times.append(time.monotonic())
         return _FakeResponse()
 
+    async def aclose(self) -> None:
+        self.closed = True
+
+
+class _FailingAsyncClient:
+    def __init__(self) -> None:
+        self.closed = False
+
+    async def request(self, **kwargs):
+        request = httpx.Request(kwargs["method"], kwargs["url"])
+        response = httpx.Response(500, request=request, text="boom")
+        raise httpx.HTTPStatusError("server error", request=request, response=response)
+
+    async def aclose(self) -> None:
+        self.closed = True
+
 
 @pytest.mark.asyncio
 async def test_api_client_global_rate_limit_applies_to_concurrent_requests() -> None:
@@ -48,3 +67,50 @@ async def test_api_client_global_rate_limit_applies_to_concurrent_requests() ->
     assert len(fake_client.call_times) == 3
     third_delay = fake_client.call_times[2] - fake_client.call_times[0]
     assert third_delay >= 0.15
+
+
+@pytest.mark.asyncio
+async def test_api_client_request_stats_track_success_and_failure(caplog: pytest.LogCaptureFixture) -> None:
+    client = ApiClient(
+        base_url="http://example.com",
+        uid="uid",
+        secret="secret",
+    )
+    logger_name = "sync_state_machine.datasource.api.client"
+    success_client = _FakeAsyncClient()
+    client._client = success_client
+
+    with caplog.at_level(logging.INFO, logger=logger_name):
+        await client.request("GET", "/ping")
+
+        failing_client = _FailingAsyncClient()
+        client._client = failing_client
+        with pytest.raises(httpx.HTTPStatusError):
+            await client.request("GET", "/ping")
+
+        stats = client.get_request_stats()
+        assert stats == {"total": 2, "success": 1, "failure": 1}
+        assert client.format_request_stats().endswith("total=2, success=1, failure=1")
+
+        await client.close()
+
+    assert failing_client.closed is True
+    assert "📊 ApiClient request stats: total=2, success=1, failure=1" in caplog.text
+
+
+@pytest.mark.asyncio
+async def test_api_client_logs_one_line_request_summary_in_debug_mode(caplog: pytest.LogCaptureFixture) -> None:
+    client = ApiClient(
+        base_url="http://example.com",
+        uid="uid",
+        secret="secret",
+        debug=True,
+    )
+    logger_name = "sync_state_machine.datasource.api.client"
+    fake_client = _FakeAsyncClient()
+    client._client = fake_client
+
+    with caplog.at_level(logging.DEBUG, logger=logger_name):
+        await client.request("GET", "/ping")
+
+    assert "ApiClient request: method=GET endpoint=/ping elapsed=" in caplog.text

tests/unit/test_pipeline_strategy_continue_on_error.py

@@ -10,14 +10,14 @@ from sync_state_machine.common.persistence import PersistenceBackend
 from sync_state_machine.common.sqlite_backend import SQLitePersistenceBackend
 from sync_state_machine.datasource.datasource import BaseDataSource
 from sync_state_machine.pipeline.full_sync_pipeline import FullSyncPipeline
-from sync_state_machine.sync_system.strategy import BaseSyncStrategy
+from sync_state_machine.sync_system.strategy import DefaultSyncStrategy
 
 
 class _DS(BaseDataSource):
     pass
 
 
-class _FailStrategy(BaseSyncStrategy):
+class _FailStrategy(DefaultSyncStrategy):
     schema = object  # type: ignore
 
     async def bind(self, **kwargs):
@@ -30,7 +30,7 @@ class _FailStrategy(BaseSyncStrategy):
         return []
 
 
-class _OkStrategy(BaseSyncStrategy):
+class _OkStrategy(DefaultSyncStrategy):
     schema = object  # type: ignore
 
     def __init__(self, *args, **kwargs):

tests/unit/test_project_api_handler_dangerous_delete_config.py

@@ -1,57 +0,0 @@
-from __future__ import annotations
-
-import pytest
-
-from sync_state_machine.common.collection import DataCollection
-from sync_state_machine.domain.project.api_handler import ProjectApiHandler
-
-
-class _FakeProjectResponse:
-    def __init__(self, project_id: str):
-        self.id = project_id
-
-    def model_dump(self) -> dict[str, str]:
-        return {"id": self.id, "name": f"project-{self.id}"}
-
-
-@pytest.mark.asyncio
-async def test_project_handler_purge_project_data_requires_data_id_filter() -> None:
-    handler = ProjectApiHandler()
-    handler.set_handler_config({"purge_project_data": True})
-
-    with pytest.raises(ValueError, match="requires non-empty data_id_filter"):
-        await handler.load()
-
-
-@pytest.mark.asyncio
-async def test_project_handler_purge_project_data_calls_remote_endpoint(monkeypatch: pytest.MonkeyPatch) -> None:
-    handler = ProjectApiHandler()
-    handler.set_handler_config(
-        {
-            "purge_project_data": True,
-            "data_id_filter": ["project-1"],
-        }
-    )
-    handler.api_client = object()
-    handler.set_collection(DataCollection("remote"))
-
-    purged: list[str] = []
-
-    async def fake_purge(client, project_id: str):
-        purged.append(project_id)
-        return {"project_id": project_id}
-
-    async def fake_get_project(client, project_id: str):
-        return _FakeProjectResponse(project_id)
-
-    async def fake_get_all_info(cls, client, project_id: str):
-        return {}
-
-    monkeypatch.setattr("sync_state_machine.domain.project.api_handler.api_purge_project_data", fake_purge)
-    monkeypatch.setattr("sync_state_machine.domain.project.api_handler.api_get_project", fake_get_project)
-    monkeypatch.setattr(ProjectApiHandler, "get_all_info", classmethod(fake_get_all_info))
-
-    nodes = await handler.load()
-
-    assert purged == ["project-1"]
-    assert [node.data_id for node in nodes] == ["project-1"]

tests/unit/test_project_api_handler_normalization.py

@@ -1,7 +1,10 @@
 from __future__ import annotations
 
+import pytest
 from uuid import uuid4
 
+from sync_state_machine.common.collection import DataCollection
+from sync_state_machine.domain.project.api_handler import ProjectApiHandler
 from sync_state_machine.domain.project.api_handler import _merge_project_all_info_fields
 from sync_state_machine.domain.project.sync_node import ProjectSyncNode
 
@@ -82,3 +85,12 @@ def test_project_sync_node_preserves_extension_backed_fields() -> None:
     assert normalized["implementation_estimate"] == 120.0
     assert normalized["static_total_investment"] == 110.0
     assert normalized["completed_investment"] == 70.0
+
+
+@pytest.mark.asyncio
+async def test_project_api_handler_load_requires_api_client() -> None:
+    handler = ProjectApiHandler()
+    handler.set_collection(DataCollection("remote"))
+
+    with pytest.raises(RuntimeError, match="API client not set"):
+        await handler.load()

tests/unit/test_project_detail_sync_strategy.py

@@ -0,0 +1,68 @@
+from types import SimpleNamespace
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+
+from sync_state_machine.domain.project_detail.sync_strategy import ProjectDetailSyncStrategy
+from sync_state_machine.sync_system.config import UpdateDirection
+from sync_state_machine.sync_system.strategy_ops import BoundNodePair
+
+
+def _fake_node(*, node_id: str, key: str):
+    data = {"id": node_id, "key": key}
+    return SimpleNamespace(
+        node_id=node_id,
+        data_id=node_id,
+        action=None,
+        context={},
+        get_data=lambda: data,
+    )
+
+
+@pytest.mark.asyncio
+async def test_project_detail_update_partitions_group_production_plans_before_generic_logic():
+    strategy = ProjectDetailSyncStrategy("project_detail_data", MagicMock(), MagicMock(), MagicMock())
+
+    pull_pair = BoundNodePair(
+        local_node=_fake_node(node_id="local-pull", key="ensure_production"),
+        remote_node=_fake_node(node_id="remote-pull", key="ensure_production"),
+    )
+    generic_pair = BoundNodePair(
+        local_node=_fake_node(node_id="local-generic", key="production"),
+        remote_node=_fake_node(node_id="remote-generic", key="production"),
+    )
+    pull_node = SimpleNamespace(node_id="pull-update")
+    generic_node = SimpleNamespace(node_id="generic-update")
+
+    with patch.object(
+        strategy,
+        "collect_update_pairs",
+        new=AsyncMock(return_value=[pull_pair, generic_pair]),
+    ), patch(
+        "sync_state_machine.domain.project_detail.sync_strategy.build_data_id_normalization_map",
+        new=AsyncMock(return_value={}),
+    ), patch.object(
+        strategy,
+        "prepare_update_for_direction",
+        new=AsyncMock(return_value=pull_node),
+    ) as mock_prepare, patch.object(
+        strategy,
+        "update_pair",
+        new=AsyncMock(return_value=[generic_node]),
+    ) as mock_update_pair:
+        updated_nodes = await strategy.update()
+
+    assert updated_nodes == [pull_node, generic_node]
+    mock_prepare.assert_awaited_once_with(
+        pull_pair.local_node,
+        pull_pair.remote_node,
+        UpdateDirection.PULL,
+        {},
+    )
+    mock_update_pair.assert_awaited_once_with(generic_pair.local_node, generic_pair.remote_node, {})
+
+
+def test_project_detail_domain_option_is_typed_schema() -> None:
+    strategy = ProjectDetailSyncStrategy("project_detail_data", MagicMock(), MagicMock(), MagicMock())
+
+    assert strategy.domain_option.pull_group_production_plans is True

tests/unit/test_project_sync_strategy.py

@@ -0,0 +1,68 @@
+from types import SimpleNamespace
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+
+from sync_state_machine.domain.project.sync_strategy import ProjectSyncStrategy
+from sync_state_machine.sync_system.config import UpdateDirection
+
+
+def _fake_node(*, node_id: str, data_id: str, data: dict, context=None):
+    return SimpleNamespace(
+        node_id=node_id,
+        data_id=data_id,
+        data=data,
+        context=context or {},
+        action=None,
+        get_data=lambda: data,
+    )
+
+
+def test_project_get_node_update_payload_enriches_group_plan_dates_from_all_info():
+    strategy = ProjectSyncStrategy("project", MagicMock(), MagicMock(), MagicMock())
+    node = _fake_node(
+        node_id="node-1",
+        data_id="project-1",
+        data={"id": "project-1", "project_name": "demo"},
+        context={
+            "all_info": {
+                "wind_extension": {
+                    "ic_plan_first_production_date": "2026-01-01",
+                    "ic_plan_full_production_date": "2026-06-01",
+                }
+            }
+        },
+    )
+
+    payload = strategy.get_node_update_payload(node)
+
+    assert payload["ic_plan_first_production_date"] == "2026-01-01"
+    assert payload["ic_plan_full_production_date"] == "2026-06-01"
+
+
+@pytest.mark.asyncio
+async def test_project_update_pair_runs_special_pull_then_generic_update():
+    strategy = ProjectSyncStrategy("project", MagicMock(), MagicMock(), MagicMock())
+    strategy.config.domain_option = {"pull_group_plan_production_date": True}
+
+    local_node = _fake_node(node_id="local", data_id="local-1", data={"id": "local-1"})
+    remote_node = _fake_node(node_id="remote", data_id="remote-1", data={"id": "remote-1"})
+    pull_node = SimpleNamespace(node_id="pull-node")
+    push_node = SimpleNamespace(node_id="push-node")
+
+    with patch.object(
+        strategy,
+        "prepare_update_for_direction",
+        new=AsyncMock(side_effect=[pull_node, push_node]),
+    ) as mock_prepare:
+        updated_nodes = await strategy.update_pair(local_node, remote_node, {"project": "remote-project"})
+
+    assert updated_nodes == [pull_node, push_node]
+
+    pull_call = mock_prepare.await_args_list[0]
+    assert pull_call.args == (local_node, remote_node, UpdateDirection.PULL, {"project": "remote-project"})
+    assert pull_call.kwargs["include_fields"] == strategy._PULL_ONLY_FIELDS
+
+    generic_call = mock_prepare.await_args_list[1]
+    assert generic_call.args == (local_node, remote_node, UpdateDirection.PUSH, {"project": "remote-project"})
+    assert generic_call.kwargs["exclude_fields"] == strategy._PULL_ONLY_FIELDS

tests/unit/test_run_presets_yaml_loader.py

@@ -176,7 +176,9 @@ strategies:
 
     snapshot = config_snapshot(config)
     assert snapshot["strategies"][0]["config"]["compare_ignore_fields"] == []
-    assert snapshot["strategies"][0]["config"]["field_direction_overrides"] == {}
+    assert snapshot["strategies"][0]["config"]["domain_option"] == {
+      "pull_group_plan_production_date": False
+    }
 
 
 def test_build_config_from_file_rejects_legacy_strategy_keys(tmp_path: Path) -> None:

tests/unit/test_strategy_runtime_config.py

@@ -1,14 +1,21 @@
 from __future__ import annotations
 
+from pydantic import BaseModel
+
 from sync_state_machine.config.builder import _apply_strategy_overrides
-from sync_state_machine.config.strategy_config import StrategyConfig, StrategyRuntimeConfig
+from sync_state_machine.config.strategy_config import (
+    StrategyConfig,
+    StrategyRuntimeConfig,
+    resolve_domain_option_config,
+)
 
 
 def test_strategy_runtime_config_skip_sync_defaults_false() -> None:
     runtime_config = StrategyRuntimeConfig(node_type="project")
 
-    assert runtime_config.skip_sync is False
-    assert runtime_config.skip_post_check is False
+    assert runtime_config.config.skip_sync is False
+    assert runtime_config.config.skip_post_check is False
+    assert runtime_config.config.domain_option == {}
 
 
 def test_apply_strategy_overrides_allows_skip_sync_override() -> None:
@@ -24,8 +31,34 @@ def test_apply_strategy_overrides_allows_skip_sync_override() -> None:
 
     assert len(resolved) == 1
     assert resolved[0].node_type == "project"
-    assert resolved[0].skip_sync is True
-    assert resolved[0].skip_post_check is True
+    assert resolved[0].config.skip_sync is True
+    assert resolved[0].config.skip_post_check is True
+
+
+def test_apply_strategy_overrides_allows_domain_option_override() -> None:
+    resolved = _apply_strategy_overrides(
+        node_types=["project"],
+        strategy_overrides={
+            "project": {
+                "domain_option": {
+                    "pull_group_plan_production_date": True,
+                }
+            }
+        },
+    )
+
+    assert resolved[0].config.domain_option == {"pull_group_plan_production_date": True}
+
+
+def test_apply_strategy_overrides_preserves_project_detail_default_domain_option() -> None:
+    resolved = _apply_strategy_overrides(
+        node_types=["project_detail_data"],
+        strategy_overrides={},
+    )
+
+    assert resolved[0].config.domain_option == {
+        "pull_group_production_plans": True,
+    }
 
 
 def test_strategy_config_supports_both_post_check_ignore_fields() -> None:
@@ -39,4 +72,20 @@ def test_strategy_config_supports_both_post_check_ignore_fields() -> None:
 
     assert config.post_check_ignore_fields == ["code"]
     assert config.post_check_ignore_list_item_fields == {"plan_node": ["is_audit"]}
-    assert config.allow_many_to_one_auto_bind is True
+    assert config.allow_many_to_one_auto_bind is True
+
+
+def test_resolve_domain_option_config_fills_schema_defaults() -> None:
+    class ExampleDomainOption(BaseModel):
+        enabled: bool = True
+        mode: str = "strict"
+
+    resolved = resolve_domain_option_config(
+        ExampleDomainOption,
+        {"enabled": False},
+    )
+
+    assert resolved == {
+        "enabled": False,
+        "mode": "strict",
+    }

tests/unit/test_update_ops.py

@@ -1,74 +1,78 @@
 import pytest
+from types import SimpleNamespace
 from unittest.mock import AsyncMock, patch, MagicMock
 from sync_state_machine.domain.company.sync_strategy import CompanySyncStrategy
-from sync_state_machine.sync_system.config import UpdateDirection
-from sync_state_machine.sync_system.strategy_ops.update_ops import run_update
+from sync_state_machine.sync_system.strategy_ops.update_ops import BoundNodePair, build_merged_update_payload, run_update
 from sync_state_machine.domain.construction.sync_strategy import ConstructionTaskSyncStrategy
 from sync_state_machine.domain.project_detail.sync_strategy import ProjectDetailSyncStrategy
 from schemas.project_extensions.project_detail import ProjectDetailKey
 
 @pytest.mark.asyncio
-async def test_run_update_with_direction_overrides():
-    class FakeNode:
-        def __init__(self, data):
-            self._data = data
+async def test_run_update_delegates_to_strategy_pair_planner():
+    local_node = SimpleNamespace(node_id="local-1")
+    remote_node = SimpleNamespace(node_id="remote-1")
 
-        def get_data(self):
-            return self._data
-
-    # Setup mock strategy
     strategy = MagicMock()
     strategy.node_type = "TestNode"
-    strategy.config.field_direction_key = "sync_direction"
-    strategy.config.field_direction_overrides = {
-        "local_only": UpdateDirection.PUSH,
-        "remote_only": UpdateDirection.PULL,
-        "ignore": UpdateDirection.BIDIRECTIONAL # Just to check filtering
-    }
-    strategy.config.update_direction = UpdateDirection.PUSH
     strategy.local_collection = MagicMock()
     strategy.remote_collection = MagicMock()
+    strategy.binding_manager = MagicMock()
+    strategy.reset_schema_diff_validator = MagicMock()
+    strategy.emit_schema_diff_report = MagicMock()
+    updated_node = SimpleNamespace(node_id="target-1")
+    strategy.update_pair = AsyncMock(return_value=[updated_node])
 
-    with patch('sync_state_machine.sync_system.strategy_ops.update_ops.add_update_action', new_callable=AsyncMock) as mock_add_action:
-        mock_add_action.return_value = []
-        
-        await run_update(strategy)
-        
-        # In the active directions, PUSH and PULL should be handled because of overrides.
-        # Check PUSH call
-        assert mock_add_action.call_count == 2
-        
-        push_call = mock_add_action.call_args_list[0]
-        assert push_call.kwargs['source_is_local'] is True
-        assert push_call.kwargs['from_collection'] == strategy.local_collection
-        
-        pull_call = mock_add_action.call_args_list[1]
-        assert pull_call.kwargs['source_is_local'] is False
-        assert pull_call.kwargs['from_collection'] == strategy.remote_collection
-        
-        # Test the node filters
-        push_filter = push_call.kwargs['node_filter']
-        pull_filter = pull_call.kwargs['node_filter']
+    with patch(
+        'sync_state_machine.sync_system.strategy_ops.update_ops.build_data_id_normalization_map',
+        new=AsyncMock(return_value={"project": "remote-project"}),
+    ), patch(
+        'sync_state_machine.sync_system.strategy_ops.update_ops.collect_bound_node_pairs',
+        new=AsyncMock(return_value=[BoundNodePair(local_node=local_node, remote_node=remote_node)]),
+    ):
+        updated = await run_update(strategy)
 
-        # Node that has 'local_only' -> PUSH
-        node_push = FakeNode({"sync_direction": "local_only"})
-        assert push_filter(node_push) is True
-        assert pull_filter(node_push) is False
+    assert updated == [updated_node]
 
-        # Node that has 'remote_only' -> PULL
-        node_pull = FakeNode({"sync_direction": "remote_only"})
-        assert push_filter(node_pull) is False
-        assert pull_filter(node_pull) is True
+    strategy.update_pair.assert_awaited_once_with(
+        local_node,
+        remote_node,
+        {"project": "remote-project"},
+    )
 
-        # Node with unknown key -> default direction (PUSH)
-        node_default = FakeNode({"sync_direction": "unknown"})
-        assert push_filter(node_default) is True
-        assert pull_filter(node_default) is False
 
-        # Node with empty data -> default direction (PUSH)
-        node_empty = FakeNode(None)
-        assert push_filter(node_empty) is True
-        assert pull_filter(node_empty) is False
+def test_build_merged_update_payload_supports_include_and_exclude_fields():
+    source_data = {
+        "id": "remote-1",
+        "project_name": "new-name",
+        "special": "from-source",
+    }
+    target_data = {
+        "id": "remote-1",
+        "project_name": "old-name",
+        "special": "from-target",
+    }
+
+    include_only = build_merged_update_payload(
+        source_data=source_data,
+        target_data=target_data,
+        include_fields=["special"],
+    )
+    exclude_special = build_merged_update_payload(
+        source_data=source_data,
+        target_data=target_data,
+        exclude_fields=["special"],
+    )
+
+    assert include_only == {
+        "id": "remote-1",
+        "project_name": "old-name",
+        "special": "from-source",
+    }
+    assert exclude_special == {
+        "id": "remote-1",
+        "project_name": "new-name",
+        "special": "from-target",
+    }
 
 
 def test_construction_task_compare_payload_ignores_plan_node_is_audit():