historic EPC: read via infrastructure/s3, not the utils.s3 utility

HistoricEpcS3Repository reached into utils/s3.py (read_csv_gz_from_s3 +
parse_s3_uri), the legacy utility that self-constructs boto3 inside free
functions. The other S3 repositories deliberately depend on the
infrastructure/s3 layer instead (UnstandardisedAddressListCsvS3Repository
injects a CsvS3Client). Bring historic EPC into line.

- Add GzipCsvS3Client(S3Client) in infrastructure/s3: read_csv_gz(key) ->
  DataFrame (get_object + gzip decode).
- Inject it into HistoricEpcS3Repository; the bucket lives in the client and
  the repo only builds the per-postcode key + maps rows (no S3/HTTP code).
  Add with_default_s3_client(s3_root) for composition roots.
- Update main.py and the match_addresses_for_postcode seam to the factory.
- Repo tests inject a real GzipCsvS3Client over a controlled boto stub
  (exact key assertions + AccessDenied); add a moto-based client test and a
  factory test covering s3_root -> bucket+key.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQE5TsSuQTeNSCSz9A9GQf

backend/address2UPRN/main.py

@@ -51,7 +51,7 @@ def get_uprn_from_historic_epc(
     historic addresses use a more verbose format that systematically depresses
     lexiscores.
     """
-    repo = HistoricEpcS3Repository()
+    repo = HistoricEpcS3Repository.with_default_s3_client()
     return HistoricEpcResolver(repo).resolve_uprn(user_inputed_address, postcode)
 
 

datatypes/epc/domain/historic_epc_matching.py

@@ -110,5 +110,5 @@ def match_addresses_for_postcode(
         HistoricEpcS3Repository,
     )
 
-    repo = HistoricEpcS3Repository(s3_root=s3_root)
+    repo = HistoricEpcS3Repository.with_default_s3_client(s3_root)
     return HistoricEpcResolver(repo).match(user_address, postcode)

datatypes/epc/domain/tests/test_historic_epc_matching.py

@@ -11,7 +11,7 @@ from datatypes.epc.domain.historic_epc_matching import (
     ScoredHistoricEpc,
     match_addresses_for_postcode,
 )
-from repositories.historic_epc import historic_epc_s3_repository as repo_mod
+from infrastructure.s3.gzip_csv_s3_client import GzipCsvS3Client
 
 # Columns required by the HistoricEpc dataclass (lower-cased CSV columns).
 # The matcher only reads ADDRESS + UPRN to score; everything else is filled
@@ -126,9 +126,12 @@ def _build_df(rows: list[dict]) -> pd.DataFrame:
 
 @pytest.fixture
 def patch_read():
-    # match_addresses_for_postcode now reads via the historic-EPC repository, so
-    # the S3 read is patched where the repository binds it.
-    with patch.object(repo_mod, "read_csv_gz_from_s3") as m:
+    # match_addresses_for_postcode now reads through GzipCsvS3Client
+    # (infrastructure/s3) — not the old utils.s3 free function. Patch the
+    # client's read (it is called with the per-postcode key only; the bucket
+    # lives in the client) and stub boto3.client so the seam runs with no S3 and
+    # no AWS environment.
+    with patch("boto3.client"), patch.object(GzipCsvS3Client, "read_csv_gz") as m:
         yield m
 
 
@@ -164,11 +167,11 @@ class TestMatchAddressesForPostcode:
         assert scores == sorted(scores, reverse=True)
 
     def test_s3_key_built_from_default_root(self, patch_read):
+        # The default root's prefix threads into the per-postcode key; the
+        # bucket parsing is covered by HistoricEpcS3Repository's factory test.
         patch_read.return_value = _build_df([_row("47 GORDON ROAD", "100")])
         match_addresses_for_postcode("47 Gordon Road", "AB33 8AL")
-        patch_read.assert_called_once_with(
-            "retrofit-data-dev", "historical_epc/AB338AL/data.csv.gz"
-        )
+        patch_read.assert_called_once_with("historical_epc/AB338AL/data.csv.gz")
 
     def test_s3_key_respects_custom_root_with_trailing_slash(self, patch_read):
         patch_read.return_value = _build_df([_row("47 GORDON ROAD", "100")])
@@ -177,9 +180,7 @@ class TestMatchAddressesForPostcode:
             "AB33 8AL",
             s3_root="s3://my-bucket/some/prefix/",
         )
-        patch_read.assert_called_once_with(
-            "my-bucket", "some/prefix/AB338AL/data.csv.gz"
-        )
+        patch_read.assert_called_once_with("some/prefix/AB338AL/data.csv.gz")
 
     def test_missing_postcode_object_yields_empty_matches(self, patch_read):
         # A valid postcode with no stored shard is a normal miss, not an error:

infrastructure/s3/gzip_csv_s3_client.py

@@ -0,0 +1,22 @@
+from __future__ import annotations
+
+from io import BytesIO
+
+import pandas as pd
+
+from infrastructure.s3.s3_client import S3Client
+
+
+class GzipCsvS3Client(S3Client):
+    """Reads a gzipped-CSV S3 object into a pandas DataFrame.
+
+    The S3-facing half of the historic-EPC read: an :class:`S3Client` (injected
+    boto client + bucket) plus the gzip/CSV decode, so a Repo can depend on this
+    instead of the ``utils.s3`` free functions. ``low_memory=False`` so the wide,
+    mixed-type historic-EPC columns infer a dtype from the whole column rather
+    than per-chunk (which would otherwise split one column across object/float).
+    """
+
+    def read_csv_gz(self, key: str) -> pd.DataFrame:
+        raw = self.get_object(key)
+        return pd.read_csv(BytesIO(raw), compression="gzip", low_memory=False)

repositories/historic_epc/historic_epc_s3_repository.py

@@ -1,50 +1,61 @@
 from __future__ import annotations
 
-from collections.abc import Callable
-from typing import Optional
+from typing import Any
 
-import pandas as pd
 from botocore.exceptions import ClientError
 
-from domain.postcode import Postcode
 from datatypes.epc.domain.historic_epc import HistoricEpc
 from datatypes.epc.domain.historic_epc_matching import (
     map_historic_epc_pandas_row_to_domain,
 )
+from domain.postcode import Postcode
+from infrastructure.s3.gzip_csv_s3_client import GzipCsvS3Client
+from infrastructure.s3.s3_uri import parse_s3_uri
 from repositories.historic_epc.historic_epc_repository import (
     HistoricEpcRepository,
     PostcodeNotFound,
 )
-from utils.s3 import parse_s3_uri, read_csv_gz_from_s3
 
 DEFAULT_S3_ROOT = "s3://retrofit-data-dev/historical_epc"
 
-# (bucket, key) -> DataFrame. Injected so the dataset is sourced from S3 in
-# production or a fake in tests — the Repo holds no S3/HTTP code of its own
-# (mirrors GeospatialS3Repository).
-CsvGzReader = Callable[[str, str], pd.DataFrame]
-
 
 class HistoricEpcS3Repository(HistoricEpcRepository):
-    """Reads per-postcode ``data.csv.gz`` shards of the historic EPC backup."""
+    """Reads per-postcode ``data.csv.gz`` shards of the historic EPC backup.
 
-    def __init__(
-        self,
-        read_csv_gz: Optional[CsvGzReader] = None,
-        s3_root: str = DEFAULT_S3_ROOT,
-    ) -> None:
-        # Resolve the default reader at call time (not import time) so it stays
-        # patchable and the captured default can't go stale.
-        self._read_csv_gz: CsvGzReader = read_csv_gz or read_csv_gz_from_s3
-        self._s3_root = s3_root
+    The bucket and the raw read live in the injected :class:`GzipCsvS3Client`
+    (``infrastructure/s3``); the Repo only builds the per-postcode key and maps
+    rows to domain records, so it holds no S3/HTTP code of its own — the same
+    shape as :class:`UnstandardisedAddressListCsvS3Repository`, and unlike the
+    old ``utils.s3`` free-function dependency.
+    """
+
+    def __init__(self, client: GzipCsvS3Client, root_prefix: str) -> None:
+        self._client = client
+        self._root_prefix = root_prefix
+
+    @classmethod
+    def with_default_s3_client(
+        cls, s3_root: str = DEFAULT_S3_ROOT
+    ) -> "HistoricEpcS3Repository":
+        """Build a repository backed by a real S3 client for ``s3_root``
+        (``s3://bucket/prefix``).
+
+        The composition-root convenience constructor used by the lambda and the
+        ``match_addresses_for_postcode`` seam; tests inject a ``GzipCsvS3Client``
+        over a moto-mocked boto client through ``__init__`` instead.
+        """
+        import boto3
+
+        bucket, root_prefix = parse_s3_uri(s3_root)
+        boto_s3: Any = boto3.client("s3")  # pyright: ignore[reportUnknownMemberType]
+        return cls(GzipCsvS3Client(boto_s3, bucket), root_prefix)
 
     def get_for_postcode(self, postcode: Postcode) -> list[HistoricEpc]:
         if not postcode.is_valid():
             raise PostcodeNotFound(f"{postcode.value!r} is not a valid UK postcode")
-        bucket, root_prefix = parse_s3_uri(self._s3_root)
-        key = f"{root_prefix.rstrip('/')}/{postcode}/data.csv.gz"
+        key = f"{self._root_prefix.rstrip('/')}/{postcode}/data.csv.gz"
         try:
-            df = self._read_csv_gz(bucket, key)
+            df = self._client.read_csv_gz(key)
         except ClientError as e:
             if e.response.get("Error", {}).get("Code") in ("NoSuchKey", "404"):
                 return []

tests/infrastructure/test_gzip_csv_s3_client.py

@@ -0,0 +1,45 @@
+import gzip
+from collections.abc import Iterator
+
+import pytest
+from botocore.exceptions import ClientError
+from moto import mock_aws
+
+from infrastructure.s3.gzip_csv_s3_client import GzipCsvS3Client
+from tests.infrastructure import make_boto_client
+
+BUCKET = "gzip-csv-bucket"
+
+
+@pytest.fixture
+def gzip_client() -> Iterator[GzipCsvS3Client]:
+    with mock_aws():
+        boto_client = make_boto_client("s3")
+        boto_client.create_bucket(Bucket=BUCKET)
+        yield GzipCsvS3Client(boto_client, BUCKET)
+
+
+def test_read_csv_gz_decodes_gzipped_csv_into_dataframe(
+    gzip_client: GzipCsvS3Client,
+) -> None:
+    # arrange
+    csv = "ADDRESS,UPRN\n47 GORDON ROAD,100\n48 GORDON ROAD,200\n"
+    gzip_client.put_object("historical_epc/AB338AL/data.csv.gz", gzip.compress(csv.encode()))
+
+    # act
+    df = gzip_client.read_csv_gz("historical_epc/AB338AL/data.csv.gz")
+
+    # assert
+    assert list(df.columns) == ["ADDRESS", "UPRN"]
+    assert df.shape == (2, 2)
+    assert df["ADDRESS"].tolist() == ["47 GORDON ROAD", "48 GORDON ROAD"]
+
+
+def test_read_csv_gz_raises_no_such_key_when_object_missing(
+    gzip_client: GzipCsvS3Client,
+) -> None:
+    # act / assert — a missing object surfaces as a ClientError; translating that
+    # into a domain miss is the Repo's job, not the client's.
+    with pytest.raises(ClientError) as exc_info:
+        gzip_client.read_csv_gz("historical_epc/ZZ999ZZ/data.csv.gz")
+    assert exc_info.value.response["Error"]["Code"] == "NoSuchKey"

tests/repositories/historic_epc/test_historic_epc_s3_repository.py

@@ -2,21 +2,29 @@
 
 A reference-data lookup, not a Fetcher (ADR-0011): no live EPC API call. The
 adapter takes a normalised ``Postcode`` and reads
-``historical_epc/{POSTCODE}/data.csv.gz`` via an injected reader, so the tests
-exercise mapping + key construction + absence against a fake reader with no
-network.
+``historical_epc/{POSTCODE}/data.csv.gz`` through an injected
+``GzipCsvS3Client`` (infrastructure/s3) — never the ``utils.s3`` free functions.
+
+The client is wrapped over a tiny fake boto client (rather than moto) so the
+tests can assert the *exact* S3 key the repo builds and inject a non-missing
+``ClientError``; the real gzip/CSV decode in ``GzipCsvS3Client`` still runs.
 """
 
 from __future__ import annotations
 
+import csv
 import dataclasses
+import gzip
+from io import BytesIO, StringIO
+from typing import Any, Optional
+from unittest.mock import patch
 
-import pandas as pd
 import pytest
 from botocore.exceptions import ClientError
 
 from datatypes.epc.domain.historic_epc import HistoricEpc
 from domain.postcode import Postcode
+from infrastructure.s3.gzip_csv_s3_client import GzipCsvS3Client
 from repositories.historic_epc.historic_epc_repository import PostcodeNotFound
 from repositories.historic_epc.historic_epc_s3_repository import (
     HistoricEpcS3Repository,
@@ -25,23 +33,56 @@ from repositories.historic_epc.historic_epc_s3_repository import (
 # HistoricEpc requires every CSV column; derive the (upper-cased) column list
 # straight from the dataclass so it can never drift from the domain type.
 _COLS = [f.name.upper() for f in dataclasses.fields(HistoricEpc)]
+_ROOT_PREFIX = "historical_epc"
 
 
-def _row(address: str, uprn: object) -> dict[str, object]:
-    row: dict[str, object] = {col: "" for col in _COLS}
-    row["ADDRESS"] = address
-    row["UPRN"] = uprn
-    return row
+def _gzip_csv(rows: list[tuple[str, str]]) -> bytes:
+    """A gzipped CSV carrying every HistoricEpc column; only ADDRESS/UPRN set."""
+    buffer = StringIO()
+    writer = csv.DictWriter(buffer, fieldnames=_COLS)
+    writer.writeheader()
+    for address, uprn in rows:
+        row = {col: "" for col in _COLS}
+        row["ADDRESS"] = address
+        row["UPRN"] = uprn
+        writer.writerow(row)
+    return gzip.compress(buffer.getvalue().encode())
 
 
-def _df(rows: list[dict[str, object]]) -> pd.DataFrame:
-    return pd.DataFrame(rows, columns=_COLS)
+class _FakeBoto:
+    """A minimal stand-in for a boto3 S3 client: serves one canned object (or
+    raises a chosen ``ClientError``) and records the ``(Bucket, Key)`` of every
+    request, so the repo tests can assert the exact S3 location without a live
+    bucket."""
+
+    def __init__(
+        self, *, body: Optional[bytes] = None, error_code: Optional[str] = None
+    ) -> None:
+        self._body = body
+        self._error_code = error_code
+        self.calls: list[tuple[str, str]] = []
+
+    def get_object(self, *, Bucket: str, Key: str) -> dict[str, Any]:
+        self.calls.append((Bucket, Key))
+        if self._error_code is not None:
+            raise ClientError(
+                {"Error": {"Code": self._error_code, "Message": "x"}}, "GetObject"
+            )
+        return {"Body": BytesIO(self._body or b"")}
+
+    @property
+    def requested_keys(self) -> list[str]:
+        return [key for _bucket, key in self.calls]
 
 
-def test_get_for_postcode_maps_reader_rows_to_historic_epc_records():
+def _repo(boto: _FakeBoto) -> HistoricEpcS3Repository:
+    client = GzipCsvS3Client(boto, "retrofit-data-dev")
+    return HistoricEpcS3Repository(client, _ROOT_PREFIX)
+
+
+def test_get_for_postcode_maps_shard_rows_to_historic_epc_records():
     # Arrange
-    df = _df([_row("47 GORDON ROAD", "100")])
-    repo = HistoricEpcS3Repository(read_csv_gz=lambda bucket, key: df)
+    repo = _repo(_FakeBoto(body=_gzip_csv([("47 GORDON ROAD", "100")])))
 
     # Act
     records = repo.get_for_postcode(Postcode("AB33 8AL"))
@@ -53,14 +94,21 @@ def test_get_for_postcode_maps_reader_rows_to_historic_epc_records():
     assert records[0].uprn == "100"
 
 
+def test_builds_s3_key_from_postcode_and_root_prefix():
+    # Arrange
+    boto = _FakeBoto(body=_gzip_csv([("47 GORDON ROAD", "100")]))
+    repo = _repo(boto)
+
+    # Act — the Postcode value object has already normalised casing/spacing.
+    repo.get_for_postcode(Postcode("ab33 8al"))
+
+    # Assert
+    assert boto.requested_keys == ["historical_epc/AB338AL/data.csv.gz"]
+
+
 def test_non_empty_postcode_with_no_stored_object_returns_empty_list():
     # Arrange — a postcode whose shard does not exist in S3.
-    def missing(bucket: str, key: str) -> pd.DataFrame:
-        raise ClientError(
-            {"Error": {"Code": "NoSuchKey", "Message": "missing"}}, "GetObject"
-        )
-
-    repo = HistoricEpcS3Repository(read_csv_gz=missing)
+    repo = _repo(_FakeBoto(error_code="NoSuchKey"))
 
     # Act
     records = repo.get_for_postcode(Postcode("AB33 8AL"))
@@ -69,31 +117,9 @@ def test_non_empty_postcode_with_no_stored_object_returns_empty_list():
     assert records == []
 
 
-def test_builds_s3_key_from_postcode_and_default_root():
-    # Arrange
-    calls: list[tuple[str, str]] = []
-
-    def capture(bucket: str, key: str) -> pd.DataFrame:
-        calls.append((bucket, key))
-        return _df([_row("47 GORDON ROAD", "100")])
-
-    repo = HistoricEpcS3Repository(read_csv_gz=capture)
-
-    # Act — the Postcode value object has already normalised the casing/spacing.
-    repo.get_for_postcode(Postcode("ab33 8al"))
-
-    # Assert
-    assert calls == [("retrofit-data-dev", "historical_epc/AB338AL/data.csv.gz")]
-
-
 def test_non_missing_read_error_propagates():
     # Arrange — an error that is NOT a missing object must not be swallowed.
-    def denied(bucket: str, key: str) -> pd.DataFrame:
-        raise ClientError(
-            {"Error": {"Code": "AccessDenied", "Message": "nope"}}, "GetObject"
-        )
-
-    repo = HistoricEpcS3Repository(read_csv_gz=denied)
+    repo = _repo(_FakeBoto(error_code="AccessDenied"))
 
     # Act / Assert
     with pytest.raises(ClientError):
@@ -102,7 +128,7 @@ def test_non_missing_read_error_propagates():
 
 def test_empty_postcode_raises_postcode_not_found():
     # Arrange — Postcode normalises whitespace away, leaving an empty key.
-    repo = HistoricEpcS3Repository(read_csv_gz=lambda bucket, key: _df([]))
+    repo = _repo(_FakeBoto())
 
     # Act / Assert — an unusable key, distinct from a non-empty absent postcode.
     with pytest.raises(PostcodeNotFound):
@@ -111,7 +137,7 @@ def test_empty_postcode_raises_postcode_not_found():
 
 def test_malformed_postcode_raises_postcode_not_found():
     # Arrange — a non-empty but malformed postcode can't key a real shard.
-    repo = HistoricEpcS3Repository(read_csv_gz=lambda bucket, key: _df([]))
+    repo = _repo(_FakeBoto())
 
     # Act / Assert
     with pytest.raises(PostcodeNotFound):
@@ -119,13 +145,29 @@ def test_malformed_postcode_raises_postcode_not_found():
 
 
 def test_uprn_trailing_dot_zero_is_stripped():
-    # Arrange — pandas reads an integer UPRN column as float, so the CSV cell
-    # arrives as "151020766.0"; the domain UPRN must be the bare integer string.
-    df = _df([_row("47 GORDON ROAD", "151020766.0")])
-    repo = HistoricEpcS3Repository(read_csv_gz=lambda bucket, key: df)
+    # Arrange — pandas reads an integer UPRN column written with a decimal as
+    # float, so the cell stringifies to "151020766.0"; the domain UPRN must be
+    # the bare integer string.
+    repo = _repo(_FakeBoto(body=_gzip_csv([("47 GORDON ROAD", "151020766.0")])))
 
     # Act
     records = repo.get_for_postcode(Postcode("AB33 8AL"))
 
     # Assert
     assert records[0].uprn == "151020766"
+
+
+def test_with_default_s3_client_threads_bucket_and_key_from_s3_root():
+    # Arrange — the factory parses ``s3://bucket/prefix`` into the client's
+    # bucket and the repo's root prefix, so a read lands at the right location.
+    boto = _FakeBoto(error_code="NoSuchKey")
+    with patch("boto3.client", return_value=boto):
+        repo = HistoricEpcS3Repository.with_default_s3_client(
+            "s3://my-bucket/some/prefix/"
+        )
+
+    # Act
+    repo.get_for_postcode(Postcode("ab33 8al"))
+
+    # Assert — bucket from the URI authority, key from the URI path + postcode.
+    assert boto.calls == [("my-bucket", "some/prefix/AB338AL/data.csv.gz")]