Case study · Inheriting a system that already has customers

Modernizing Storefront While the Pilot Ships

Most leads do not get a clean sheet. They get a nine-year-old system with real customers, undocumented consumers, two recent incidents, thirty percent test coverage — and a product deadline that does not move. This is what the work actually looks like, decision by decision.

Reading 30 minWorking through it with your own system: 2–3 hoursPrerequisite: lessons 0033, 0035, 0037, 0039–0050, 0057, 0058

Mission tie-in: the capstone shows a well-run initiative on a system that mostly cooperates. This case shows the same judgment applied where the system does not. Nothing here is a rewrite; every move is a small, reversible step taken while the business keeps trading.

The situation on day one

You have just taken technical lead of the returns-resolution initiative — the same initiative framed in lesson 0039 and worked end to end in the capstone packet. The outcome is unchanged: median resolution for eligible domestic returns from 72 hours to under 4 hours, support-contact rate down 25% from 41%, fraud loss no worse, manual review queue at or below 50 cases per day, WCAG 2.2 AA on critical flows, pilot by 30 September. Today is Monday 4 August. That is eight weeks.

What is new is the system you must do it in.

Three groups read the database directly, and none of them asked permission — because until now nobody said they needed to.

Outside the company there is a fourth consumer you cannot deploy with. The partner API — GET|POST /api/v2/orders/{order_id}/returns — serves 63 marketplace sellers. The top three are 61% of partner volume; nine sellers have not deployed a client in two years. There is no versioning policy, no deprecation window, and no record of which fields anyone actually reads. "We will tell the partners" is not available to you. Whatever you do to returns must leave that payload byte-compatible.

And there are the two incidents that are still open in everyone's memory.

Finally, the constraint you did not choose: the payments dependency belongs to another team. Four engineers, their own roadmap. Their refund-service v3 — with per-request idempotency keys — is committed for 14 November. That is six weeks after your pilot. You will not get it moved, and you should not spend your credibility trying.

Decision 1 · Pin the behavior before touching it

The first week buys verification, not architecture. A characterization test does not assert what the system should do; it records what it does, so that any later change to that record is a decision someone made rather than a surprise someone shipped. Feathers' framing in Working Effectively with Legacy Code (ch. 13) is the one to hold: these tests are not a quality statement, they are a vise.

Which behavior to pin first is a risk question, not a coverage question. The team ranked candidates by what breaks silently and expensively:

The golden master is the technique that makes 900 untestable lines tractable: run the real function over a fixed sample of real inputs, record a small stable projection of the output, and assert byte-equality forever after. Projecting matters — recording the whole result would fail on every timestamp and make the test noise rather than a vise.

# tests/characterization/test_return_decisions.py
# Pins what Storefront DOES today. Not what the policy document says.
import json
from pathlib import Path

from storefront.returns.legacy import assess_return  # the 900-line function

GOLDEN = Path(__file__).parent / "golden" / "return_decisions.json"


def fingerprint(order_id: str) -> dict:
    """A small, stable projection of a large, unstable result."""
    result = assess_return(order_id)
    return {
        "order_id": order_id,
        "decision": result["decision"],            # approved | refused | manual
        "reason_code": result.get("reason_code"),
        "refund_cents": result.get("refund_cents"),
        "label_count": len(result.get("labels", [])),
    }


def test_return_decisions_are_unchanged(frozen_clock, seeded_db):
    recorded = json.loads(GOLDEN.read_text())
    actual = [fingerprint(row["order_id"]) for row in recorded]
    assert actual == recorded          # a diff is a behavior change, not a bug fix


def rerecord() -> None:
    """Run deliberately. The diff is reviewed in the pull request. Never in CI."""
    order_ids = [row["order_id"] for row in json.loads(GOLDEN.read_text())]
    GOLDEN.write_text(json.dumps([fingerprint(o) for o in order_ids], indent=2))

Three details make the difference between a vise and a nuisance. The sample is stratified: 2,000 orders drawn so that every reason code and every one of the 11 blob shapes appears at least 20 times, rather than 2,000 orders from last Tuesday that are all the same shape. The clock and the database are injected — a golden master that depends on now() re-records itself daily and pins nothing. Re-recording is a reviewed act: the rerecord() function exists so that intentional behavior changes appear as a readable diff in a pull request, with a human writing down why.

† Coverage went from 12% to 58% in the returns package in six days — and the team wrote almost no assertions about correctness. That is the point. Characterization tests are not the tests you keep forever; several were deleted in week six once the module's own example-based tests covered the same rules with intent. They are scaffolding that lets you move at all, and lesson 0037's claim — tests are the speed technology — is measurable here: the median returns pull request went from four days in review to under one.

Decision 2 · Find the seams, then protect behavior through them

Feathers' seam is the load-bearing idea of the whole case: a place where you can alter behavior without editing in that place. Every seam has an enabling point — somewhere you decide which behavior runs. In Python the useful ones are almost always object seams: a parameter with a default, a protocol, a factory. The legacy view below has none, which is why nothing about it can be tested.

# Before: no seam. The rule is welded to the request, the clock and the ORM.
class LegacyReturnView(View):
    def post(self, request, order_id):
        order = Order.objects.get(pk=order_id)
        blob = order.return_json or {}
        if (timezone.now() - order.delivered_at).days <= 30 and not blob:
            blob = {"state": "requested", "opened": request.POST.get("opened")}
            order.return_json = blob
            order.save(update_fields=["return_json"])
        return render(request, "returns/requested.html", {"order": order})

Two moves get new behavior into that method without a refactor you cannot verify.

Sprout. Write the new rule outside the legacy code, test it properly, and call it from a single line. Nothing existing is restructured; the risk of the change is the risk of one added call. This is the move for new behavior.

# Sprout: the new rule is born outside — tested, pure, and called from one line.
def classify_request(delivered_at, now, opened: bool) -> str:
    """New code. No Django, no database, no clock of its own."""
    if opened:
        return "manual_review"
    return "eligible" if (now - delivered_at).days <= 30 else "outside_window"


class LegacyReturnView(View):
    def post(self, request, order_id):
        order = Order.objects.get(pk=order_id)
        blob = order.return_json or {}
        blob["classification"] = classify_request(          # the sprout, one line
            order.delivered_at,
            timezone.now(),
            opened=bool(request.POST.get("opened")),
        )
        order.return_json = blob
        order.save(update_fields=["return_json"])
        return render(request, "returns/requested.html", {"order": order})

Wrap. Rename the existing function, give the old name to a new one that calls it, and put the new behavior around the outside. This is the move when you need something to happen on every existing call — which is exactly the shape of INC-2481's fix. The team could not make the payments team's endpoint idempotent, so they made their own intent idempotent, at the one seam every refund passes through.

# Wrap: the old body is untouched; the new behavior surrounds every call site.
def issue_refund(return_id: str, amount_cents: int) -> str:
    """The seam INC-2481 needed: a local record of intent, before the call."""
    claim, created = RefundClaim.objects.get_or_create(   # unique on return_id
        return_id=return_id, defaults={"amount_cents": amount_cents}
    )
    if not created:
        if claim.payments_reference is None:
            raise RefundInFlight(return_id)   # a human decides; never retry blind
        return claim.payments_reference       # the retry storm stops here
    claim.payments_reference = _issue_refund_legacy(return_id, amount_cents)
    claim.save(update_fields=["payments_reference"])
    return claim.payments_reference

Note what the wrap does not do: it does not retry, and it does not guess. A claim row with no reference means the call may or may not have reached the gateway, and the honest response is to stop and surface it — the fail-fast rule from lesson 0014, applied to money. The number of in-flight claims became an alert with a threshold of one, and a daily reconciliation report against the payments team's ledger became the compensating control for a dependency the team does not own.

Decision 3 · Strangler fig or branch by abstraction?

These are often used as synonyms and they are not. Choosing wrongly costs weeks.

The team used both, in that order, and the order is the insight. Branch by abstraction creates the seam; the strangler routes traffic across it. Doing the strangler first gives you two divergent implementations with no shared contract; doing only branch by abstraction gives you a big-bang swap wearing an interface.

# The abstraction, created first. Both implementations satisfy it.
from typing import Protocol


class ReturnsBackend(Protocol):
    """The seam. Every caller in the monolith goes through this shape."""

    def assess(self, order_id: str) -> "ReturnDecision": ...

    def record_request(self, order_id: str, decision: "ReturnDecision") -> str: ...

    def to_partner_v2(self, order_id: str) -> dict: ...


class BlobBackend:
    """Reads and writes orders.return_json. Nine years of accumulated shapes."""


class ModuleBackend:
    """Reads and writes returns / return_lines / return_events."""

# The routing rule, added second. One function, one decision, logged once.
def backend_for(order, flags, log) -> ReturnsBackend:
    reason = "legacy_default"
    backend = BlobBackend()
    if order.channel == "partner_api":
        reason = "partner_never_routes"          # 63 sellers, no atomic deploy
    elif not order.is_domestic or order.is_guest:
        reason = "outside_pilot_slice"           # non-goals from the frame
    elif flags.enabled("returns_module", key=order.customer_id):
        backend, reason = ModuleBackend(), "cohort_enabled"
    log.info("returns.route", order_id=order.id, backend=type(backend).__name__,
             reason=reason, correlation_id=order.correlation_id)
    return backend

Four rules govern that router, and each was written down because breaking it has a name:

• The default is always the system that works. Every branch that is not an explicit "yes" returns BlobBackend. A flag service outage must degrade to the nine-year-old code path, not to an exception.
• Route by cohort, not by endpoint. The flag key is customer_id, hashed — so a customer who starts a return in the new module finishes it there. Routing per request would leave returns half-written in two places, which is the failure mode that makes people distrust strangler migrations.
• Partner traffic never routes. Until the compatibility evidence in section six exists, partner-originated orders are pinned to the legacy path regardless of the flag. One line, and it removes the whole class of "we broke a seller" risk from the first five weeks.
• The routing decision is observable. Every request logs which backend served it and why, under the same correlation id as the rest of the request. Without that line, a p95 regression is unattributable and someone will roll back the wrong thing.

† The machinery has a removal date. Lesson 0035's honesty clause applies: returns_module is a release flag, not a product setting. It is in the debt ledger with an owner and a date — 31 October, two weeks after 100% — and the BlobBackend class is deleted in the same pull request that removes it. A strangler that never finishes is just two systems.

Decision 4 · Expand and contract the blob, one deployable step at a time

Splitting orders.return_json into returns, return_lines and return_events is the riskiest thing in the eight weeks: 3.1M rows, three unseen consumers, and a table that checkout writes to 340 times a second. Expand/contract (Fowler and Sato call it parallel change) is the pattern; the discipline is that every row below is a separate deploy that can stand on its own indefinitely.

The lock-avoiding tactic, in full. INC-2503 was not caused by expensive DDL; it was caused by an ACCESS EXCLUSIVE request joining a queue behind a slow reader, after which PostgreSQL made every subsequent reader queue behind it (see the explicit-locking documentation). The fix is to make the lock request give up rather than queue.

# Django migration: the lock is the risk, not the DDL.
from django.db import migrations


class Migration(migrations.Migration):
    atomic = False   # CREATE INDEX CONCURRENTLY cannot run inside a transaction

    operations = [
        # Never wait in the lock queue. Fail in 2s and let the deploy retry.
        migrations.RunSQL("SET lock_timeout = '2s'",
                          reverse_sql=migrations.RunSQL.noop),
        migrations.RunSQL(
            "CREATE INDEX CONCURRENTLY IF NOT EXISTS returns_order_id_idx "
            "ON returns (order_id)",
            reverse_sql="DROP INDEX CONCURRENTLY IF EXISTS returns_order_id_idx",
        ),
    ]

Four rules, now enforced by a CI check rather than by a reviewer's memory: set lock_timeout on every migration; build indexes CONCURRENTLY; add constraints NOT VALID and VALIDATE them in a later deploy (validation takes only SHARE UPDATE EXCLUSIVE); and never ship DDL in the same release as the code that depends on it. The last rule is what makes every row of the table above independently revertible.

The dual-write window. Between step 2 and step 8 — about fourteen weeks, ten of them after the pilot — both forms are written in one transaction and the blob remains the source of truth until step 5 moves the reads. That window is the entire safety mechanism: at any moment before step 8, turning one flag off returns the system to a state that has been continuously exercised in production. It is also a real cost, which is why it is the first line of the debt ledger rather than an unspoken assumption.

The verification query. A dual write you do not check is a dual write that has already drifted. Every ten minutes, the team compares the two forms for every return touched in the last 24 hours: join orders to returns on order_id for rows whose return_json is non-null and whose updated_at is within the day, and select the pairs where the blob's state differs from the row's state, or the blob's refund_cents differs from the row's — using IS DISTINCT FROM so that a null on one side counts as a difference rather than silently matching. The threshold is not a percentage. One row is a bug, and the read switch does not advance while any row is returned.

# Runs every 10 minutes for the whole dual-write window.
DRIFT_SQL = """
SELECT o.id                                       AS order_id,
       o.return_json ->> 'state'                  AS blob_state,
       r.state                                    AS row_state,
       (o.return_json ->> 'refund_cents')::bigint AS blob_refund,
       r.refund_cents                             AS row_refund
FROM   orders o
JOIN   returns r ON r.order_id = o.id
WHERE  o.return_json IS NOT NULL
  AND  o.updated_at > now() - interval '24 hours'
  AND  (o.return_json ->> 'state' IS DISTINCT FROM r.state
        OR (o.return_json ->> 'refund_cents')::bigint
             IS DISTINCT FROM r.refund_cents)
LIMIT  500
"""


def check_drift(cursor, alert) -> int:
    cursor.execute(DRIFT_SQL)
    rows = cursor.fetchall()
    if rows:                      # one row is a bug; the read switch does not move
        alert("returns.dual_write.drift", count=len(rows), sample=rows[:5])
    return len(rows)

It found three real defects in week four: a partial-refund path that rounded differently in the two writers, an admin bulk action that wrote the blob directly and bypassed the seam entirely, and a Celery task that wrote outside the transaction so a rollback left the row behind. All three would have shipped invisibly under a big-bang cutover — and the third is precisely the class of bug that makes teams conclude, wrongly, that incremental migration "does not work".

Decision 5 · Compatibility contracts for an API you cannot change atomically

The partner API is the constraint that most cleanly separates a brownfield lead from a greenfield one. You have 63 consumers, nine of whom will not deploy anything this year, and no leverage over any of them. Lesson 0033's question applies directly: you cannot pay in coupling here, because a coupled deploy across 63 organizations does not exist. You will pay in coordination — and coordination only exists if someone writes it down.

The team's first act was archaeology, not design: replay a week of partner traffic and record which fields are actually read. They could not observe partner code, so they used the next-best evidence — the fields their own v2 serializer emits, and the fields the three largest sellers send back on write. Everything else is presumed load-bearing, because absence of evidence about a two-year-old client is not evidence of absence.

That last row is the one that turns a document into a contract. The rules are executable:

# The test that lets returns move without telling 63 sellers.
def assert_v2_compatible(legacy: dict, module: dict) -> None:
    """Every key the legacy response had, with the same value. New keys allowed."""
    missing = sorted(set(legacy) - set(module))
    assert not missing, f"v2 lost fields: {missing}"
    changed = sorted(k for k in legacy if legacy[k] != module[k])
    assert not changed, f"v2 changed the meaning of: {changed}"


def test_partner_payload_is_unchanged(recorded_partner_calls):
    for call in recorded_partner_calls:      # 4,100 responses replayed from July
        legacy = BlobBackend().to_partner_v2(call.order_id)
        module = ModuleBackend().to_partner_v2(call.order_id)
        assert_v2_compatible(legacy, module)

The same discipline runs in the other direction, because Storefront is itself a consumer it cannot control — of the payments team's refund API, which is being rebuilt on their schedule, not yours. Here the team is the tolerant reader: require what you use, keep what you do not understand rather than discarding it, and handle unknown values by policy instead of by exception.

# We consume payments v2 today and payments v3 in November. One reader, both.
KNOWN_OUTCOMES = {"settled", "pending", "declined"}


def read_refund_response(payload: dict) -> "RefundOutcome":
    """Tolerant reader: unknown values are a policy decision, not a crash."""
    outcome = payload["outcome"]
    if outcome not in KNOWN_OUTCOMES:
        outcome = "pending"          # treat the unknown as not-yet-final, and alert
    known = {"reference", "outcome", "amount_cents"}
    return RefundOutcome(
        reference=payload["reference"],
        outcome=outcome,
        amount_cents=payload["amount_cents"],
        unrecognized={k: v for k, v in payload.items() if k not in known},
    )

Two things are deliberate. Unknown outcomes become pending — the conservative state, because treating an unrecognized value as settled would lose money and treating it as declined would refund a customer twice — and they raise an alert, so tolerance never becomes silence. And unrecognized is kept rather than dropped: when the payments team's v3 adds a dedupe field in November, the evidence that it is arriving is already in the logs.

† Tolerance has limits, and lesson 0057 names them: closed enums, exhaustive matches, signature checks and payload size limits all make "additive" changes breaking. The v2 contract above closes the status enum on purpose — it is a promise that costs the team expressiveness and buys 63 sellers the right to write an exhaustive match. That is the trade, stated rather than discovered.

Decision 6 · Sequence by risk, and keep shipping product

The plan's ordering principle is lesson 0047's: each week must retire the largest remaining uncertainty and leave behind something demonstrable. Note where the product slice lands — week three, before the migration is anywhere near finished. That is not a compromise; it is the point. If the outcome cannot move until the modernization is done, the modernization has become the initiative.

Two things are deliberately absent. There is no "refactoring week" — every structural change above is attached to the slice that needs it, per lesson 0047's rule about free-floating architecture runway. And there is no week in which returns cannot be changed; the seam exists from week two precisely so that a production bug and the migration are not competing for the same code.

The dependency you do not own

The payments team's refund-service v3 will not exist before the pilot. Lesson 0047's rule is that "waiting on platform" is not a plan; a dependency needs an owner, a needed-by date, readiness evidence, a fallback and an escalation trigger. All five, written down in week one:

Decision 7 · The debt ledger, with interest and triggers

Everything above takes loans. Lesson 0038's rule is that a loan without a ledger entry is never repaid — and in a brownfield initiative the ledger is also the honest answer to "are we leaving it better than we found it?". Each entry names what was borrowed, what it charges per month, what event triggers repayment, and one person who owns it. A trigger that is a date nobody looks at is not a trigger.

The ledger is reviewed in the weekly initiative review alongside the outcome, not filed in a wiki. Two entries were closed during the eight weeks; five were carried past the pilot deliberately, with dates. That is a defensible position — what is not defensible is discovering them in January.

Decision 8 · Deciding not to extract a service

The strongest architectural decision in this case is a refusal. Returns stays inside the monolith as a module, with its own tables, its own package boundary and an enforced import rule — and no separate deployable. Here is the analysis, in the shape lesson 0046 asks for.

The module boundary is not a promise, it is a check. An import-linter contract in CI fails the build if anything outside storefront.returns imports its internals, or if the returns package imports the orders ORM models directly instead of going through a published read port — the fitness function that makes a modular monolith mean something. Without it, "module" is a directory name.

Who carries the pager

Extraction questions are usually argued as architecture and settled as operations. Three organizational consequences were written into the decision, because leaving them implicit is how a module becomes an orphan:

• Returns joins the existing Storefront rotation; it does not create a new one. Six engineers cannot staff a second rotation, and a rotation of two is a rotation that burns two people out. The cost is that a checkout engineer will be paged for a stuck return queue, so the runbook, the disable path and the support script were written for that person, not for the team that built it, and drilled in week seven with that person driving.
• Two alerts were added to the shared rotation and one was removed. Adding pages to someone else's night without removing any is how teams lose the right to add anything. The removed one was a returns error-rate alert that had fired 31 times in six months and been actioned zero times.
• The steady-state owner is named before the pilot ends, not after. Per lesson 0050: dashboards, runbook, vendor relationship, the debt ledger and the flag removal dates transfer to a named owner with a date. An initiative that ends without that transfer has produced software and no owner.

Decision 9 · Communicating incremental progress

Incremental modernization has a communication problem that big-bang rewrites do not: there is no launch. Week four looks, from outside, exactly like week two. If the only story you can tell is "still migrating", you will be asked to go faster in ways that reintroduce every risk you removed. The answer is to report evidence retired, not percentage complete — and to say the same true thing at two different depths.

Three properties make that useful rather than reassuring. It reports an outcome number (3.4 hours) rather than an activity number. It states the forecast as a range with a confidence and a named moving variable — lesson 0047's rule, and the language lesson 0050 insists on sharing while options still exist rather than when the miss is certain. And it ends with one decidable ask, not a status. "We are on track" would have been shorter and worth nothing.

The engineering version is not the executive version with more detail — it carries different decisions. It names the gate and refuses to negotiate it, it names the one behavior that would quietly destroy the safety net (re-recording the golden file), and it moves the ledger. The executive version carries the outcome, the range, and the ask. Neither version contains a sentence the other contradicts, which is the actual test of whether you understand your own progress.

What this case demonstrates

Read and work deeper

• Working Effectively with Legacy Code, Michael Feathers — ch. 4 (seams and enabling points), ch. 6 (sprout and wrap), ch. 13 (characterization tests), ch. 25 (the dependency-breaking catalogue). The source for sections one and two.
• "Strangler Fig Application" and "Branch By Abstraction", Martin Fowler — the two patterns section three distinguishes.
• "Parallel Change", Danilo Sato — expand/contract stated as a general change pattern, not only a database one.
• Monolith to Microservices, Sam Newman — ch. 3 (strangler, branch by abstraction, parallel run) and ch. 4 (decomposing the database, including the shared-database and view patterns used in step 7).
• Refactoring Databases, Ambler & Sadalage — the migration catalogue behind the step table, including the transition window.
• PostgreSQL documentation — explicit locking: the lock-level table that explains INC-2503 and the lock_timeout discipline.
• "Technical Debt", Martin Fowler — the loan framing the ledger in section seven makes operational.

Next: put this judgment under pressure. The decision-change simulations hand you a committed plan and then change the evidence underneath it.

Related lessons: 0033 (which currency you pay), 0035 (flags, strangler, branch by abstraction, expand/contract), 0037 (reversible or irreversible), 0038 (the ledger), 0047 (sequencing and dependencies), 0049 (gates and rehearsed reversal), 0057 (compatibility policy), 0058 (expand/contract and ownership). Reference: glossary — seam, characterization test, strangler fig, branch by abstraction, expand/contract, tolerant reader, feature flag.