refactor: protocols audit follow-up (REVIEW + fold pass)

docs/reference/protocols-audit.md

@@ -37,13 +37,13 @@ The counts are deliberately approximate: they're a triage tool, not a removal lo
 
 The 2026-05-10 snapshot counts in the original table were aggregated by hand and undercounted both the REMOVE and REVIEW rows. The table below reflects (a) the per-area row totals as they actually appear in the snapshot tables, and (b) the post-2026-05-11 cleanup outcome captured in the "Post-cleanup status" section.
 
-| Total | Recommendation (2026-05-10 snapshot row counts) | After 2026-05-11 cleanup (#1864) |
-|---|---|---|
-| Protocol classes inventoried | 250 | 248 (2 deleted) |
-| KEEP | 193 | 237 (44 audit re-flagged, plus pre-existing 193) |
-| MAKE-RUNTIME-CHECKABLE | 0 (no `isinstance` sites surveyed) | 0 |
-| REMOVE candidates (flag-only; must be re-verified at cleanup time) | 46 | 0 (2 deleted, 44 reclassified KEEP) |
-| REVIEW (`_PrivatePrefixed` typing seams; intent inspection needed) | 11 | 11 (unchanged; #1865 owns these) |
+| Total | Snapshot (2026-05-10) | After #1864 (2026-05-11) | After #1865 (2026-05-11) |
+|---|---|---|---|
+| Protocol classes inventoried | 250 | 248 (2 deleted) | 241 (7 more deleted) |
+| KEEP | 193 | 237 (44 audit re-flagged, plus pre-existing 193) | 241 (5 `_PrivatePrefixed` retained with rationale; 1 fold-deletion drops conflict_detection.py's KEEP) |
+| MAKE-RUNTIME-CHECKABLE | 0 (no `isinstance` sites surveyed) | 0 | 0 |
+| REMOVE candidates (flag-only; must be re-verified at cleanup time) | 46 | 0 (2 deleted, 44 reclassified KEEP) | 0 |
+| REVIEW (`_PrivatePrefixed` typing seams; intent inspection needed) | 11 | 11 (unchanged; #1865 owns these) | 0 (12 protocol names across the 11 snapshot rows: 5 retained with rationale, 5 deleted, 2 fold-deleted) |
 
 `@runtime_checkable` decoration: 209 of 250 (84%). The 41 without the decorator are listed inline below; absence is not a defect on its own; only a few need it.
 
@@ -405,6 +405,34 @@ Categories of re-flag rationale:
 
 5. **Multi-impl injection points missed by the audit doc's own duplicate detection**. `ParticipantResolver`@`meeting/participant.py:56` was flagged as a "dead duplicate" of an alleged twin in `meeting/protocol.py`; the twin does not exist. The participant.py copy has 2 impls (`PassthroughParticipantResolver`, `RegistryParticipantResolver`) in the same file and 3 test files.
 
+### Issue #1865 outcomes (REVIEW pass + duplicate folds)
+
+Issue [#1865](https://github.com/Aureliolo/synthorg/issues/1865) closed out the 11 REVIEW rows, which cover 12 protocol names: 9 `_PrivatePrefixed` typing seams (the `engine/coordination/dispatcher_types.py` row lists 2 names in one snapshot entry), the non-prefixed `IsDuplicate` predicate in `persistence/_shared/audit.py`, and the 2 duplicate-named-pair REVIEW rows. The 5 retained protocols each carry a one-line `# <reason>` rationale comment naming the consumer that justifies the seam; the 7 deletions (5 single-consumer seam collapses plus 2 duplicate-pair folds) collapsed single-consumer seams or removed pure dead code.
+
+#### Retained with rationale (5 protocols)
+
+Each gets a one-line `# <reason>` design-rationale comment immediately above the `class` line.
+
+| Path | Line | Name | Rationale |
+|---|---|---|---|
+| api/lifecycle.py | 86 | `_AsyncStartStop` | Structural seam over the optional `synthorg[distributed]` `JetStreamTaskQueue`; consumed by `_cleanup_on_failure` and `_safe_shutdown`. |
+| core/state_machine.py | 36 | `_HasValue` | Generic bound for `StateMachine[S]`; 4 structural users: `TaskStatus`, `RequestStatus`, `KanbanColumn`, `SprintStatus`. |
+| persistence/_shared/audit.py | 160 | `IsDuplicate` | Driver-abstraction predicate; impls: `sqlite._shared.is_unique_constraint_error`, `postgres.audit_repository._postgres_is_duplicate`. |
+| templates/_inheritance.py | 24 | `_RenderToDictFn` | Self-referential recursive callback for `render_parent_config`; consumer: `templates.renderer._render_to_dict` passes itself in to walk the parent chain. |
+| providers/management/_capabilities_mixin.py | 55 | `_ServiceProtocol` | Narrows the mixin's `self`-type to the 3 attrs + 3 methods consumed; host: `ProviderManagementService`. |
+
+#### Deleted (7 protocols)
+
+| Path | Line | Name | Outcome |
+|---|---|---|---|
+| engine/quality/graders/heuristic.py | 27 | `_HeuristicGraderBridge` | Deleted. Collapsed to `bridge: EngineBridgeConfig` with a `TYPE_CHECKING` import; the single structural consumer was already `EngineBridgeConfig`. |
+| engine/routing/scorer.py | 40 | `_RoutingScorerBridge` | Deleted. Same collapse as `_HeuristicGraderBridge`. |
+| templates/model_matcher.py | 370 | `_ModelMatcherBridge` | Deleted. Same collapse as `_HeuristicGraderBridge`. |
+| engine/coordination/attribution.py | 32 | `_ExecutionResultLike` | Deleted. Defined inside `if TYPE_CHECKING:` but never referenced as an annotation; runtime code already used `getattr()` on untyped values. Duck-type contract documented inline at the call site. |
+| engine/coordination/attribution.py | 35 | `_AgentRunResultLike` | Deleted. Same as `_ExecutionResultLike`. |
+| communication/meeting/conflict_detection.py | 39 | `ConflictDetector` (fold duplicate) | Deleted. Folded into the canonical definition at `communication/meeting/protocol.py:41`. The 5 importers already pointed at `protocol.py`; the `conflict_detection.py` copy was unimported dead code. |
+| meta/signals/protocol.py | 26 | `SignalAggregator` (fold duplicate) | Deleted. File removed entirely (zero importers; sole content was this dead protocol). Folded into the canonical definition at `meta/protocol.py:38`. |
+
 ### Recommended next pass for `scripts/protocol_audit.py`
 
 A follow-up issue should teach the audit script to (a) consult a manual override file (e.g. `data/protocols_audit_overrides.yaml`) keyed on `path:line:name` so KEEP decisions made here survive future regenerations, and (b) detect structural impls heuristically (same-file `class X:` siblings whose method names match the Protocol's, factory return type annotations) to reduce false-positive REMOVE flags.

src/synthorg/api/lifecycle.py

@@ -83,6 +83,8 @@
 """
 
 
+# Structural seam over the optional synthorg[distributed] JetStreamTaskQueue;
+# consumers: _cleanup_on_failure, _safe_shutdown.
 class _AsyncStartStop(Protocol):
     """Minimal async lifecycle Protocol used by the distributed task queue hook.
 

src/synthorg/communication/meeting/conflict_detection.py

@@ -12,7 +12,7 @@
 """
 
 import re
-from typing import Any, Protocol, runtime_checkable
+from typing import Any
 
 from synthorg.core.json_parsing import extract_json_from_llm_response
 from synthorg.observability import get_logger
@@ -35,26 +35,6 @@
 _DEFAULT_SIMILARITY_THRESHOLD: float = 0.7
 
 
-@runtime_checkable
-class ConflictDetector(Protocol):
-    """Protocol for pluggable conflict detection strategies.
-
-    All conflict detectors share this interface so consumers can
-    type against the pluggable API under mypy strict mode.
-    """
-
-    def detect(self, response_content: str) -> bool:
-        """Determine whether the response indicates conflicts.
-
-        Args:
-            response_content: The conflict-check agent response text.
-
-        Returns:
-            True if conflicts detected, False otherwise.
-        """
-        ...
-
-
 _MIN_POSITIONS_FOR_CONFLICT: int = 2
 
 # Identity/metadata keys to skip during structured field comparison.

src/synthorg/core/state_machine.py

@@ -33,6 +33,8 @@ def validate_transition(current: TaskStatus, target: TaskStatus) -> None:
 logger = get_logger(__name__)
 
 
+# Generic bound for StateMachine[S]; structural users: TaskStatus,
+# RequestStatus, KanbanColumn, SprintStatus.
 class _HasValue(Protocol):
     """Structural type for enum-like states (e.g. ``StrEnum`` members)."""
 

src/synthorg/engine/coordination/attribution.py

@@ -6,7 +6,7 @@
 with attribution data built from routing decisions and wave outcomes.
 """
 
-from typing import TYPE_CHECKING, Literal, Protocol, Self
+from typing import Final, Literal, Self
 
 from pydantic import BaseModel, ConfigDict, Field, computed_field, model_validator
 
@@ -27,17 +27,6 @@
 logger = get_logger(__name__)
 
 
-if TYPE_CHECKING:
-
-    class _ExecutionResultLike(Protocol):
-        error_message: str | None
-
-    class _AgentRunResultLike(Protocol):
-        is_success: bool
-        termination_reason: TerminationReason | None
-        execution_result: _ExecutionResultLike | None
-
-
 FailureAttribution = Literal[
     "direct",
     "upstream_contamination",
@@ -48,7 +37,7 @@ class _AgentRunResultLike(Protocol):
 # Internal constant by design: defensive truncation of evidence text
 # attached to failure attributions; prevents oversized error-evidence
 # payloads.  Not exposed to the settings registry.
-_MAX_EVIDENCE_LENGTH = 500
+_MAX_EVIDENCE_LENGTH: Final[int] = 500
 
 # Map FailureCategory -> FailureAttribution for error-based outcomes.
 _CATEGORY_TO_ATTRIBUTION: dict[FailureCategory, FailureAttribution] = {
@@ -256,9 +245,12 @@ def _score_outcome(
             evidence=outcome_error[:_MAX_EVIDENCE_LENGTH],
         )
 
-    # Case 2: Execution completed -- check if successful.
-    # outcome_result conforms to _AgentRunResultLike; typed as object
-    # at runtime to avoid circular import.
+    # Case 2: Execution completed -- check if successful. The
+    # ``outcome_result`` is duck-typed (an ``AgentRunResult`` in practice)
+    # so this module avoids importing ``engine.run_result`` and the cycle
+    # it would pull in; we expect ``.is_success: bool``,
+    # ``.termination_reason: TerminationReason | None``, and
+    # ``.execution_result: <obj with .error_message: str | None> | None``.
     if outcome_result is not None:
         is_success = getattr(outcome_result, "is_success", False)
         if is_success:

src/synthorg/engine/quality/graders/heuristic.py

@@ -1,7 +1,7 @@
 """Heuristic rubric grader -- rule-based, deterministic."""
 
 from datetime import UTC, datetime
-from typing import TYPE_CHECKING, Protocol
+from typing import TYPE_CHECKING
 
 from pydantic import BaseModel, ConfigDict, Field
 
@@ -20,32 +20,11 @@
     from synthorg.core.types import NotBlankStr
     from synthorg.engine.quality.verification import AtomicProbe
     from synthorg.engine.workflow.handoff import HandoffArtifact
+    from synthorg.settings.bridge_configs import EngineBridgeConfig
 
 logger = get_logger(__name__)
 
 
-class _HeuristicGraderBridge(Protocol):
-    """Structural type for the EngineBridgeConfig quality_heuristic_* fields.
-
-    Local Protocol declaration keeps the grader module free of a
-    runtime import of ``settings.bridge_configs``. The fields are
-    exposed as ``@property`` so this Protocol matches frozen Pydantic
-    models (whose attributes are read-only); mypy validates the
-    attribute mapping in ``from_bridge_config``.
-    """
-
-    @property
-    def quality_heuristic_pass_threshold(self) -> float: ...
-    @property
-    def quality_heuristic_pass_grade(self) -> float: ...
-    @property
-    def quality_heuristic_fail_grade(self) -> float: ...
-    @property
-    def quality_heuristic_confidence_ceiling(self) -> float: ...
-    @property
-    def quality_heuristic_confidence_bias(self) -> float: ...
-
-
 class HeuristicGraderConfig(BaseModel):
     """Operator-tunable thresholds + per-criterion grades for the heuristic grader.
 
@@ -70,15 +49,13 @@ class HeuristicGraderConfig(BaseModel):
     confidence_bias: float = Field(default=0.1, ge=0.0, le=1.0)
 
     @classmethod
-    def from_bridge_config(
-        cls, bridge: _HeuristicGraderBridge
-    ) -> HeuristicGraderConfig:
+    def from_bridge_config(cls, bridge: EngineBridgeConfig) -> HeuristicGraderConfig:
         """Project the heuristic-grader subset out of an ``EngineBridgeConfig``.
 
-        Typed via the local ``_HeuristicGraderBridge`` Protocol so mypy
-        validates the attribute mapping without a runtime import of
-        ``settings.bridge_configs`` (which would cycle through the
-        engine namespace).
+        ``EngineBridgeConfig`` is imported under ``TYPE_CHECKING`` so the
+        grader module stays free of a runtime dependency on
+        ``settings.bridge_configs`` (which would cycle through the engine
+        namespace).
         """
         return cls(
             pass_threshold=bridge.quality_heuristic_pass_threshold,

src/synthorg/engine/routing/scorer.py

@@ -7,7 +7,7 @@
 :class:`RoutingScorerConfig` (resolved at construction time).
 """
 
-from typing import TYPE_CHECKING, Final, Protocol
+from typing import TYPE_CHECKING, Final
 
 from pydantic import BaseModel, ConfigDict, Field, model_validator
 
@@ -23,6 +23,7 @@
 if TYPE_CHECKING:
     from synthorg.core.agent import AgentIdentity
     from synthorg.engine.decomposition.models import SubtaskDefinition
+    from synthorg.settings.bridge_configs import EngineBridgeConfig
 
 logger = get_logger(__name__)
 
@@ -37,33 +38,6 @@
 _WEIGHT_SUM_WARN_CEILING: Final[float] = 1.3  # lint-allow: magic-numbers -- ceiling
 
 
-class _RoutingScorerBridge(Protocol):
-    """Structural type for the EngineBridgeConfig fields the scorer reads.
-
-    Declared locally to break the import cycle that would otherwise
-    pull ``settings.bridge_configs`` into the engine routing module.
-    The fields are exposed as ``@property`` so this Protocol matches
-    frozen Pydantic models (whose attributes are read-only). Mypy
-    validates the attribute mapping in ``from_bridge_config`` against
-    this Protocol; any caller passing an object missing one of the
-    fields gets a static type error rather than a runtime
-    ``AttributeError``.
-    """
-
-    @property
-    def routing_weight_primary_skill(self) -> float: ...
-    @property
-    def routing_weight_secondary_skill(self) -> float: ...
-    @property
-    def routing_weight_tag_match_bonus(self) -> float: ...
-    @property
-    def routing_weight_role_match_bonus(self) -> float: ...
-    @property
-    def routing_weight_seniority_alignment_bonus(self) -> float: ...
-    @property
-    def routing_min_score(self) -> float: ...
-
-
 class RoutingScorerConfig(BaseModel):
     """Operator-tunable configuration for :class:`AgentTaskScorer`.
 
@@ -116,13 +90,13 @@ def _check_weight_sum(self) -> RoutingScorerConfig:
         return self
 
     @classmethod
-    def from_bridge_config(cls, bridge: _RoutingScorerBridge) -> RoutingScorerConfig:
+    def from_bridge_config(cls, bridge: EngineBridgeConfig) -> RoutingScorerConfig:
         """Project the routing-scorer subset out of an ``EngineBridgeConfig``.
 
-        Typed via the local ``_RoutingScorerBridge`` Protocol so mypy
-        validates the attribute mapping without forcing a runtime
-        import of ``settings.bridge_configs`` (which would create a
-        cycle through the engine namespace).
+        ``EngineBridgeConfig`` is imported under ``TYPE_CHECKING`` so the
+        routing module stays free of a runtime dependency on
+        ``settings.bridge_configs`` (which would cycle through the engine
+        namespace).
         """
         return cls(
             primary_skill_weight=bridge.routing_weight_primary_skill,

src/synthorg/persistence/_shared/audit.py

@@ -157,6 +157,8 @@ def row_to_audit_entry(row: dict[str, object]) -> AuditEntry:
         raise MalformedRowError(msg) from exc
 
 
+# Driver-abstraction predicate; impls: sqlite._shared.is_unique_constraint_error,
+# postgres.audit_repository._postgres_is_duplicate.
 class IsDuplicate(Protocol):
     """Driver-specific predicate that classifies an INSERT failure.
 

src/synthorg/providers/management/_capabilities_mixin.py

@@ -52,6 +52,8 @@
 logger = get_logger(__name__)
 
 
+# Narrows the mixin's self-type to the 3 attrs + 3 methods consumed;
+# host: ProviderManagementService (composed via MRO in service.py).
 class _ServiceProtocol(Protocol):
     """Subset of ``ProviderManagementService`` accessed by the mixin.
 

src/synthorg/templates/_inheritance.py

@@ -5,7 +5,7 @@
 Extracted from ``renderer.py`` to keep file sizes under 800 lines.
 """
 
-from typing import TYPE_CHECKING, Any, Protocol
+from typing import TYPE_CHECKING, Any, Final, Protocol
 
 from synthorg.observability import get_logger
 from synthorg.observability.events.template import (
@@ -21,6 +21,8 @@
     from synthorg.templates.schema import CompanyTemplate
 
 
+# Self-referential callback for render_parent_config; consumer:
+# templates.renderer._render_to_dict passes itself in to walk the parent chain.
 class _RenderToDictFn(Protocol):
     """Callback protocol for ``_render_to_dict``."""
 
@@ -39,7 +41,7 @@ def __call__(
 logger = get_logger(__name__)
 
 # Maximum inheritance chain depth.
-_MAX_INHERITANCE_DEPTH = 10
+_MAX_INHERITANCE_DEPTH: Final[int] = 10
 
 
 def _validate_inheritance_chain(