refactor(modularization): Phase 6 (#20) cleanup — retire residual Protocol + promote is_terminal + identity facade (#1635)

## Phase 6 cleanup — residual seams resolved (task #20)

Lands the canonical Phase 6 cleanup log registered during Phase 5 close-out (msg=da408d0f, 5 entries). PM msg=09e6753c / msg=95b584b0 locked scope strictly to these 5 entries; no Phase 3/4/5 legacy shim sweep, no new-phase re-migration.

### Entries

- **Entry 4** `1e80679` — retire the dead ``ChatDocumentOps`` Protocol class literal (no runtime reference after Phase 5 5-S5b).
- **Entry 5** `68ef567` — promote ``_TERMINAL_RUN_STATUSES`` frozenset to ``EvaluationRunStatus.is_terminal()`` classmethod. Task #23 race-fix guard (PR #1631 54cd86b) is byte-identical — classmethod accepts both enum members and raw strings because ``EvaluationRunStatus`` inherits ``str`` and the DB column is plain ``String``.
- **Entry 2** `8ac8ce2` — retire the ``ChatCollectionServiceOps`` Protocol + DI seam. ``chat_document_service`` sibling-imports ``chat_collection_service`` directly now that both live in the conversation domain after 5-S4f. CRITICAL_WIRINGS registry collapsed 3→2 in the same commit per PM msg=f7616d25.
- **Entry 1** `d04ed2a` — replace the 5-S4f raw ``UPDATE users …`` with an ``identity_user_ops.set_chat_collection`` facade (9a-sexdec hierarchy-1 terminal state). User ORM writes now travel through a narrow identity-owned entry point; G16 stays green without raw SQL.
- **Entry 3** `4a7f0bc` — record ``PromptTemplateOps`` (and ``QuotaOps``) as permanent standalone-infra seams rather than transitional Phase-6 candidates. Doc-only refactor; no code move, no caller touch.

### Final state

The ``CRITICAL_WIRINGS`` registry converges to **2 permanent entries** — both provider modules are standalone-infrastructure (no natural domain home):

1. ``conversation.bot_service._quota_ops`` — ``aperag.service.quota_service``
2. ``agent_runtime.runtime._prompt_template_ops`` — ``aperag.service.prompt_template_service``

Original canonical estimate 5 → Phase 5 collapsed to 3 → Phase 6 collapsed to 2 permanent.

### Merge gate

- @huangheng current-head CR no-blocker for bfd5590 (Phase 6 CR chain 5/5 all no-blocker)
- lint-and-unit pass (2m54s)
- e2e-http-compose skipping (path filter by design)
- Per @earayu2 canonical (16:13/16:16): 1 CR no-blocker + lint-and-unit pass ⇒ merge authorized, e2e not gating

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Author: earayu

aperag/app.py

@@ -42,9 +42,6 @@
     chat_router as chat_router,
 )
 from aperag.domains.conversation.service.bot_service import set_quota_ops as _conv_set_quota_ops
-from aperag.domains.conversation.service.chat_document_service import (
-    set_chat_collection_ops as _conv_set_chat_collection_ops,
-)
 from aperag.domains.evaluation.api.routes import router as evaluation_v2_router
 from aperag.domains.governance.api.routes import router as governance_router
 from aperag.domains.identity.service.user_manager import (
@@ -107,33 +104,19 @@
 _kb_set_quota_ops(_legacy_quota_service)
 
 # Wire the conversation domain's consumer-owned QuotaOps DI slot for
-# ``bot_service`` (Phase 5 step 5-S4c, canonical msg=4a93c97e Q1 C).
-# The legacy quota_service structurally satisfies the narrower
-# conversation ``QuotaOps`` Protocol (``check_and_consume_quota`` +
-# ``release_quota``) via the same singleton, so the same legacy
-# instance is plugged in here — Phase 6 cleanup removes the wire-up
-# when quota_service finds its permanent home.
+# ``bot_service``. ``quota_service`` is a standalone-infrastructure
+# module with no natural domain home, so the Protocol + DI seam is
+# the permanent integration point. The singleton structurally
+# satisfies the narrower conversation ``QuotaOps`` Protocol
+# (``check_and_consume_quota`` + ``release_quota``) directly.
 _conv_set_quota_ops(_legacy_quota_service)
 
-# Wire the ``ChatCollectionServiceOps`` DI slot for ``chat_document_service``
-# (Phase 5 step 5-S4d). ``chat_document_service`` moved into the
-# conversation domain before ``chat_collection_service`` itself — the
-# sibling service still needs a Phase 4 identity-domain rebase for the
-# ``session.get(User, ...)`` rewrite. Until 5-S4f (after ``#1633``
-# merges), the legacy singleton fills the DI slot; it structurally
-# satisfies the Protocol verbatim.
-from aperag.service.chat_collection_service import (  # noqa: E402
-    chat_collection_service as _legacy_chat_collection_service,
-)
-
-_conv_set_chat_collection_ops(_legacy_chat_collection_service)
-
-
 # Wire the agent_runtime domain's consumer-owned PromptTemplateOps DI
-# slot (Phase 5 step 5-S5b). ``prompt_template_service`` stays as a
-# legacy provider through Phase 6 cleanup, so an adapter exposes the
-# three Protocol methods onto the legacy singleton + module-level
-# ``build_agent_query_prompt`` helper.
+# slot. ``prompt_template_service`` is a standalone-infrastructure
+# module (no natural domain home), so the Protocol + DI seam is the
+# permanent integration point — an adapter exposes the three Protocol
+# methods onto the singleton + module-level ``build_agent_query_prompt``
+# helper.
 from aperag.service.prompt_template_service import (  # noqa: E402
     build_agent_query_prompt as _legacy_build_agent_query_prompt,
 )

aperag/db/repositories/evaluation_v2.py

@@ -16,10 +16,9 @@
 ``aperag.domains.evaluation.db.repositories.evaluation_v2`` after
 Phase 5 step 5-S6.
 
-``aperag.db.ops`` pulls in the repository mixin from here (plus the
-``_TERMINAL_RUN_STATUSES`` frozenset if any caller still wants the
-module-level alias), so the existing call sites keep working without
-a rename sweep. Phase 6 cleanup removes the shim.
+``aperag.db.ops`` pulls in the repository mixin from here so the
+existing call sites keep working without a rename sweep. Phase 6
+cleanup removes the shim.
 """
 
 from aperag.domains.evaluation.db.repositories.evaluation_v2 import (  # noqa: F401

aperag/domains/agent_runtime/ports.py

@@ -14,20 +14,13 @@
 
 """Cross-domain contracts owned by the ``agent_runtime`` domain.
 
-Phase 5 step 5-S5b adds the ``PromptTemplateOps`` Protocol for the
-legacy ``aperag.service.prompt_template_service`` provider —
-``prompt_template_service`` stays in ``aperag/service/`` through
-Phase 6 cleanup, so the agent_runtime domain cannot ``from
-aperag.service.*`` import it directly without tripping G1.
-
-The ``ChatDocumentOps`` Protocol originally seeded in Phase 5 step 1
-has been **retired** per msg=940bd884 simplification: after
-``chat_document_service`` physically moved into the conversation
-domain (Phase 5 step 5-S4d), ``runtime.py`` can reach it via a
-direct cross-domain import (domain→domain is allowed by G1). The
-Protocol is kept here only in archived form (below the ``__all__``
-for the live surface) so any in-flight branch still referencing it
-resolves the same shape; Phase 6 removes it outright.
+``PromptTemplateOps`` is the consumer-owned Protocol seam for
+``aperag.service.prompt_template_service``. The service is a
+standalone-infrastructure module (cross-cutting across agent_runtime
+runtime calls, conversation bot-config resolution, indexing prompt
+resolution, and a user-facing ``/prompts`` REST surface) with no
+natural domain home — the Protocol + DI seam is its permanent
+integration point under G1.
 
 ``AuthenticatedUser`` stays per-domain (lesson 9a-ter); runtime
 handlers only need ``id`` for turn ownership / lease checks.
@@ -53,8 +46,7 @@ class AuthenticatedUser(Protocol):
 
 @runtime_checkable
 class PromptTemplateOps(Protocol):
-    """Consumer-owned view of the legacy
-    ``aperag.service.prompt_template_service``.
+    """Consumer-owned view of ``aperag.service.prompt_template_service``.
 
     Exposes the three surfaces ``runtime.py`` actually uses:
 
@@ -73,11 +65,6 @@ class PromptTemplateOps(Protocol):
     singleton plus the module-level ``build_agent_query_prompt``
     function, so ``aperag/app.py`` wires an adapter that fans out to
     both to satisfy the Protocol.
-
-    Phase 6 cleanup will either move ``prompt_template_service`` into
-    a canonical domain home (agent_runtime or model_platform candidate
-    per msg=65a3b27d) or retire the Protocol in favour of a direct
-    cross-domain import.
     """
 
     async def resolve_agent_system_prompt(self, *, bot: Any, user_id: str) -> str: ...
@@ -99,17 +86,3 @@ def build_agent_query_prompt(
     "AuthenticatedUser",
     "PromptTemplateOps",
 ]
-
-
-# ``ChatDocumentOps`` retained for any in-flight caller that still
-# imports it. Phase 5 step 5-S5b replaced the runtime's DI seam with a
-# direct cross-domain import of
-# ``aperag.domains.conversation.service.chat_document_service`` now
-# that the conversation domain is merged — the Protocol definition is
-# no longer wired at app startup and is scheduled for removal in
-# Phase 6.
-
-
-@runtime_checkable
-class ChatDocumentOps(Protocol):  # pragma: no cover - retired, Phase 6 deletion candidate
-    async def has_documents_in_chat(self, chat_id: str, user_id: str) -> bool: ...

aperag/domains/agent_runtime/runtime.py

@@ -296,11 +296,6 @@ async def emit(
             )
 
             history_context = await self.history_writer.build_history_context(user, chat.id)
-            # Direct cross-domain import of the sibling conversation service
-            # (Phase 5 step 5-S4d moved it into the conversation domain) —
-            # G1 allows domain→domain. msg=940bd884 simplification avoids a
-            # ``ChatDocumentOps`` Protocol seam now that the provider has
-            # moved out of the legacy ``aperag.service.*`` ban target.
             from aperag.domains.conversation.service.chat_document_service import chat_document_service
 
             has_chat_files = await chat_document_service.has_documents_in_chat(chat.id, user)

aperag/domains/conversation/ports.py

@@ -115,34 +115,8 @@ async def release_quota(
     ) -> None: ...
 
 
-@runtime_checkable
-class ChatCollectionServiceOps(Protocol):
-    """``chat_document_service``'s view of ``chat_collection_service``.
-
-    Phase 5 step 5-S4d moves ``chat_document_service`` into the
-    conversation domain before ``chat_collection_service`` itself —
-    ``chat_collection_service.create_user_chat_collection`` reaches
-    ``session.get(User, ...)`` which requires the Phase 4 identity
-    domain merge before it can be rewritten without leaking a G1
-    legacy-aggregate ``User`` import into the domain module.
-
-    Until the sibling service follows (5-S4f after ``#1633`` merges,
-    per PM ``msg=0f9f10c4`` β ruling), the two surfaces
-    ``chat_document_service`` actually calls —
-    ``get_user_chat_collection`` and ``create_user_chat_collection``
-    — are exposed here as a consumer-owned Protocol. ``aperag/app.py``
-    wires the legacy singleton in at startup; the legacy instance
-    structurally satisfies the Protocol verbatim.
-    """
-
-    async def get_user_chat_collection(self, user_id: str) -> Any: ...
-
-    async def create_user_chat_collection(self, user_id: str) -> Any: ...
-
-
 __all__ = [
     "KnowledgeBaseCollectionView",
     "AuthenticatedUser",
     "QuotaOps",
-    "ChatCollectionServiceOps",
 ]

aperag/domains/conversation/service/chat_collection_service.py

@@ -12,52 +12,33 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-"""Chat-collection management service moved to the conversation domain
-in Phase 5 step 5-S4f.
-
-Phase 4 ``#1633`` + Phase 3 ``#1629`` are merged on main, so the
-cross-domain ORM imports take the β canonical path from
-``msg=4ed698d8`` — direct domain-to-domain imports of ``User`` /
-``Collection`` + sibling domain services, no stopgap Protocol + DI
-wrapper:
-
-* ``aperag.db.models.User`` → ``aperag.domains.identity.db.models.User``
-* ``aperag.db.models.Collection`` →
-  ``aperag.domains.knowledge_base.db.models.Collection``
-* ``aperag.schema.view_models.CollectionConfig / ModelSpec /
-  TagFilterCondition / TagFilterRequest`` → ``aperag.schema.common``
-  for the first two and
-  ``aperag.domains.model_platform.schemas`` for the tag filters.
-* ``aperag.service.collection_service`` →
-  ``aperag.domains.knowledge_base.service.collection_service``
-* ``aperag.service.llm_available_model_service`` →
-  ``aperag.domains.model_platform.service.llm_available_model_service``
-
-The pre-move ``KnowledgeBaseCollectionView`` return type (from
-``aperag.domains.conversation.ports``) is kept — the Protocol was
-established in Phase 3 Step 5c for exactly this module, and there is
-no benefit in swapping it for the concrete ``Collection`` class here.
-
-The ``_mark_as_chat_collection`` transaction closure uses
-``session.get(Collection, ...)`` — Collection is now imported from the
-knowledge_base domain, which G1 permits. The ``User.chat_collection_id``
-update is issued via ``sqlalchemy.text`` rather than ``session.get(User,
-...)`` + attribute assignment so the conversation domain does not have
-to import the identity-owned ``User`` ORM class (G16 canonical per
-``msg=6d2ae86a`` forbids ``User`` imports outside the identity domain).
-The raw UPDATE hits the same ``users`` table that the ORM mapper binds
-to, so the transaction semantics are identical.
+"""Chat-collection management service in the conversation domain.
+
+Cross-domain dependencies are direct imports:
+
+* ``aperag.domains.identity.db.models.User`` is **not** imported here —
+  ``User.chat_collection_id`` writes travel through the identity-owned
+  ``set_chat_collection`` facade (lesson 9a-sexdec hierarchy-1) so G16
+  stays green without the conversation domain leaking the ``User`` ORM.
+* ``aperag.domains.knowledge_base.db.models.Collection`` is imported
+  directly (domain→domain is legal under G1).
+* ``aperag.domains.knowledge_base.service.collection_service`` + the
+  ``model_platform`` availability service similarly sibling-import.
+
+The pre-move ``KnowledgeBaseCollectionView`` return type is kept —
+the Protocol was established in Phase 3 Step 5c for exactly this
+module, and there is no benefit in swapping it for the concrete
+``Collection`` class here.
 """
 
 from __future__ import annotations
 
 import logging
 from typing import Optional
 
-from sqlalchemy import text
-
 from aperag.db.ops import async_db_ops
 from aperag.domains.conversation.ports import KnowledgeBaseCollectionView
+from aperag.domains.identity.service.identity_user_ops import set_chat_collection as identity_set_chat_collection
 from aperag.domains.knowledge_base.db.models import Collection
 from aperag.domains.knowledge_base.schemas import CollectionCreate
 from aperag.domains.knowledge_base.service.collection_service import collection_service
@@ -172,13 +153,7 @@ async def _mark_as_chat_collection(session):
                 session.add(collection_obj)
                 await session.flush()
 
-            # Update User.chat_collection_id via raw SQL so this module
-            # does not have to import the identity-owned ``User`` ORM
-            # class (G16 canonical).
-            await session.execute(
-                text("UPDATE users SET chat_collection_id = :cid WHERE id = :uid"),
-                {"cid": collection_response.id, "uid": user_id},
-            )
+            await identity_set_chat_collection(session, user_id, collection_response.id)
             await session.flush()
 
         await self.db_ops.execute_with_transaction(_mark_as_chat_collection)