# Phase 1 — reference solution Code in this directory: `p1_money.py`, `p1_return.py`, `p1_refund_policy.py`, `p1_ports.py`, `p1_carrier.py`, `p1_repository.py`. Verified by `verify_phase_1.py` (29 tests). --- ## 1.1 Coupling and cohesion There is no code for this one. `../DEFECT_MAP.md` is the marking scheme. **What a strong list has that a weak one does not:** a cost column. "`SETTINGS` is a global" is an observation. "`SETTINGS` is a global, so changing the restocking fee at runtime silently changes what every in-flight return is worth, and no audit record says who changed it" is a finding. **Consequence accepted.** Ranking by cost means some genuinely ugly code stays — the five order-system calls, for instance. Say so out loud; an inventory that implies everything must be fixed will be ignored wholesale. --- ## 1.2 The train wrecks → `p3_tolerant_reader.py` The reference for this lives in phase 3, because the same translation layer answers both exercises. `parse_order` names every field the system depends on, in one function, and returns an `Order` the rules can ask questions of. **Forces.** A translation layer is code you maintain and it lags the upstream schema. You pay that for one place to change when the order team renames a field, and for rules that read like rules. **A second valid solution.** If the upstream payload were used in exactly one place and were stable, indexing it directly would be honest and the wrapper would be ceremony. It is used in three places by a team you do not control, so it is not. --- ## 1.3 The refund strategy → `p1_refund_policy.py` Two strategies, not five, with a dict from reason to strategy. The gold-tier rule is inside `refund_less_restocking_fee` where it belongs — it is a modification of the fee, not a separate policy. **Forces.** Five reasons mapped to two functions is the smallest thing that removes the duplication. A class per reason would be five files that all say `return context.gross`. **Consequence accepted.** `STRATEGY_BY_REASON` is a dict, so a strategy cannot carry configuration. The first per-category fee override breaks this design. **A second valid solution — when you would choose it.** If refunds start needing per-category rates, partner-specific overrides, or a promotional calendar, the functions become objects with constructor arguments and the dict becomes a registry built at the composition root. The trigger to switch is a strategy that needs state, not a strategy count. --- ## 1.4 The generated module Delete `ai_helpers.py`. Its four responsibilities go to four owners: display formatting to the API layer, the reason vocabulary to `p1_refund_policy`, validation to the edge, and `parse_timestamp` to the tolerant reader where a bad timestamp becomes `OrderContractBroken` instead of `None`. **Forces.** Delete-and-rewrite is right for 120 lines with two callers. The discipline worth practising is the one that scales — characterize, then restructure — which is why the lab ships `tests/test_characterization_ai_helpers.py` pinning all four defect families before you touch anything. **Consequence accepted.** Deleting the cache costs a string rebuild per render. Measure before you replace it: an LRU with a size bound and an invalidation on write is the version worth having, and it is not worth having yet. --- ## 1.5 / 1.6 Reasons to change, and composition No separate module: the split is expressed by `p1_ports.py` (what the workflow depends on) and `p1_return.py` (what it decides). `submit` becomes: parse the order, ask the policy, ask risk, tell the aggregate, save, publish events. **Forces.** Decide-then-do buys table tests over the six eligibility outcomes without a database. It costs one extra type and one extra hop. --- ## 1.7 The carrier port → `p1_ports.py`, `p1_carrier.py` `ReturnLabels.buy(order_ref, destination_country) -> ReturnLabel`. The adapter owns the SDK, the DTO and the vendor's error codes; `SM-422` becomes `DestinationNotServed` (permanent) and everything else becomes `CarrierUnavailable` (retryable). That distinction is what exercise 3.3's retry needs and what the lab's swallowed `except` destroyed. **Forces.** A port with one implementation is speculative unless it buys something now. This one buys three things: substitutability in tests without patching, containment of the vendor's field names, and a place for exercise 3.1's build-versus-buy decision to stand. **Consequence accepted.** `service_level` is still the vendor's vocabulary (`GROUND`, `STANDARD`). `FlatRateLabels` exists to make that leak visible — writing a second implementation is the cheapest test of whether a port is real. Closing that leak means a domain enum and a mapping per carrier, which is worth doing when there is a second carrier and not before. --- ## 1.8 Money → `p1_money.py` Minor units in an `int`, currency attached, immutable, refuses floats at construction, refuses to add across currencies, rounds half-up at the minor unit, and exposes `to_minor_units()` for the payment boundary with no arithmetic on the way out. `Money.parse("76.49", "USD").to_minor_units()` is `7649`. The lab's `int(76.49 * 100)` is `7648`. **Forces.** `Decimal` is slower than `float` and more verbose at every call site. In a payments path that is not a trade-off, it is table stakes — the lab ships a test proving the ledger and the vendor disagree by a cent per refund. **Consequence accepted.** You can no longer store the schema's `REAL` column faithfully. That is what makes exercise 3.5 necessary, and it is the lab's central lesson: a correct domain model has a migration cost, and the migration is the hard part. **A second valid solution — when you would choose it.** Some systems store money as a `Decimal` with an explicit scale rather than an integer of minor units. Choose that when you deal in currencies with variable precision or in unit prices with four decimal places (fuel, telecom). For retail refunds in two-decimal currencies, integers are simpler and every arithmetic result is exact by construction. --- ## 1.9 The Return aggregate → `p1_return.py` One `ReturnStatus` enum with one spelling per state, transitions as methods that refuse illegal moves, and `review_reason` split into `denial_code` and `hold_reason` because it was carrying two facts. The important addition is `AWAITING_LABEL`. The lab strands returns in `APPROVED` when the carrier fails, and nothing ever moves them. Naming that state is what makes the failure findable and gives exercise 5.1 something to retry. **Forces.** Every rule inside the aggregate is a rule the API cannot bypass — and a rule that existing rows may violate. **Consequence accepted.** Rows already written as `APPROVED` need a decision. The reference takes the tolerant route: read both spellings, write one. That is expand/contract applied to a vocabulary, and it is the same shape as exercise 3.6. --- ## 1.10 One vocabulary `VALID_REASONS` is derived from `STRATEGY_BY_REASON` in `p1_refund_policy.py`, so a reason cannot exist without a refund rule and vice versa. The edge translates the wire into that vocabulary and rejects what will not translate. **The product question first.** Is `broken` a synonym the product wants, or a typo the API should reject? The reference rejects it: synonyms in an API vocabulary become undocumented behaviour that clients depend on. If the product does want synonyms, they belong in a declared alias map at the edge, not in a helper's `elif`. --- ## 1.11 The repository → `p1_repository.py` Two implementations, one contract test class (`RepositoryContract`) run against both. That is the honest mitigation for fake drift — not "we will remember". `count_for_customer_since` is a domain question with a domain answer, and the reference fixes D27 while it is there: denied returns no longer count toward the repeat-returner rule, because three denials that cost the business nothing should not send a customer's next legitimate return to a human. **Forces.** A repository plus a fake is two implementations to keep honest. It buys tests that run in microseconds and a domain that survives a storage change. **Consequence accepted.** Finance stays outside the repository, reading the schema directly. Forcing a reporting consumer through an aggregate-shaped interface produces N+1 queries and an unhappy analyst. Exercise 5.5 makes that a declared read model instead of an accident. --- ## 1.12 Fail fast Three separate things: - `opened_today() > cap` becomes `>=`. One character; the cap was 51. - `Money` refuses negatives at construction, so `refund_amount=-500` has no path into the database. - `fraud.assess` no longer swallows `OrderSystemUnavailable`. **The decision the exercise is really about.** When the order system is down, the watch-list rule cannot run. Three defensible answers: 1. **Fail the submission** — safest, and makes your availability a function of theirs. 2. **Proceed without the rule, and log that you did** — available, guardrail weakened for the duration, and the log lets you review what got through. 3. **Hold the return for a human** — safe, and spends the review budget you promised support would stay under 50/day. An order-system outage now looks like a support incident. The reference takes (2), because the watch-list rule is one of three fraud signals and the other two still work, and because (3) converts an upstream outage into a breach of a commitment to another team. If the watch-list rule were the *only* signal, (1) would be right. Whichever you choose, it must be visible in the logs — the lab's version is silent, and that is the actual defect.