Changeset 9577c79 for docs/P4-Prototype/PrototypeImplementationAIUsage.md

Timestamp:

09/16/26 23:37:15 (13 days ago)

Author:

Stefan <trsunovstefan@…>

Branches:

main

Children:

8b447ef

Parents:

df05838

Message:

add reserved_quantity and modify the phases, add v_03.png and v_03.xml for P1

File:

• docs/P4-Prototype/PrototypeImplementationAIUsage.md (modified) (2 diffs)

docs/P4-Prototype/PrototypeImplementationAIUsage.md

@@ -39 +39 @@
 ## Summary of AI involvement
 
-| | Session 1 — 2026-04-21 | Session 2 — 2026-08-06/07 |
-|---|---|---|
-| **What I brought** | My existing Go backend (Chi HTTP handlers) and a half-finished frontend | The CLI prototype as it stood after session 1 |
-| **What the AI did** | Rewrote the backend as a CLI covering UC0001–UC0007, wrote the market bot | Reviewed the code, found and fixed three bugs, improved the holding upsert |
-| **What I decided** | To delete the frontend, to build a CLI rather than a web app, to keep the market simulator | To ask for a code review pass rather than documentation alone |
+| | Session 1 — 2026-04-21 | Session 2 — 2026-08-06/07 | Session 3 — 2026-09-16 |
+|---|---|---|---|
+| **What I brought** | My existing Go backend (Chi HTTP handlers) and a half-finished frontend | The CLI prototype as it stood after session 1 | A design review: the sell path had no way to reserve crypto committed to an order |
+| **What the AI did** | Rewrote the backend as a CLI covering UC0001–UC0007, wrote the market bot | Reviewed the code, found and fixed three bugs, improved the holding upsert | Added `holdings.reserved_quantity`, changed the sell path to reserve-then-settle, gave `Orders.status` a real lifecycle, tested it live including under real concurrency |
+| **What I decided** | To delete the frontend, to build a CLI rather than a web app, to keep the market simulator | To ask for a code review pass rather than documentation alone | To keep reserve+settle in one transaction rather than split it across two, since there is no cancel-order use case to recover a stuck reservation |
 
 The prototype was built in session 1 and worked. What session 2 added was a
@@ -116 +116 @@
 > presentation. You will be asked how the buy transaction works, and the answer
 > has to be yours.
+
+### Session 3 — 2026-09-16
+
+Prompted by a design review I did myself, logged in full in
+[ERModelAIUsage](../P1-ConceptualModel/ERModelAIUsage.md#session-3--2026-09-16).
+The report: a user who owns 2 BTC and places a sell order for 0.5 BTC has that
+crypto immediately removed from `quantity`, but nothing in the model recorded
+that a *pending* order had already committed part of a position before it
+settled — `holdings` had `quantity` and `avg_price` only, no equivalent of the
+`available_balance`/`invested_balance` split already used for cash.
+
+**Bug fixed**
+
+`server/trade.go`'s sell path checked `held < qty` directly against
+`holdings.quantity`. This happened to be safe against concurrent double-sells
+only because the whole operation — order, holding check, holding update,
+balance update, ledger, trade — runs inside one transaction with a
+`SELECT ... FOR UPDATE` lock. It was not safe against the actual scenario
+described: nothing distinguished "owned" from "owned, but already promised to
+this order," which matters the moment an order can legitimately sit `open`
+across more than one transaction — exactly what a limit-order matcher would
+need, and what `orders.status` already implied was coming.
+
+**What changed**
+
+1. `holdings.reserved_quantity numeric(20,4) NOT NULL DEFAULT 0 CHECK
+   (reserved_quantity >= 0 AND reserved_quantity <= quantity)` added to
+   `schema_creation.sql`.
+2. `v_portfolio` gained `reserved_quantity` and derived `available_quantity`.
+3. `trade.go`'s sell path now: locks the holding, computes
+   `available := quantity - reserved_quantity`, rejects if `available < qty`
+   (previously: rejects if `quantity < qty`), reserves
+   (`reserved_quantity += qty`), then settles (`quantity -= qty;
+   reserved_quantity -= qty`) — two statements instead of one, kept inside the
+   same transaction rather than split into two commits, which would risk an
+   order stuck `open` with reserved crypto and no cancel command to free it.
+4. Both `PlaceOrder` branches now insert the order as `status='open'` and
+   `UPDATE ... SET status='executed', executed_at=now()` at the end, instead
+   of inserting `'executed'` directly — `Orders.status` is now a real
+   lifecycle rather than a label written once.
+5. `portfolio.go` gained `Reserved`/`Available` columns, reading
+   `v_portfolio.reserved_quantity`/`available_quantity`, so the new field is
+   something a Trader can actually see.
+6. The error message on the sell path changed from
+   `"Insufficient holding: trying to sell X, hold Y"` to
+   `"Insufficient holding: trying to sell X, available Y (of Z held, W
+   reserved)"`, since "how much you hold" is no longer the only number that
+   matters.
+
+**Test evidence**
+
+Verified against PostgreSQL 16 (`bp_database`, `localhost:5433`):
+
+```
+$ eduberza sell 0.5 BTC  (Alice: 2.0000 BTC held, 0.0000 reserved)
+Order executed: sell 0.5000 BTC @ 67140.000000 (notional 33570.0000 USD)
+# holdings.quantity: 2.0000 -> 1.5000, reserved_quantity: 0.0000 (unchanged net of reserve+release)
+
+$ eduberza sell 1 ETH   (Bob: no holdings row at all)
+Insufficient holding: trying to sell 1.0000, available 0.0000 (of 0.0000 held, 0.0000 reserved)
+
+# Two concurrent processes, Alice at 1.5 BTC / 0 reserved, each selling 1.0 BTC:
+=== process A === Insufficient holding: trying to sell 1.0000, available 0.5000 (of 0.5000 held, 0.0000 reserved)
+=== process B === Order executed: sell 1.0000 BTC @ 67140.000000 (notional 67140.0000 USD)
+# final holding: quantity 0.5000, reserved_quantity 0.0000 — exactly one sell went through
+
+# Reserve visible mid-transaction, in one psql session (BEGIN; ...; COMMIT;):
+before:            quantity 2.0000, reserved_quantity 0.0000, available 2.0000
+after reserve:     quantity 2.0000, reserved_quantity 0.5000, available 1.5000
+after settle:      quantity 1.5000, reserved_quantity 0.0000, available 1.5000
+
+# The CHECK constraint holds even without going through trade.go:
+UPDATE holdings SET reserved_quantity = quantity + 1 WHERE ...;
+ERROR:  new row for relation "holdings" violates check constraint "holdings_check"
+```
+
+Full transcripts are on
+[UseCase0005Implementation](UseCase0005Implementation.md). The rejected-order
+rollback guarantee from session 2 was re-checked too: after both the
+insufficient-funds and insufficient-holding failure paths, the affected user
+still has zero new rows in `orders`.
+
+**What I decided:** to keep reserve and settle inside a single transaction —
+splitting them into two, so an order genuinely sits `open` and reserved
+between two commits, is what a real limit-order matcher will eventually need,
+but building that now would add a way for an order to get stuck without also
+building a way to cancel it, which is out of scope for this review. `go build
+./...` was run after every change; all seven use cases were re-exercised
+manually.