Freeze Warehouse Hold Tuples, Then Move Availability

A messy reserve function should not be rewritten in one pass. Freeze current hold decisions with characterization tests before edits. Move only the availability check after those tests stay green.

This walkthrough uses a warehouse reserve function as the specimen. The function mixes stock math, hold writes, and alert strings. The target is one safe extract, not a broad redesign.

Why this extract breaks

Branch order is part of the behavior in this kind of code. A stockout alert fires only after the bad-quantity check passes. Moving that check can change both the dict and the alert list.

Characterization tests store outputs for a fixed set of inputs. They do not claim those outputs match a product spec. They claim the next patch left those outputs unchanged.

The specimen

Keep this function as the public entry point during the extract. It returns a decision dict and mutates two caller lists. Those mutations are observable behavior, so the tests must see them.

def reserve_units(sku, requested, stock_rows, holds, alerts):
    on_hand = 0
    for row in stock_rows:
        if row["sku"] == sku and row["status"] == "sellable":
            on_hand += row["qty"]
    reserved = 0
    for hold in holds:
        if hold["sku"] == sku and hold["state"] == "open":
            reserved += hold["qty"]
    free = on_hand - reserved
    if requested <= 0:
        return {"decision": "deny", "qty": 0, "reason": "bad_qty"}
    if free <= 0:
        alerts.append("stockout:%s" % sku)
        return {"decision": "deny", "qty": 0, "reason": "none_free"}
    take = requested if requested <= free else free
    holds.append({"sku": sku, "qty": take, "state": "open"})
    if take < requested:
        alerts.append("partial:%s:%s" % (sku, take))
        return {"decision": "partial", "qty": take, "reason": "short"}
    return {"decision": "hold", "qty": take, "reason": "ok"}

The loops always run, even when requested quantity is zero. A draft that skips them can hide a bad row until later. Pin the current order instead of cleaning it up in this pass.

Negative free stock is possible when open holds exceed on-hand units. The current function treats that case as a stockout deny. Record that result, and do not repair it while extracting.

Rows that lock the branches

Six frozen rows cover the branches this function can take. Four of them are deny or partial paths, plus two hold paths. Add another row only when logs show a path you missed.

Case	Requested	Sellable	Same-sku open	Reason	Alerts	End holds
bad_qty_beats_stockout	0	0	0	bad_qty	none	0
none_free	2	4	4	none_free	stockout	1
oversold_is_stockout	1	2	5	none_free	stockout	1
partial_writes_hold_and_alert	5	4	2	short	partial	2
full_hold_ignores_damaged	2	6	2	ok	none	3
closed_hold_does_not_reduce_free	2	6	2	ok	none	3

The closed-hold control uses the same requested quantity as a full hold. Its closed row must not reduce the free unit count. A later edit that counts closed holds should fail this control.

The full-hold row also carries an open hold for a different sku. That foreign hold must not change the free count for BOLT-4. End length still rises by one, because the new hold is appended.

Step 1. Put fixtures in one module

Store every input and expected tuple in one fixture module. Tests should import that module rather than hiding numbers inline. A reviewer can then diff data without reading assertion noise.

# warehouse/hold_cases.py
CASES = [
    {
        "name": "bad_qty_beats_stockout",
        "sku": "BOLT-4",
        "requested": 0,
        "stock_rows": [
            {"sku": "BOLT-4", "status": "sellable", "qty": 0},
        ],
        "holds": [],
        "want": {"decision": "deny", "qty": 0, "reason": "bad_qty"},
        "alerts": [],
        "hold_count": 0,
    },
    {
        "name": "none_free",
        "sku": "BOLT-4",
        "requested": 2,
        "stock_rows": [
            {"sku": "BOLT-4", "status": "sellable", "qty": 4},
        ],
        "holds": [
            {"sku": "BOLT-4", "qty": 4, "state": "open"},
        ],
        "want": {"decision": "deny", "qty": 0, "reason": "none_free"},
        "alerts": ["stockout:BOLT-4"],
        "hold_count": 1,
    },
    {
        "name": "oversold_is_stockout",
        "sku": "BOLT-4",
        "requested": 1,
        "stock_rows": [
            {"sku": "BOLT-4", "status": "sellable", "qty": 2},
        ],
        "holds": [
            {"sku": "BOLT-4", "qty": 5, "state": "open"},
        ],
        "want": {"decision": "deny", "qty": 0, "reason": "none_free"},
        "alerts": ["stockout:BOLT-4"],
        "hold_count": 1,
    },
    {
        "name": "partial_writes_hold_and_alert",
        "sku": "BOLT-4",
        "requested": 5,
        "stock_rows": [
            {"sku": "BOLT-4", "status": "sellable", "qty": 4},
            {"sku": "BOLT-4", "status": "damaged", "qty": 9},
        ],
        "holds": [
            {"sku": "BOLT-4", "qty": 2, "state": "open"},
        ],
        "want": {"decision": "partial", "qty": 2, "reason": "short"},
        "alerts": ["partial:BOLT-4:2"],
        "hold_count": 2,
    },
    {
        "name": "full_hold_ignores_damaged",
        "sku": "BOLT-4",
        "requested": 2,
        "stock_rows": [
            {"sku": "BOLT-4", "status": "sellable", "qty": 6},
            {"sku": "BOLT-4", "status": "damaged", "qty": 20},
        ],
        "holds": [
            {"sku": "BOLT-4", "qty": 2, "state": "open"},
            {"sku": "NUT-1", "qty": 100, "state": "open"},
        ],
        "want": {"decision": "hold", "qty": 2, "reason": "ok"},
        "alerts": [],
        "hold_count": 3,
    },
    {
        "name": "closed_hold_does_not_reduce_free",
        "sku": "BOLT-4",
        "requested": 2,
        "stock_rows": [
            {"sku": "BOLT-4", "status": "sellable", "qty": 6},
            {"sku": "BOLT-4", "status": "damaged", "qty": 20},
        ],
        "holds": [
            {"sku": "BOLT-4", "qty": 2, "state": "open"},
            {"sku": "BOLT-4", "qty": 9, "state": "closed"},
        ],
        "want": {"decision": "hold", "qty": 2, "reason": "ok"},
        "alerts": [],
        "hold_count": 3,
    },
]

These listings are proposed examples for a local checkout. No pytest run was recorded while preparing this draft. Treat the expected tuples as the contract you must confirm locally.

Step 2. Call only the public function

Call reserve_units only from the characterization test module. Do not assert on a helper that does not exist yet. Copy both lists before each call so cases stay isolated.

# tests/test_reserve_characterize.py
import copy

import pytest

from warehouse.hold_cases import CASES
from warehouse.reserve import reserve_units


@pytest.mark.parametrize("case", CASES, ids=lambda case: case["name"])
def test_reserve_matches_frozen_case(case):
    holds = copy.deepcopy(case["holds"])
    alerts = []
    got = reserve_units(
        case["sku"],
        case["requested"],
        copy.deepcopy(case["stock_rows"]),
        holds,
        alerts,
    )
    assert got == case["want"]
    assert alerts == case["alerts"]
    assert len(holds) == case["hold_count"]

Deepcopy matters because the function appends into the hold list. A shared list would leak one case into the next case. That leak looks like a product bug and wastes the review.

Assert the decision dict, the alert list, and the hold length. Those three checks catch a pure math move that drops a write. They also catch an alert rewrite that keeps the decision string.

Add an empty init file so the warehouse imports resolve cleanly. Run pytest from the repository root, not from inside tests. A wrong working directory makes this failure look like drift.

mkdir -p warehouse tests
python -m pytest tests/test_reserve_characterize.py -q --tb=short

Step 3. Run one command twice

Use the same command before the edit and after the edit. A changed flag or path can hide a real behavior drift. Store the short output beside the patch notes for review.

python -m pytest tests/test_reserve_characterize.py -q --tb=short
git diff --stat -- warehouse/reserve.py

Install pytest in the environment you already trust for this repo. This walkthrough does not pin a pytest release or Python minor. Use the versions your repository already runs in review.

Treat a red first run as a bad expected tuple. The legacy function is the oracle for this characterization pass. Update the fixture until it matches the current return values.

Do not fix the function while you are still writing fixtures. A mixed commit makes the next failure ambiguous for reviewers. Land the fixture file only after the first run is green.

Step 4. Ask for a draft, then discard most of it

This workflow uses MonkeyCode for free model access and a free server. Point that model at the fixtures, and point that server at pytest. Disclosure: This article was prepared as part of MonkeyCode's product outreach.

Give the model the fixture module and the current function only. Ask for one new function and a single call-site swap. Reject any draft that edits alert text or return keys.

Extract free-unit math from reserve_units into free_units.
Keep reserve_units as the only public entry point.
Do not change decision strings, alert text, or append order.
Do not add imports. Do not coerce qty values.
Return a unified diff and stop.

This draft states no quotas, hardware sizes, or access duration. Check current product terms before you depend on a long session. If the remote run disappears, rerun the same file locally.

Step 5. Keep the smallest diff

Extract a function that only returns free units for one sku. Leave every branch, string, and list append inside reserve_units. Pass stock rows and holds through without sorting or filtering elsewhere.

def free_units(sku, stock_rows, holds):
    on_hand = 0
    for row in stock_rows:
        if row["sku"] == sku and row["status"] == "sellable":
            on_hand += row["qty"]
    reserved = 0
    for hold in holds:
        if hold["sku"] == sku and hold["state"] == "open":
            reserved += hold["qty"]
    return on_hand - reserved


def reserve_units(sku, requested, stock_rows, holds, alerts):
    free = free_units(sku, stock_rows, holds)
    if requested <= 0:
        return {"decision": "deny", "qty": 0, "reason": "bad_qty"}
    if free <= 0:
        alerts.append("stockout:%s" % sku)
        return {"decision": "deny", "qty": 0, "reason": "none_free"}
    take = requested if requested <= free else free
    holds.append({"sku": sku, "qty": take, "state": "open"})
    if take < requested:
        alerts.append("partial:%s:%s" % (sku, take))
        return {"decision": "partial", "qty": take, "reason": "short"}
    return {"decision": "hold", "qty": take, "reason": "ok"}

The new function contains the two loops and nothing else. reserve_units still decides deny, partial, and full hold. Alert strings stay in the orchestrator, beside the returns they describe.

Step 6. Re-run and stop

Re-run the same pytest command against the same fixture module. Green means this change may include the extract and nothing else. Red means revert that diff and leave the fixtures in place.

Do not fold a reason-string cleanup into the green commit. Do not rename decision values while this review is still open. Those edits need new expected tuples and a separate review.

Reject signals before you paste

Read the reject table before you apply a model draft. The accept condition is one helper plus unchanged green tests. Any wider accept condition turns the check into a suggestion.

Diff signal	Result	Reason
Second helper or class added	Reject	Review surface exceeds one check
Alert format or decision key changed	Reject	Callers can parse those strings
Test file edited in the extract commit	Reject	The recorded oracle was replaced
New import or qty coercion added	Reject	Integer rows will still pass
Branch order inside reserve_units changed	Reject	bad_qty must still beat stockout
One helper added, tests untouched and green	Accept	Planned scope for this pass

Integer fixtures will not catch a silent int coercion. A draft can add int calls and still pass these rows. Note that gap in the patch notes, and do not claim full coverage.

A skipped loop on zero quantity can also slip through these rows. The bad-quantity row never reads a poisoned stock value today. Add a raising row only if your real data can contain one.

What green does not mean

Green tests do not prove the hold rules are commercially fair. They prove the listed inputs still return the same tuples. Unlisted skus, races, and lock waits remain outside this file.

This specimen has no clock, random source, or network call. If your real function has those, inject them before freezing outputs. Otherwise the fixture records one accident of the test environment.

A model draft can still reorder branches inside the new helper. A remote runner can still use a different Python patch level. Pin the runtime in the project file when that split matters.

Who should not use this pass

Skip this pass when the commit must change user-visible behavior. Skip it when you cannot execute the function fully offline. Skip it when a hold write is unsafe to repeat in tests.

Payment capture, authz decisions, and irreversible deletes need stricter review. This output check is too weak to clear those paths alone. Pair a human review with an explicit rollback step there.

After the fixtures stay green

Leave the characterization module in the tree after the extract. It is the fence for the next small move, not a temporary scaffold. The next candidate is alert ownership, not another math rewrite.

If a free MonkeyCode server is already available, run the file there. Compare that output with your local run before you merge. Keep the line-by-line diff review even when both runs are green.