feat(sp27): dedup ack ID helpers — one copy in handlers/_ack_id.py

backend/src/cyclone/api.py

@@ -665,41 +665,21 @@ async def parse_835_endpoint(
 # 999 ACK (Implementation Acknowledgment)
 # --------------------------------------------------------------------------- #
 
-
-def _ack_count_summary(result) -> tuple[int, int, int, str]:
-    """Aggregate (received, accepted, rejected, ack_code) from a ParseResult999.
-
-    Counts are derived from the set-level ``IK5`` responses (one per
-    AK2 in the 999), not the functional-group ``AK9``. Gainwell's
-    MFT ships AK9 segments that contradict the per-set IK5
-    (e.g. ``AK9*A*1*1*1`` with ``IK5*A``), so trusting AK9's
-    rejected count would over-report rejections. The set-level
-    IK5 is the authoritative per-claim accept/reject signal.
-    """
-    sets = result.set_responses
-    received = len(sets)
-    accepted = sum(1 for s in sets if s.set_accept_reject.code == "A")
-    rejected = received - accepted
-    if rejected == 0:
-        code = "A"
-    elif accepted == 0:
-        code = "R"
-    else:
-        code = "P"
-    return (received, accepted, rejected, code)
-
-
-def _ack_synthetic_source_batch_id(interchange_control_number: str) -> str:
-    """Return a synthetic batches.id for a received 999 with no source batch.
-
-    The acks.source_batch_id FK requires a row in batches; for received
-    999s we synthesize an id of the form ``999-<ISA13>``. The synthetic
-    row is NOT created in batches — the FK enforcement is a no-op in
-    SQLite without ``PRAGMA foreign_keys=ON`` (the project default for
-    tests). The dashboard never surfaces these synthetic ids; they exist
-    solely to satisfy the ORM contract.
-    """
-    return f"999-{(interchange_control_number or '').strip() or '000000001'}"
+# SP27 Task 6: ack ID helpers were deduped. Both api.py and scheduler.py
+# used to define these locally. Canonical copies now live in
+# ``cyclone.handlers._ack_id`` (set up in Task 1). The aliased imports
+# below keep the existing callsites (``_ack_count_summary(result)``,
+# ``_ack_synthetic_source_batch_id(icn)``, ``_277ca_synthetic_source_batch_id(icn)``)
+# working unchanged.
+from cyclone.handlers._ack_id import (
+    ack_count_summary as _ack_count_summary,
+)
+from cyclone.handlers._ack_id import (
+    ack_synthetic_source_batch_id as _ack_synthetic_source_batch_id,
+)
+from cyclone.handlers._ack_id import (
+    two77ca_synthetic_source_batch_id as _277ca_synthetic_source_batch_id,
+)
 
 
 @app.post("/api/parse-999", dependencies=[Depends(matrix_gate)])
@@ -913,16 +893,9 @@ async def parse_ta1_endpoint(
 # --------------------------------------------------------------------------- #
 
 
-def _277ca_synthetic_source_batch_id(interchange_control_number: str) -> str:
-    """Return a synthetic ``batches.id`` for a received 277CA with no source batch.
-
-    Mirrors :func:`_ack_synthetic_source_batch_id`. The 277CA row's
-    ``source_batch_id`` FK requires a row in batches; for received
-    277CAs we synthesize an id of the form ``277CA-<ISA13>``. The row
-    is NOT created in batches — same FK-is-no-op convention as the 999
-    path.
-    """
-    return f"277CA-{(interchange_control_number or '').strip() or '000000001'}"
+# (The _277ca_synthetic_source_batch_id helper was moved to
+# cyclone.handlers._ack_id in SP27 Task 6; this alias import at the
+# top of the file binds the name for any inline callsites below.)
 
 
 @app.post("/api/parse-277ca", dependencies=[Depends(matrix_gate)])

backend/tests/test_api_dedup.py

@@ -0,0 +1,112 @@
+"""Regression tests: api.py + scheduler.py delegate to handlers/_ack_id (SP27 Task 6).
+
+After the dedup, the inline `_ack_count_summary`,
+`_ack_synthetic_source_batch_id`, and `_277ca_synthetic_source_batch_id`
+helpers should be gone from both `api.py` and `scheduler.py`. Their
+callers should reach the canonical copies under
+``cyclone.handlers._ack_id``.
+
+This test pins two things:
+
+1. The module attributes exist where expected (import paths intact).
+2. The endpoints that depend on the helpers still respond 200 (or
+   401 if unauthenticated) — i.e. the dedup didn't break the import
+   chain.
+"""
+from __future__ import annotations
+
+import pytest
+from fastapi.testclient import TestClient
+
+from cyclone import api, scheduler
+from cyclone.api import app
+from cyclone.handlers._ack_id import (
+    ack_count_summary,
+    ack_synthetic_source_batch_id,
+    two77ca_synthetic_source_batch_id,
+)
+
+
+# ---- 1. The dedup actually happened ---------------------------------------
+
+
+def test_api_no_longer_defines_inline_ack_helpers():
+    """api.py must not re-define the helpers as local symbols.
+
+    The alias imports at the top of api.py (``from cyclone.handlers._ack_id
+    import ack_count_summary as _ack_count_summary``) leave the
+    underscore-prefixed names bound in ``api.__dict__`` — that's the
+    whole point of an alias import and is required so the inline
+    call sites keep working. What we want to check is that the
+    *implementation source* is no longer the api module itself.
+
+    Concretely: the function's ``__module__`` attribute should be
+    ``cyclone.handlers._ack_id``, not ``cyclone.api``.
+    """
+    for name in (
+        "_ack_count_summary",
+        "_ack_synthetic_source_batch_id",
+        "_277ca_synthetic_source_batch_id",
+    ):
+        assert hasattr(api, name), (
+            f"api.py doesn't even have {name} — alias import missing"
+        )
+        fn = getattr(api, name)
+        assert fn.__module__ == "cyclone.handlers._ack_id", (
+            f"api.{name} resolves to {fn.__module__}, expected "
+            f"cyclone.handlers._ack_id — inline copy still exists"
+        )
+
+
+def test_scheduler_no_inline_ack_helpers():
+    """scheduler.py must not re-define the ack helpers as local symbols.
+
+    In SP27 Task 2 the inline copies of ``_ack_count_summary`` and
+    ``_ack_synthetic_source_batch_id`` were deleted from
+    scheduler.py (they had become dead code after Task 1 lifted the
+    canonical copies into ``cyclone.handlers._ack_id``). Task 4
+    removed ``_277ca_synthetic_source_batch_id`` the same way. So
+    none of the three names should be present in the scheduler
+    module at all.
+    """
+    for name in (
+        "_ack_count_summary",
+        "_ack_synthetic_source_batch_id",
+        "_277ca_synthetic_source_batch_id",
+    ):
+        assert not hasattr(scheduler, name), (
+            f"scheduler.py still defines {name} inline; dedup incomplete"
+        )
+
+
+def test_canonical_helpers_resolve_to_handlers_ack_id():
+    """The names exported by ``cyclone.handlers._ack_id`` must be
+    reachable from the canonical import path."""
+    assert callable(ack_count_summary)
+    assert callable(ack_synthetic_source_batch_id)
+    assert callable(two77ca_synthetic_source_batch_id)
+
+
+# ---- 2. Endpoints still respond -------------------------------------------
+
+
+@pytest.fixture
+def client() -> TestClient:
+    return TestClient(app)
+
+
+def test_ack_endpoints_respond_after_dedup(client):
+    """Hitting each ack endpoint must not raise ImportError. Either
+    200 (auth OK) or 401/403 (auth required) is acceptable; 500 means
+    the import chain broke."""
+    # Acquire an authenticated session by hitting login (idempotent —
+    # the same admin user already exists in the test DB).
+    for path in (
+        "/api/acks",
+        "/api/ta1-acks",
+        "/api/277ca-acks",
+    ):
+        resp = client.get(path)
+        assert resp.status_code in (200, 401, 403), (
+            f"{path} broke after dedup: {resp.status_code} {resp.text}"
+        )