# Phase 1 — Core Designer

Lessons 0001–0014. See the structure, then protect the domain core.

Work these in order; each one leans on the last. Before you start, run the demo
and read `api.py` → `service.py` → `policy.py`. An hour of reading beats an
hour of guessing.

**Ground rules.** The characterization tests in `tests/` describe the system as
it is, warts included. Refactors must not change them. If one goes red you have
changed behaviour — decide whether you meant to, and if you did, change the
test in the same commit with a message saying why.

To take on a fitness function, delete its `@checkpoint(...)` line and make it
pass.

---

## 1.1 Make the coupling and cohesion visible

**Lessons:** 0001 (Seeing Coupling & Cohesion), 0002 (Principles in Context)

**Do this.** Without changing any code, write a one-page inventory of this
system's structural problems. For each: where it is, what it costs, and what
kind of coupling it is (content, common, control, stamp, data — the ladder in
`../reference/design-principles.html`). Aim for fifteen; there are more than
thirty. Then rank them by *what they cost you today*, not by how ugly they are.

**Observable outcome.** No test changes. Your list, compared against
`solutions/DEFECT_MAP.md` — which you read only after you have written yours.
A good list catches at least: `policy.check_eligibility` doing rules and I/O,
`config.SETTINGS` as common coupling, the vendor DTO inside `service.py`, money
as a float, and `fraud.assess` counting denied returns as returns.

**Hints.**
- Cohesion question: for each module, what is the one sentence that describes
  everything in it? `ai_helpers.py` will not survive this question.
- Coupling question: if I change this line, whose tests break?
- `tests/support.reset_world()` is a measurement, not a helper. Its length is
  the amount of hidden state in the system.

**Forces to weigh.** Every item on your list is a claim that something costs
money. Some do not: five order-system calls per submit costs nothing while the
order system is fast and the volume is low. Ranking by cost rather than by
offence is the difference between a design review and a taste review.

---

## 1.2 Kill the train wrecks

**Lessons:** 0003 (The Law of Demeter)

**Do this.** `policy.py` and `service.py` navigate the order system's document:
`order["delivery"]["country"]`, `order["customer"]["email"]`,
`order["lines"][0]["final_sale"]`. Introduce a small `Order` /`OrderLine` view
that the client returns, and have the rules ask it questions
(`order.was_delivered()`, `order.line_for(sku)`, `order.destination_country`).

**Observable outcome.** `test_fitness_vendor_names.py::
test_the_order_system_answer_is_translated_before_the_domain_sees_it` goes
green. All of `tests/test_characterization_eligibility_and_fraud.py` stays
green.

**Hints.**
- Put the translation in `orders.py`, not in `policy.py`. The client's job is
  to turn their document into our vocabulary.
- Do not model the whole payload. Model the four facts the rules actually use.
- This is also the seam exercise 3.7 (tolerant reader) needs.

**Forces to weigh.** A translation layer is code you now maintain, and it will
lag the upstream schema. You pay that in exchange for one place to change when
they rename a field, and for rules that read like rules. If the upstream
payload were stable and used in exactly one place, the wrapper would be
ceremony — it is not, it is used in three.

---

## 1.3 Name the strategy that is already there

**Lessons:** 0004 (Spotting Emerging Strategies)

**Do this.** The refund calculation in `policy.check_eligibility` is an
`if/elif` chain over `reason`, with a customer-tier special case hidden inside
one branch. Four of the five branches are identical. Extract the calculation
into named strategies selected by reason, and make the tier rule explicit
rather than buried.

**Observable outcome.** `test_restocking_fee_is_applied_as_a_float`,
`test_gold_tier_pays_no_restocking_fee` and
`test_damaged_returns_are_refunded_in_full` still pass, and
`policy.check_eligibility` no longer contains an arithmetic branch.

**Hints.**
- The signal that a strategy is emerging: a conditional on a *type-like value*
  where each arm does the same shape of work.
- Four identical branches are not four strategies. Start with two — "full
  refund" and "refund less the restocking fee" — and a mapping from reason to
  strategy. Do not build five classes for five reasons.
- Keep the strategy pure: amount in, amount out. It should need no database.

**Forces to weigh.** A dict of five reasons to two functions is smaller and
clearer than a class hierarchy, and Python does not need the hierarchy. But
when a strategy starts needing configuration (a per-category fee, a partner
override), the function loses and the object wins. Choose for the strategies
you have, and note the trigger that would change your mind.

---

## 1.4 Review the module the assistant wrote

**Lessons:** 0005 (Structural Issues in AI Code), 0010 (Guiding AI Toward
Better Structure)

**Do this.** `returns_lab/ai_helpers.py` carries one comment:
`# generated by an assistant, reviewed: no`. Run lesson 0005's five-point
checklist over it — god functions and helper sprawl, hidden global state,
missing invariants, bare exception handling, missing seams. Write down what you
find *before* you look at the tests. Then fix it: delete what is dead, name the
abstraction for what it does, give the cache an owner and a bound or remove it,
and put the validation in one place.

**Observable outcome.** All three tests in `fitness/test_fitness_generated_code.py`
go green:
- `test_no_file_admits_it_was_never_reviewed`
- `test_no_module_level_cache_without_an_eviction_rule`
- `test_no_class_named_manager_or_helper_or_handler_utils`

`tests/test_characterization_ai_helpers.py` will go red — that is the point.
Every assertion in that file describes a defect, so each one you fix is a test
you delete or invert, deliberately, in the same commit.

**Hints.**
- Three branches in that file can never execute. Two of them look like
  defensive programming and one looks like a feature.
- `ReturnManager` has four methods; two of them are the same method.
- `_FORMAT_CACHE` is keyed by return id and never invalidated, so the summary
  freezes at whatever status it first saw — and because the cache is at module
  level, a fresh `ReturnManager()` inherits the stale entry.
- `validate_return_payload` and `api.submit_return` disagree about what a valid
  reason is. Pick one and delete the other; exercise 1.10 finishes the job.
- The comment marker is the last thing you remove, and only once you have
  actually read every line.

**Forces to weigh.** The cheap move is to delete the file and rewrite it. That
is often right — it is 120 lines and two callers. But the discipline worth
practising is Feathers': treat generated code as legacy code from an author who
will not answer questions, pin it with tests, then restructure. You will not
get to delete-and-rewrite when the generated module is 2,000 lines and has
eleven callers, so practise the move that scales.

---

## 1.5 Count the reasons to change

**Lessons:** 0006 (Designing Around Reasons to Change)

**Do this.** List every reason `ReturnsService` would have to be edited: a new
eligibility rule, a new fraud signal, a carrier swap, a payments swap, a new
notification channel, a new role, a schema change, a new status. For each,
write down which of its eight methods you would touch. Then propose a split —
on paper — and say which seam you would cut first and why.

**Observable outcome.** A written split with a first cut named. No code yet;
exercises 1.6, 1.7 and 1.11 execute it.

**Hints.**
- A reason to change usually has a person behind it. Who asks for it? Fraud
  analysts, finance, support ops and the carrier account manager are four
  different people with four different release cadences.
- The seam to cut first is the one with the highest (rate of change ×
  blast radius), not the one that is easiest to see.

**Forces to weigh.** Splitting a 220-line class into six modules for one team
of six people can be worse than leaving it: more files, more indirection, more
places to look. The argument for cutting is not tidiness — it is that carrier
changes and fraud changes arrive from different people on different weeks and
currently collide in one file.

---

## 1.6 Compose the workflow instead of branching it

**Lessons:** 0007 (Composition over Inheritance), 0008 (Strategy, Adapter,
Facade)

**Do this.** `ReturnsService.submit` is a straight line with three decision
points and four side effects. Rewrite it as a composition: a decision function
that takes the facts and returns *what should happen*, and a thin executor that
carries it out. No inheritance anywhere.

**Observable outcome.** `tests/test_characterization_submit.py` stays entirely
green, and `submit` fits on one screen with no `try` in it.

**Hints.**
- The decision is a value: "deny with reason X", "hold for review because Y",
  "approve for amount Z". Make that value explicit and the executor becomes
  boring.
- This is the shape that makes the eligibility rules testable without a
  database, which is what exercise 1.7 needs.

**Forces to weigh.** Splitting decide-from-do adds a type and an indirection.
It buys you a pure function you can table-test across every path — six
eligibility outcomes and two fraud outcomes — instead of twelve integration
tests each of which needs SQLite and three fakes.

---

## 1.7 Put the carrier behind a port

**Lessons:** 0009 (Ports & Adapters)

**Do this.** `service.issue_label` imports the vendor module, calls
`create_return_label`, and copies `label.trk` and `label.pdf_href` onto our
return. Define the port in our language — something like
`ReturnLabels.buy(order_ref, destination) -> ReturnLabel` — put the ShipMaster
call behind an adapter, and have the service depend on the port.

**Observable outcome.** Six fitness functions go green:
- `test_domain_modules_import_no_infrastructure_module`
- `test_no_vendor_name_appears_in_a_domain_module`
- `test_the_vendor_dto_never_crosses_into_the_service`
- `test_the_carrier_is_reachable_through_a_port_named_for_the_job` (3.1)

`test_the_vendor_dto_fields_are_copied_straight_onto_the_return` in
`tests/test_characterization_refund_and_label.py` will go red. That test exists
to record the leak; delete it when the leak is gone.

**Hints.**
- The port is named for the job (`ReturnLabels`), never for the seller
  (`ShipMasterGateway`). Lesson 0008 fixed exactly this mistake in its own
  example.
- The adapter owns the translation *and* the vendor's error type. Nothing above
  it should ever see `ShipMasterAPIError`.
- `SUPPORTED_COUNTRIES` is a product rule that currently lives in the vendor
  module. Decide where it belongs before you move it — exercise 3.1 revisits
  this.

**Forces to weigh.** A port you have one implementation of is speculative
generality unless it buys something now. It does: it makes the carrier
substitutable in tests without patching, it stops the vendor's field names
spreading, and it gives exercise 3.1 (build vs buy) somewhere to stand. Ports
that buy none of those three are ceremony.

---

## 1.8 Make invalid money impossible

**Lessons:** 0011 (Domain Invariants), 0012 (Value Objects & Aggregates)

**Do this.** `refund_amount: float` plus a separate `currency: str` allows
`-500.0`, allows `449.00` to be paired with `"GBP"` by accident, and loses a
cent on the way to the payment vendor: `int(76.49 * 100)` is `7648`. Introduce
a `Money` value object — minor units in an `int`, currency attached, immutable,
arithmetic on the object — and use it everywhere an amount travels.

**Observable outcome.** In `fitness/test_fitness_domain_invariants.py`:
- `test_refund_amount_is_not_a_float`
- `test_a_negative_refund_cannot_be_constructed`
- `test_amount_and_currency_travel_together`

`test_a_76_49_refund_reaches_the_vendor_as_7648_minor_units` goes red, and the
demo's step 2 prints the same number in both places for the first time.

**Hints.**
- Convert at the edge: `Money.from_major("76.49", "USD")` using `Decimal`, never
  `float`. Inside, it is an integer count of cents.
- The API still has to emit a JSON number for three consumer teams. That is a
  serialization decision at the boundary, not a reason to keep floats in the
  domain — but see exercise 3.6 before you change the wire format.
- Two `Money` values in different currencies must refuse to add.

**Forces to weigh.** You are now unable to store the schema's `REAL` column
faithfully, which is what makes exercise 3.5 (expand/contract) necessary and
what makes finance's nightly job a stakeholder in your refactor. This is the
lab's central lesson: a correct domain model has a migration cost, and the
migration is the hard part.

---

## 1.9 Give the Return an aggregate and its rules

**Lessons:** 0012 (Value Objects & Aggregates), 0011 (Domain Invariants)

**Do this.** `ReturnRequest` is a bag of mutable fields with a free-string
status. Turn it into an aggregate that owns its transitions: `submit`,
`deny(reason)`, `hold(reason)`, `approve()`, `attach_label(label)`,
`refund(payment_ref)`. Illegal moves raise. Status becomes a closed set with
one spelling per state.

**Observable outcome.** In `tests/test_characterization_submit.py`, the whole
`ReturnRequestHasNoInvariants` class goes red — delete it, deliberately, and
replace it with tests that assert the refusals. `SubmitStatusVocabulary` also
goes red: `APPROVED` and `approved` become one state.

**Hints.**
- The aggregate boundary is the Return and its transitions. The review case is
  a different aggregate with its own life — see exercise 5.2.
- `review_reason` is doing two jobs (a denial code and a fraud explanation).
  Separate them or the state machine will not close.
- Do not build the full state machine yet. Exercise 5.1 does that with a
  transition table; here, get the invariants enforced at the object.

**Forces to weigh.** Every rule you move into the aggregate is a rule the API
layer can no longer bypass — and a rule that now needs a migration path for
rows already in the database that violate it. Decide, and write down, what you
do with existing `APPROVED` rows: rewrite them, or accept both spellings on
read and emit one on write.

---

## 1.10 One vocabulary, one validation

**Lessons:** 0006 (Designing Around Reasons to Change), 0028 (Abstraction vs
Duplication)

**Do this.** The set of valid return reasons is defined in `ai_helpers.py`,
enforced differently in `api.py`, and reinterpreted a third time by
`normalize_reason`. The API rejects `reason="broken"`; the service maps it to
`damaged`. Pick one home for the vocabulary and one place that validates.

**Observable outcome.** In `fitness/test_fitness_domain_invariants.py`:
- `test_the_reason_vocabulary_is_defined_once`
- `test_incoming_payloads_are_validated_in_one_place`

`test_api_rejects_a_reason_the_service_would_have_accepted` and
`test_the_service_maps_that_same_reason_to_damaged` become contradictory —
resolve them into one decision and delete the loser.

**Hints.**
- Decide first *which behaviour is correct*: is `broken` a synonym the product
  wants, or a typo the API should reject? That is a product question, and the
  answer determines the code.
- The vocabulary belongs to the domain. The API's job is to translate the wire
  into it and reject what will not translate.

**Forces to weigh.** Two validators is duplication with a cost — they disagree,
and the disagreement is invisible until a customer hits it. But merging them
couples the wire format to the domain vocabulary, so adding a reason now means
a coordinated release with the client teams. Exercise 3.6 gives you the
versioning tool that makes that bearable.

---

## 1.11 Draw the repository boundary

**Lessons:** 0013 (Repository Boundaries)

**Do this.** `db.query` hands `sqlite3.Row` objects to callers,
`ReturnRequest.from_row` knows the column names, and `reporting.py` builds its
dictionaries out of the same schema. Introduce a `ReturnRepository` that speaks
in aggregates — `get(id)`, `save(ret)`, `for_customer(id)`,
`count_for_customer_since(id, when)` — and returns no rows and no SQL.

**Observable outcome.** `test_no_domain_module_contains_sql` (2.9) and
`test_domain_modules_import_no_infrastructure_module` (1.7) both need this to
stay green. Write a fake in-memory repository for the tests and watch how many
of them stop needing SQLite.

**Hints.**
- A repository is a collection from the domain's point of view. If a method
  name contains `select`, `join` or `row`, it is a query API, not a repository.
- `fraud.assess` needs a count, not a query. `count_for_customer_since` is a
  domain question; `SELECT COUNT(*) ... WHERE created_at >= ?` is an answer.
- Finance's job is a *reporting* concern and is allowed to read the schema
  directly. Do not force it through the repository; give it its own read model
  and say so. Exercise 5.5 revisits this.

**Forces to weigh.** A repository plus a fake is two more implementations to
keep honest, and fakes drift from the real thing. It buys fast tests and a
domain that survives a storage change. The honest mitigation is one contract
test run against both implementations — not "we will remember".

---

## 1.12 Fail fast at the right boundary

**Lessons:** 0014 (Fail-Fast Error Handling)

**Do this.** Two bugs and one policy question hide in error handling.
`agent_review.open_case` uses `>` where the cap is 50, so 51 cases fit.
`fraud.assess` swallows an order-system outage and silently skips a rule, so an
outage quietly weakens the guardrail. And `api.submit_return` has a catch-all
`except Exception` that turns any programming error into a 500 with no trace.
Fix the boundary: validate at the edge, raise inside, and let the edge decide
what the caller sees.

**Observable outcome.**
`test_the_daily_cap_lets_one_more_case_through_than_the_setting_says` goes red
(invert it), and
`test_an_order_system_outage_is_swallowed_and_the_watchlist_rule_skipped` needs
a decision — see below. `test_no_exception_is_swallowed_without_a_trace` (2.8)
gets closer to green.

**Hints.**
- "Fail fast" does not mean "raise everywhere". It means detect at the earliest
  point where the answer is knowable and refuse there, rather than carrying an
  invalid value forward.
- The cap check has a race as well as an off-by-one: two concurrent submits can
  both read 50. Naming it is enough for now; exercise 5.2 gives you the tool.

**Forces to weigh.** When the order system is down, the watch-list rule cannot
run. You have three options: fail the whole submission (safe, unavailable),
proceed without the rule (available, weaker guardrail), or hold the return for
a human (safe, and it spends the review budget you promised support would stay
under 50/day). There is no right answer here, only a decision with a named
cost — and whichever you pick, it must be visible in the logs, not silent.
Write down which you chose and what would change your mind. Exercise 3.4
returns to this as the consistency-versus-availability trade.

---

## Done when

- Your defect inventory exists and you have compared it to the defect map.
- These fitness functions are green: the four in 1.7, the three in 1.8, the two
  in 1.10, the three in 1.4.
- Every characterization test that went red went red *on purpose*, and each has
  a commit message saying which behaviour changed and why.
- `python3 -m returns_lab.demo` still runs and still prints a scenario you can
  follow.

Next: `phase-2.md`.