[aleksUIX]

Summary

Fixes #68 and #70. POST /api/v1/orders wrote deal_id and quote_id straight through. An unknown deal_id persisted as a draft; a change request against that order then inherited the reference and reached pending_approval at material severity with validation_errors empty. An unknown quote_id did the same on a valid deal, so the order's quote disagreed with the deal's.

The adjacent hop already resolves its parent: POST /api/v1/deals returns 404 quote_not_found for the same missing quote. GET /api/v1/deals/{id} returns 404 deal_not_found. Orders now match that contract.

Omitted / null ids stay allowed. An order can still be drafted before commercial terms exist. A provided id must already be in storage.

Fix

In create_order, after get_storage():

• non-empty deal_id → storage.get_deal; miss → 404 deal_not_found (same detail as get_deal)
• non-empty quote_id → storage.get_quote; miss → 404 quote_not_found (same detail as book_deal / get_quote)
• nothing is persisted on either miss

For orders already stored with a phantom deal_id (the #68 change-request path), create_change_request does the same lookup after the order exists and before classifying severity or writing a CR. Empty / None is left alone so this does not collide with #65 (deal_id_required for dealless orders).

if deal_id:
    deal = await storage.get_deal(deal_id)
    if not deal:
        raise HTTPException(
            status_code=404,
            detail={"error": "deal_not_found", "message": f"Deal '{deal_id}' not found."},
        )
if quote_id:
    quote = await storage.get_quote(quote_id)
    if not quote:
        raise HTTPException(
            status_code=404,
            detail={"error": "quote_not_found", "message": f"Quote '{quote_id}' not found."},
        )

Out of scope (deliberate)

• Does not require a deal or quote on create. Draft-without-terms remains valid.
• Does not check that a resolvable quote_id belongs to the given deal (POST /api/v1/orders accepts and persists an unresolvable quote_id for a valid deal #70 explicitly untested).
• Does not take POST /api/v1/change-requests returns 500 for an order created without a deal_id #61 / fix: reject change requests for dealless orders #65: no None → "" write-side normalization, no deal_id_required 400. Those PRs compose: fix: reject change requests for dealless orders #65 rejects missing ids, this rejects present-but-unknown ids.
• Does not touch GAM reporting (gam_reporting_service consumption of order["deal_id"]). Once create refuses unknown ids, new orders cannot feed that path a phantom reference.

Test plan

• omitted ids still create a draft (existing test_create_order_returns_draft)
• seeded deal + quote still 200 and persist
• unknown deal_id → 404 deal_not_found, no order row (POST /api/v1/orders accepts an unresolvable deal_id, and the reference propagates into a material change request #68 curl)
• unknown quote_id → 404 quote_not_found, no order row (POST /api/v1/orders accepts and persists an unresolvable quote_id for a valid deal #70, including with a known deal)
• service-layer 404s match the HTTP tests
• change request against a booked order whose deal_id is not in storage → 404 deal_not_found, no CR record, never pending_approval with empty validation_errors
• uv run --locked ruff check src/ and ruff format --check clean
• full unit suite: 1445 passed

Closes #68
Closes #70

[garvitkaushik-123]

Heads up on sequencing, not a concern with the fix itself — your composition reasoning in the description checks out (verified independently: an empty/None `deal_id` never reaches your `storage.get_deal` lookup, so #65's `deal_id_required` and this PR's `deal_not_found` don't collide at runtime).

#65 is about to merge (Siraj + atc964 signed off, pushed the last fix). Both PRs insert a block at the identical spot in `create_change_request` — right after the order-exists check, before building diffs — so once #65 lands this branch will hit a literal (mechanical, not logical) merge conflict there on rebase. Nothing to redesign, just flagging so it's not a surprise: the two checks compose in either order (`if not deal_id: 400` then `if deal_id: ...404`, or vice versa), so resolving it should just mean keeping both blocks back to back.

[Sirajmx]

Conflicts with main now that #65 landed (both touch the same deal_id check in
create_change_request) — needs a rebase. The two checks compose cleanly once ordered right:
#65's deal_id_required (400, missing/empty) must run before #71's deal_not_found (404,
present-but-unknown), since an empty deal_id has nothing to resolve:

deal_id = order.get("deal_id")
if not deal_id:
    raise HTTPException(
        status_code=400,
        detail={"error": "deal_id_required", "message": f"Order '{request.order_id}' has no deal_id and cannot be changed."},
    )

# A non-empty deal_id on the order must still resolve.
deal = await storage.get_deal(deal_id)
if not deal:
    raise HTTPException(
        status_code=404,
        detail={"error": "deal_not_found", "message": f"Deal '{deal_id}' not found."},
    )

One test needs a one-line fix too: test_unresolvable_deal_id_does_not_create_pending_cr's
payload predates #64's required idempotency_key and 422s before reaching the deal_not_found
path it's testing. Add "idempotency_key": "idem-orphan-deal-1" to that payload.

Verified both changes together locally - unit + integration passed (0 failed), including
both #65's and #71's own test scenarios side by side. Happy to re-review once rebased.

[aleksUIX]

Updated the branch against main. Missing deal IDs still return deal_id_required before the lookup, and unknown IDs return deal_not_found. The orphan-deal regression now supplies its idempotency key. All 76 targeted tests and scoped lint pass.

[Sirajmx]

Thanks @jaanijuk for filing #68 and #70 — both reproduce exactly and are closed by this fix. And
thanks @aleksUIX for already merging current main into this branch and resolving the conflict
before I even got to re-check it — it fast-forwards cleanly now, zero conflicts. The deal_id
collision I'd flagged earlier is resolved correctly: create_order keeps
deal_id/quote_id optional (matches #101's own create_order semantics), while
create_change_request requires an existing order to already have a deal_id (400
deal_id_required) before checking whether it resolves (404 deal_not_found) — two legitimately
different rules for two different operations, not a contradiction.

Verified all four claimed behaviors live, independently: omitted ids still draft an order; an
unknown deal_id on create 404s deal_not_found with nothing persisted (closes #68); an unknown
quote_id 404s quote_not_found (closes #70); a change request against a booked order whose
deal_id points nowhere 404s deal_not_found with no CR record written — the actual #68
scenario (a phantom reference reaching pending_approval with empty validation_errors),
confirmed closed. Also sanity-checked the composed rule holds: an order with no deal_id at all
still correctly 400s deal_id_required on a change request, not swallowed by the new check.

Touched tests: 76/76 passed. Full suite: 1670 passed. Ruff clean.

Cross-repo: zero impact, confirmed by grep — buyer-agent only ever reads order status
(GET /api/v1/orders/{id}, /history); it never creates orders or files change requests against
the seller at all.

Scope is right where it should be: doesn't check a resolvable quote_id actually belongs to the
given deal_id (explicitly flagged as untested), and doesn't touch GAM reporting's consumption
of order["deal_id"] — reasonable, since refusing unknown ids at creation means new orders can't
feed that path a phantom reference going forward. Approving.