Phase 2 assessment · System Designer

Strong

Ownership table.

State	Owner	Lifetime	Writers
CONFIG	the process entry point	read once at start-up, then frozen	one, before anything else runs
_CONNECTION	a Database object	per process, but re-openable	the pool, nobody else
FEATURE_FLAGS	a FeatureFlags object	per request, so a flag flip takes effect without a deploy	the flag source, read-only to the app
_carrier_client	the composition root	per process	constructed once, never reassigned

The row that matters most is FEATURE_FLAGS. Today it is a dict frozen at import, so the one thing a flag exists for — changing behavior without a deploy — is exactly what it cannot do. Giving it a per-request lifetime is a behavior change, not a tidy-up, and it is worth saying so out loud.

Composition root.

def build(settings: Settings) -> ReturnsApp:
    database = Database(settings.dsn)
    labels = CarrierLabelService(settings.carrier_key, timeout=2.0)
    flags = FeatureFlags(source=settings.flag_url)
    returns = ReturnService(
        orders=SqlOrderRepository(database),
        labels=labels,
        flags=flags,
    )
    return ReturnsApp(returns)


if __name__ == "__main__":
    build(Settings.from_environment()).run()

Three things changed that are not about tidiness. Construction order is now a fact of the code rather than of the import graph. A test builds the same graph with an in-memory repository and a fake label service, in one line, with nothing to reset afterwards. And the carrier timeout moved from 30 seconds to 2, because it became visible at the point where somebody had to choose it.

Left global: nothing mutable. Settings.from_environment() reads the environment once, at the top of main, and returns a frozen object. The property that makes that safe is not that it is “only config” — it is that it is immutable after construction and has exactly one writer, so no reader can depend on when the write happened. If settings later need to be reloadable, that property disappears and it stops being safe.

Strong

Allowed-import table. Direction runs one way, from volatile to stable.

Package	May import	May not import
domain	the standard library only	application, adapters, api, common
application	domain	adapters, api, common
adapters	domain, and its vendor libraries	application, api
api	application, domain, and the composition root	adapters directly — it receives them wired
common	the standard library only	everything in this project

Three violations, and what each costs.

domain/eligibility.py imports common.models.ReturnRow. The eligibility rule — the most valuable thing in the package — now needs a database row to run. Cost: every test of the policy needs a schema, and a column rename edits a business rule. This is the one to fix first.

application/request.py imports CarrierHttpClient concretely. The use case is welded to HTTP, so the carrier cannot be faked and cannot be swapped. Cost: no test of the request flow can run without a network, which is why there are none.

common/models.py imports domain.money, while domain imports common. That is a cycle, and it is what common always becomes: not a library but a junction, where any change reaches everything. Cost: common cannot be versioned, extracted, or reasoned about, and its import graph makes the other two violations look local when they are not.

Fitness function. It fails today, three times, which is the point.

import ast
import pathlib

FORBIDDEN = {
    "returns.domain": ("returns.application", "returns.adapters",
                       "returns.api", "returns.common"),
    "returns.application": ("returns.adapters", "returns.api"),
    "returns.common": ("returns.domain", "returns.application",
                       "returns.adapters", "returns.api"),
}


def imported_modules(path):
    tree = ast.parse(path.read_text())
    for node in ast.walk(tree):
        if isinstance(node, ast.ImportFrom) and node.module:
            yield node.module
        elif isinstance(node, ast.Import):
            for alias in node.names:
                yield alias.name


def test_dependency_direction():
    violations = []
    for path in pathlib.Path("returns").rglob("*.py"):
        package = "returns." + path.relative_to("returns").parts[0]
        for module in imported_modules(path):
            if module.startswith(FORBIDDEN.get(package, ())):
                violations.append(str(path) + " imports " + module)
    assert not violations, violations

It checks direct imports only. Transitive violations slip through, which I am accepting for now and writing down: the direct check catches the way this rule is actually broken, which is somebody adding an import to get a job finished on a Friday.

Strong

1. Carrier 503 — an infrastructure failure, handled at the adapter. Detected by the HTTP adapter, which is the only code that knows what 503 means. Decided by the adapter for the retry (two attempts, 200 ms and 800 ms, jittered, only on 502/503/504 and timeouts — never on 4xx) and by the application layer for what happens when retries are exhausted: accept the return, mark the label pending, and hand the intent to a worker. The customer sees “your return is approved, the label is on its way” and an email when it arrives. The domain never learns that a carrier exists. Log: one line per attempt at debug, one warning when the request finally degrades to pending, with the correlation id and the return id.

2. Item is 34 days old — not an error at all. Detected by the EligibilityPolicy in the domain, which is the point of it existing. Decided by the domain too: it returns a Refused decision with a reason, rather than raising. The customer sees a plain explanation of the window and a route to appeal. Nothing is retried, because nothing failed. Log: info at most, or nothing — and definitely not the error stream, where a normal business outcome pollutes every alert threshold built on top of it.

3. Duplicate return for the order — a race, and probably a repeat. Detected by the SQL adapter as a unique-constraint violation, which is the database doing its job. Decided by the application layer, and the decision depends on the idempotency key that came in with the request: same key means this is a retry of an intent already satisfied, so return the original return’s id and label with the original result. A different key means a genuine second attempt to return the same order, which is a domain refusal (case 2’s shape: AlreadyReturned). Either way the customer never sees a constraint name. Log: info for the idempotent replay, with both keys, so a support agent can tell a duplicate click from a duplicate request.

The one that is not an error is case 2, and two things follow. First, a refusal is a value the domain returns, not an exception it raises: exceptions are for the paths the caller cannot handle, and every caller here handles a refusal. Second, refusals must not reach the error log, or the initiative’s own guardrail metric — how many returns are refused, and why — ends up buried in the same stream as carrier outages. That number is a product measure. It wants a counter, not a stack trace.

Case 3 is the interesting near-miss: a constraint violation looks like case 1 (infrastructure) and is often case 2 (a domain rule the schema happened to notice first). Deciding which it is needs the idempotency key, which is why the key belongs in the request contract rather than in the adapter.