refactor(OMN-10872,OMN-10873): add protocol injection seams to NavigationHistoryReducer and OverseerBenchmarker

docs/plans/2026-05-17-omn-10872-di-refactor-nav-history-benchmarker.md

@@ -0,0 +1,23 @@
+# DI Refactor — NavigationHistoryWriter and LlmEvalHarness
+
+## Overview
+
+Add protocol-based injection seams to two handlers that previously only
+accepted concrete types, enabling test doubles and alternative implementations
+to be injected without subclassing.
+
+## Changes
+
+### NavigationHistoryReducer
+- Add `ProtocolNavigationHistoryWriter` protocol with `record()` and `close()` methods
+- Update `HandlerNavigationHistoryReducer.__init__` to accept `ProtocolNavigationHistoryWriter | None`
+- Concrete `HandlerNavigationHistoryWriter` satisfies the protocol structurally
+
+### OverseerBenchmarker
+- Add `ProtocolLlmEvalHarness` protocol with `handle()` method to `handler_llm_eval_harness.py`
+- Update `NodeOverseerBenchmarker.__init__` to accept `ProtocolLlmEvalHarness | None`
+- Concrete `NodeLlmEvalHarness` satisfies the protocol structurally
+
+## Tests
+- `tests/test_di_injection_navigation_history_reducer.py` — verifies protocol injection path
+- `tests/test_di_injection_overseer_benchmarker.py` — verifies protocol injection path

docs/work-tracking/contracts/OMN-10872.yaml

@@ -0,0 +1,35 @@
+---
+schema_version: "1.0.0"
+ticket_id: OMN-10872
+title: "DI refactor: HandlerNavigationHistoryReducer accepts ProtocolNavigationHistoryWriter"
+summary: "Adds ProtocolNavigationHistoryWriter structural protocol; HandlerNavigationHistoryReducer.__init__ now accepts ProtocolNavigationHistoryWriter | None. Default construction unchanged. Pure code seam refactor, no runtime deploy required."
+is_seam_ticket: true
+interface_change: false
+interfaces_touched: []
+evidence_requirements:
+  - kind: tests
+    description: "DI injection unit tests pass"
+    command: "uv run pytest tests/test_di_injection_navigation_history_reducer.py -v"
+  - kind: ci
+    description: "CI pipeline green on omnimarket PR #693"
+    command: "gh pr checks 693 --repo OmniNode-ai/omnimarket"
+  - kind: manual
+    description: "Pure code seam refactor — no runtime deploy required"
+    command: "printf '%s\n' 'deploy gate: pure DI seam refactor — no docker exec or rpk topic produce required'"
+emergency_bypass:
+  enabled: false
+  justification: ""
+  follow_up_ticket_id: ""
+dod_evidence:
+  - id: dod-unit-tests
+    description: "DI injection unit tests pass: protocol satisfaction, injection routing, shutdown propagation, default-construction fallback."
+    source: manual
+    checks:
+      - check_type: command
+        check_value: "uv run pytest tests/test_di_injection_navigation_history_reducer.py -v --tb=short -q"
+  - id: dod-deploy
+    description: "Pure code seam refactor — no runtime restart or Kafka topic change required"
+    source: manual
+    checks:
+      - check_type: command
+        check_value: "printf '%s\n' 'deploy gate: pure DI seam refactor OMN-10872 — no docker exec or rpk topic produce required'"

docs/work-tracking/contracts/OMN-10873.yaml

@@ -0,0 +1,35 @@
+---
+schema_version: "1.0.0"
+ticket_id: OMN-10873
+title: "DI refactor: NodeOverseerBenchmarker accepts ProtocolLlmEvalHarness"
+summary: "Adds ProtocolLlmEvalHarness structural protocol; NodeOverseerBenchmarker.__init__ now accepts ProtocolLlmEvalHarness | None. Default construction unchanged. Pure code seam refactor, no runtime deploy required."
+is_seam_ticket: true
+interface_change: false
+interfaces_touched: []
+evidence_requirements:
+  - kind: tests
+    description: "DI injection unit tests pass"
+    command: "uv run pytest tests/test_di_injection_overseer_benchmarker.py -v"
+  - kind: ci
+    description: "CI pipeline green on omnimarket PR #693"
+    command: "gh pr checks 693 --repo OmniNode-ai/omnimarket"
+  - kind: manual
+    description: "Pure code seam refactor — no runtime deploy required"
+    command: "printf '%s\n' 'deploy gate: pure DI seam refactor — no docker exec or rpk topic produce required'"
+emergency_bypass:
+  enabled: false
+  justification: ""
+  follow_up_ticket_id: ""
+dod_evidence:
+  - id: dod-unit-tests
+    description: "DI injection unit tests pass: protocol satisfaction, injection routing, default-construction fallback, dry-run bypass."
+    source: manual
+    checks:
+      - check_type: command
+        check_value: "uv run pytest tests/test_di_injection_overseer_benchmarker.py -v --tb=short -q"
+  - id: dod-deploy
+    description: "Pure code seam refactor — no runtime restart or Kafka topic change required"
+    source: manual
+    checks:
+      - check_type: command
+        check_value: "printf '%s\n' 'deploy gate: pure DI seam refactor OMN-10873 — no docker exec or rpk topic produce required'"

src/omnimarket/nodes/node_llm_eval_harness/handlers/handler_llm_eval_harness.py

@@ -19,7 +19,7 @@
 from enum import StrEnum
 from pathlib import Path
 from statistics import mean
-from typing import Protocol
+from typing import Protocol, runtime_checkable
 
 from pydantic import BaseModel, ConfigDict, Field
 
@@ -333,6 +333,18 @@ def _score(
         return score, False, False, hits
 
 
+@runtime_checkable
+class ProtocolLlmEvalHarness(Protocol):
+    """Structural protocol for an LLM eval harness.
+
+    Any object with a ``handle()`` method matching this signature satisfies
+    the protocol, allowing test doubles and subclasses to be injected into
+    ``NodeOverseerBenchmarker`` without inheriting from ``NodeLlmEvalHarness``.
+    """
+
+    def handle(self, request: LlmEvalRequest) -> LlmEvalResult: ...
+
+
 _FENCE_RE = re.compile(r"^```[a-zA-Z0-9_+-]*\n?|```$", re.MULTILINE)
 
 

src/omnimarket/nodes/node_navigation_history_reducer/__init__.py

@@ -35,6 +35,7 @@
 from omnimarket.nodes.node_navigation_history_reducer.handlers import (
     HandlerNavigationHistoryReducer,
     HandlerNavigationHistoryWriter,
+    ProtocolNavigationHistoryWriter,
 )
 from omnimarket.nodes.node_navigation_history_reducer.models import (
     ModelNavigationHistoryRequest,
@@ -56,4 +57,5 @@
     "ModelPlanStep",
     "NavigationOutcome",
     "NodeNavigationHistoryReducer",
+    "ProtocolNavigationHistoryWriter",
 ]

src/omnimarket/nodes/node_navigation_history_reducer/handlers/__init__.py

@@ -7,6 +7,11 @@
 from .handler_navigation_history_reducer import (
     HandlerNavigationHistoryReducer,
     HandlerNavigationHistoryWriter,
+    ProtocolNavigationHistoryWriter,
 )
 
-__all__ = ["HandlerNavigationHistoryReducer", "HandlerNavigationHistoryWriter"]
+__all__ = [
+    "HandlerNavigationHistoryReducer",
+    "HandlerNavigationHistoryWriter",
+    "ProtocolNavigationHistoryWriter",
+]

src/omnimarket/nodes/node_navigation_history_reducer/handlers/handler_navigation_history_reducer.py

@@ -30,7 +30,7 @@
 import json
 import logging
 import os
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, Protocol, runtime_checkable
 from uuid import UUID
 
 import structlog
@@ -54,12 +54,33 @@
 logger = logging.getLogger(__name__)
 structured_logger = structlog.get_logger(__name__)
 
-__all__ = ["HandlerNavigationHistoryReducer", "HandlerNavigationHistoryWriter"]
+__all__ = [
+    "HandlerNavigationHistoryReducer",
+    "HandlerNavigationHistoryWriter",
+    "ProtocolNavigationHistoryWriter",
+]
 
 # ---------------------------------------------------------------------------
 # Constants (env-var driven; no hardcoded internal IPs)
 # ---------------------------------------------------------------------------
 
+
+@runtime_checkable
+class ProtocolNavigationHistoryWriter(Protocol):
+    """Structural protocol for a navigation session persistence writer.
+
+    Any object with ``record()`` and ``close()`` satisfies this protocol,
+    allowing test doubles and alternative implementations to be injected into
+    ``HandlerNavigationHistoryReducer`` without subclassing.
+    """
+
+    async def record(
+        self, session: ModelNavigationSession
+    ) -> ModelNavigationHistoryResponse: ...
+
+    async def close(self) -> None: ...
+
+
 _DEFAULT_PG_DSN = os.environ.get("OMNIMEMORY_PG_DSN", "")
 _DEFAULT_QDRANT_HOST = os.environ.get("QDRANT_HOST", "")
 _DEFAULT_QDRANT_PORT = int(os.environ.get("QDRANT_PORT", "6333"))
@@ -471,7 +492,7 @@ class HandlerNavigationHistoryReducer:
     failures.
 
     Attributes:
-        _writer: The underlying ``HandlerNavigationHistoryWriter`` instance.
+        _writer: The underlying ``ProtocolNavigationHistoryWriter`` instance.
         _initialized: Whether ``initialize()`` has been called.
 
     Example::
@@ -487,7 +508,7 @@ class HandlerNavigationHistoryReducer:
 
     def __init__(
         self,
-        writer: HandlerNavigationHistoryWriter | None = None,
+        writer: ProtocolNavigationHistoryWriter | None = None,
         pg_dsn: str = _DEFAULT_PG_DSN,
         qdrant_host: str = _DEFAULT_QDRANT_HOST,
         qdrant_port: int = _DEFAULT_QDRANT_PORT,
@@ -497,14 +518,17 @@ def __init__(
         """Initialize the handler.
 
         Args:
-            writer: Optional pre-constructed ``HandlerNavigationHistoryWriter``.
-                If not provided, one is created from the remaining kwargs.
+            writer: Optional injected ``ProtocolNavigationHistoryWriter``.
+                Accepts any conforming implementation, including test doubles.
+                If not provided, a ``HandlerNavigationHistoryWriter`` is
+                constructed from the remaining kwargs.
             pg_dsn: PostgreSQL DSN (used only if ``writer`` is None).
             qdrant_host: Qdrant hostname (used only if ``writer`` is None).
             qdrant_port: Qdrant port (used only if ``writer`` is None).
             embedding_url: Embedding endpoint URL (used only if ``writer`` is None).
             embedding_model: Embedding model name (used only if ``writer`` is None).
         """
+        self._writer: ProtocolNavigationHistoryWriter
         if writer is not None:
             self._writer = writer
         else:

src/omnimarket/nodes/node_overseer_benchmarker/handlers/handler_overseer_benchmarker.py

@@ -25,6 +25,7 @@
     ModelLlmEvalSample,
     NodeLlmEvalHarness,
     ProtocolLlmClient,
+    ProtocolLlmEvalHarness,
 )
 
 LEDGER_FILENAME = "overseer_performance_ledger.jsonl"
@@ -113,9 +114,10 @@ class NodeOverseerBenchmarker:
 
     def __init__(
         self,
-        harness: NodeLlmEvalHarness | None = None,
+        harness: ProtocolLlmEvalHarness | None = None,
         client: ProtocolLlmClient | None = None,
     ) -> None:
+        self._harness: ProtocolLlmEvalHarness
         if harness is not None:
             self._harness = harness
         else:

tests/test_di_injection_navigation_history_reducer.py

@@ -0,0 +1,129 @@
+# SPDX-FileCopyrightText: 2026 OmniNode.ai Inc.
+# SPDX-License-Identifier: MIT
+"""DI injection-path tests for OMN-10872.
+
+Verifies that HandlerNavigationHistoryReducer accepts a ProtocolNavigationHistoryWriter
+stub and routes all calls through it instead of constructing a concrete writer.
+"""
+
+from __future__ import annotations
+
+from datetime import UTC, datetime
+from uuid import uuid4
+
+import pytest
+
+from omnimarket.nodes.node_navigation_history_reducer import (
+    HandlerNavigationHistoryReducer,
+    HandlerNavigationHistoryWriter,
+    ModelNavigationHistoryRequest,
+    ModelNavigationSession,
+    ProtocolNavigationHistoryWriter,
+)
+from omnimarket.nodes.node_navigation_history_reducer.models.model_navigation_history_response import (
+    ModelNavigationHistoryResponse,
+)
+from omnimarket.nodes.node_navigation_history_reducer.models.model_navigation_session import (
+    ModelNavigationOutcomeSuccess,
+    ModelPlanStep,
+)
+
+
+def _make_session() -> ModelNavigationSession:
+    step = ModelPlanStep(
+        step_index=0,
+        from_state_id="s0",
+        to_state_id="s1",
+        action="move",
+        executed_at=datetime.now(tz=UTC),
+    )
+    return ModelNavigationSession(
+        session_id=uuid4(),
+        goal_condition="reach s1",
+        start_state_id="s0",
+        end_state_id="s1",
+        executed_steps=[step],
+        final_outcome=ModelNavigationOutcomeSuccess(reached_state_id="s1"),
+        graph_fingerprint="fp123",
+        created_at=datetime.now(tz=UTC),
+    )
+
+
+class _StubWriter:
+    """Minimal ProtocolNavigationHistoryWriter stub for injection tests."""
+
+    def __init__(self) -> None:
+        self.record_calls: list[ModelNavigationSession] = []
+        self.close_calls: int = 0
+
+    async def record(
+        self, session: ModelNavigationSession
+    ) -> ModelNavigationHistoryResponse:
+        self.record_calls.append(session)
+        return ModelNavigationHistoryResponse(
+            session_id=session.session_id,
+            status="success",
+            postgres_written=True,
+            qdrant_written=True,
+        )
+
+    async def close(self) -> None:
+        self.close_calls += 1
+
+
+@pytest.mark.unit
+class TestNavigationHistoryReducerDiInjection:
+    """OMN-10872: handler accepts ProtocolNavigationHistoryWriter via DI."""
+
+    def test_protocol_is_runtime_checkable(self) -> None:
+        stub = _StubWriter()
+        assert isinstance(stub, ProtocolNavigationHistoryWriter)
+
+    def test_concrete_writer_satisfies_protocol(self) -> None:
+        writer = HandlerNavigationHistoryWriter.__new__(HandlerNavigationHistoryWriter)
+        assert isinstance(writer, ProtocolNavigationHistoryWriter)
+
+    @pytest.mark.asyncio
+    async def test_injected_writer_is_used(self) -> None:
+        stub = _StubWriter()
+        handler = HandlerNavigationHistoryReducer(writer=stub)
+        await handler.initialize()
+
+        session = _make_session()
+        request = ModelNavigationHistoryRequest(session=session)
+        result = await handler.execute(request)
+
+        assert len(stub.record_calls) == 1
+        assert stub.record_calls[0].session_id == session.session_id
+        assert result.status == "success"
+
+    @pytest.mark.asyncio
+    async def test_shutdown_calls_writer_close(self) -> None:
+        stub = _StubWriter()
+        handler = HandlerNavigationHistoryReducer(writer=stub)
+        await handler.initialize()
+        await handler.shutdown()
+
+        assert stub.close_calls == 1
+
+    @pytest.mark.asyncio
+    async def test_default_construction_still_works(self) -> None:
+        """No writer injected — concrete HandlerNavigationHistoryWriter is built."""
+        handler = HandlerNavigationHistoryReducer(
+            pg_dsn="postgresql://localhost/test",
+            qdrant_host="localhost",
+        )
+        assert handler._writer is not None
+        assert isinstance(handler._writer, HandlerNavigationHistoryWriter)
+
+    @pytest.mark.asyncio
+    async def test_uninitialized_handler_returns_error(self) -> None:
+        stub = _StubWriter()
+        handler = HandlerNavigationHistoryReducer(writer=stub)
+        session = _make_session()
+        request = ModelNavigationHistoryRequest(session=session)
+
+        result = await handler.execute(request)
+        assert result.status == "error"
+        assert "not initialized" in (result.error_message or "").lower()
+        assert len(stub.record_calls) == 0