Files
doormile_backend/docs/balanced-assignment-plan.md

152 lines
12 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Balanced auto-assignment: implementation plan
**Status:** Phases A and B built and tested, not committed or deployed (2026-10-06). Phase C is a server setting (`MILER_MAX_ACTIVE_BOOKINGS=20`).
### Phase B: what was built
- `internal/assignment/release.go` (new):
- **`releaseUnaccepted`** (runs at the start of every sweep): an assignment still `Assigned` on an order still `Miler_Assigned`, older than `ASSIGNMENT_ACCEPT_TIMEOUT_MINUTES` (default 10; 0 = off), goes back to the pool. The order returns to `Pending_Pickup` with no rider, and the assignment becomes `Reassigned` with a note. The customer stage is walked back (`cxstage.Release`), and the rider is set to Available if they hold nothing else. Every write is conditional, so a rider accepting at the same moment wins.
- **Only auto-assignments are released.** They're now labelled `Remarks = "Auto-assigned"` (`autoAssignedRemark`). A rider hand-picked by ops or hub staff is never taken away. (The hub console's manual assign records no "assigned by", so the label is the only reliable marker.) Assignments made before this change have no label and are never released.
- **`ridersToSkip` / `withoutRiders`** (applied in `selectMilerWithAI`): an order is never offered back to a rider it was released from, who rejected it, or who cancelled it.
- **`SweepNear`**: called from `MilerStartDuty` in a goroutine. It assigns the pending orders within 10 km of a rider who just started duty, instead of waiting up to 5 minutes for the next sweep.
- `pendingForSweep` now also loads the pickup coordinates (needed by `pendingNear`).
- Tests: `release_test.go` (settings, skip filter, distance) and `release_pg_test.go` (release after timeout; inside the timeout, accepted and manual orders never released; the rider freed when nothing else is held; a concurrent accept wins; a released or rejected order goes to another rider; the duty-start sweep only picks nearby orders).
- End to end on the real backend: a waiting order was assigned **0.19 s** after the rider started duty. An unaccepted order was released after the timeout and given to the other rider in the same sweep. Accepted and manual orders were untouched after 3 sweeps. Bulk split was still 3/3.
### Phase A: what was built
- **In hand** = `openStopsToday` (today's open, non-cancelled orders), the same count the ceiling uses. Using all open records ever would let stale, abandoned orders skew the balance.
- `selectMilerWithAI` keeps only the least-loaded riders (`leastLoaded`), then the AI or the fallback chooses among them. It now also returns the full pool.
- `pickBestFromCandidates` → `betterChoice`: in hand ↑, then session ↑ (`sessionStops`, counted since the open `milerdutylogs.loginat`), then distance ↑, then rating ↓.
- `finalizeChoice` (both commit paths): takes `pg_advisory_xact_lock(7270001)`, recounts the pool, keeps the original choice if it's still least loaded, otherwise picks the best of the least loaded. Returns `errNoRiderCapacity` if everyone is at the ceiling (the attempt is retried later). The AI decision id is dropped when the choice changes.
- Tests: `balance_test.go` (ranking) and `balance_pg_test.go` (50 over 5 → 10 each; 23 → 5/5/5/4/4; **50 concurrent → within 1**; newcomers catch up; tie-break; all at the ceiling → waits).
- End to end on the real backend: bulk upload of 9 orders with riders at 0, 2 and 5 km → **3/3/3**. A 4th rider then joins and 4 more orders arrive: the newcomer gets 3, the 4th goes to the nearest (all tied).
**Goal:** however many orders and riders there are, split the orders **equally** among the riders, with riders logging in and out all day.
---
## 0. Where things stand (context for whoever picks this up)
### Already built (2026-10-06)
| Fix | Where |
|---|---|
| Rider search falls back to `GEORADIUS` on Redis < 6.2; `/ready` shows `redis_geo` | `internal/milergeo/` (committed `9b94be1`) |
| Only **today's**, non-cancelled open orders count towards the per-rider limit | `internal/assignment/ai_layer.go` → `openStopsToday` |
| Only riders with **live GPS** (default 15 min) are offered orders | `ai_layer.go` → `milerHasFreshGPS`, setting `ASSIGNMENT_MAX_GPS_AGE_MINUTES` |
| Console cancel (single and bulk) **frees the rider** (closes the assignment) | `controllers/adminController.go` → `closeOpenAssignments` |
| **Pending-order sweeper:** retries every unassigned pending order every 5 min (last 72 h) | `internal/assignment/sweeper.go`, started in `main.go` |
| **No double assignment:** the order is claimed only if it still has no rider and isn't cancelled | `crm_assignment.go` → `claimBooking` (used by both commit paths) |
Tests: `internal/assignment/eligibility_pg_test.go`, `sweeper_test.go`, `controllers/cancel_frees_rider_pg_test.go` (real-Postgres tests skip unless `REGISTRY_TEST_DSN` is set).
### How a rider is chosen today (what this plan changes)
1. Find riders within **10 km** of the pickup (Redis GEO), whose status allows work, with live GPS, holding fewer than `MILER_MAX_ACTIVE_BOOKINGS` (default **3**) of today's open orders.
2. Ask the AI service (`routemate …/decide-assignment`). That endpoint currently returns **404**, so the backend falls back to:
```
score = distance_km + 2 × open_orders_today − 0.5 × rating (lowest wins)
```
**The problem:** distance dominates, so riders near the pickups get more orders and the split is not equal. `MILER_MAX_ACTIVE_BOOKINGS` is only a ceiling and can't make it equal. **A code change is required.**
### Relevant facts found in the code
- A rider **can't end duty** while holding Assigned/Accepted orders (`MilerEndDuty`).
- If a rider's app dies, their orders **stay with them**: nothing releases an order a rider never accepted. (`InternalReassign` exists in `adminController.go`, but nothing calls it automatically.)
- Nothing reacts when a rider **starts duty**: they only get orders from the next retry or sweep.
- Each duty session is recorded in `milerdutylogs` (`loginat`, `logoutat`).
- The rider app sends GPS about **every 30 s** in the background (`PUT /miler/location`).
---
## 1. The balancing rule
Riders come and go, so "same number of orders **today**" isn't fair: a rider logging in at 3 PM would get *every* new order until they catch up.
| Rule | With riders joining and leaving |
|---|---|
| Equal orders today | ❌ late joiners get flooded |
| **Equal orders in hand right now** ✅ | fair at every moment; a new rider gets a fair share of *new* orders; a rider who leaves just drops out |
**Rule:** each new order goes to the eligible rider holding the **fewest unfinished orders right now**. Ties are broken by:
1. fewest orders **this duty session** (since `milerdutylogs.loginat`);
2. nearest to the pickup;
3. highest rating.
Balancing happens **among riders near each pickup** (10 km), so each area balances its own riders.
| Situation | Expected |
|---|---|
| 5 riders, 50 orders | 10 each, at most 1 apart (if the ceiling allows) |
| 23 orders, 5 riders | 5, 5, 5, 4, 4 |
| 3 riders hold 4 each; 2 riders log in | the next 8 orders go to the 2 new riders, then everyone shares |
| A rider goes offline (no GPS) | gets nothing new; the others share |
**"In hand"** = assignments `Assigned`/`Accepted` on orders that aren't `Cancelled`, `Delivered` or otherwise finished. Note: for hyperlocal parcels the assignment stays open until delivery, which is correct, because the rider is still carrying it.
---
## 2. Code changes (`doormile_backend`)
### Phase A: equal split (core) · ~½ day
| File / function | Change |
|---|---|
| `internal/assignment/ai_layer.go` → `collectEligibleCandidates` | For each eligible rider, compute **in-hand count** (replaces the today-only count for ranking; the today-only count can stay as the ceiling check) and **session count** (assignments since the latest open `milerdutylogs.loginat`). Store both on `milerCandidate` / `aiCandidate`. |
| `ai_layer.go` → `selectMilerWithAI` | Before calling the AI, **keep only candidates with the minimum in-hand count**. The AI or fallback then chooses among them, so balance holds even when the AI endpoint returns. |
| `ai_layer.go` → `pickBestFromCandidates` | Replace the weighted formula with ordering by in-hand ↑, session count ↑, distance ↑, rating ↓. |
| `internal/assignment/crm_assignment.go` → `commitAssignment`, `customer_assignment.go` → `commitCustomerAssignment` | **Bulk safety:** 50 orders arriving at once run in parallel and would all pick the same "least-loaded" rider. Inside the commit transaction, lock the rider's `milerprofiles` row (`SELECT … FOR UPDATE`), **recount** their in-hand orders, and refuse (try the next candidate) if they're at the ceiling or no longer the least loaded. Keep `claimBooking` as is. |
### Phase B: riders joining and leaving · ~½ day
| File / function | Change |
|---|---|
| `controllers/milerAppController.go` → `MilerStartDuty` | **Rider comes online:** after duty starts (and the GPS is indexed), trigger an immediate sweep of pending orders near them (new helper in `sweeper.go`, e.g. `SweepNear(lat, lon)`). New riders get orders within seconds, not up to 5 min. |
| `internal/assignment/sweeper.go` | **Rider stops responding:** release orders that are still **Assigned** (never Accepted) after `ASSIGNMENT_ACCEPT_TIMEOUT_MINUTES` (new, e.g. 10): close that assignment as `Reassigned`, clear `pickupbookings.assignedmileruserid`, set the status back to `Pending_Pickup`, and let the sweeper reassign. **Never release an Accepted order.** Reuse the logic of `InternalReassign` where possible. |
### Phase C: settings (server, no code)
| Setting | Recommended | Purpose |
|---|---|---|
| `MILER_MAX_ACTIVE_BOOKINGS` | **20** on the server (code default stays 3) | safety ceiling only; balancing decides the split |
| `ASSIGNMENT_MAX_GPS_AGE_MINUTES` | 15 (exists) | only riders whose app is running |
| `ASSIGNMENT_ACCEPT_TIMEOUT_MINUTES` | 10 (new, Phase B) | release orders never accepted |
| `ASSIGNMENT_SWEEP_SECONDS` | 300 (exists) | pending-order retry interval |
| `ASSIGNMENT_SWEEP_MAX_AGE_HOURS` | 72 (exists) | older pending orders are left alone |
---
## 3. Tests (real Postgres, same pattern as `eligibility_pg_test.go`)
1. 5 riders, 50 orders assigned one by one → 10 each, max − min ≤ 1.
2. **50 orders created concurrently** → still balanced, no rider over the ceiling, every order exactly 1 rider.
3. 3 busy riders + 2 who just started duty → new orders go to the newcomers until level.
4. A rider with stale GPS gets nothing new.
5. An order still Assigned after the timeout is released and goes to the least-loaded rider; an **Accepted** order is never released.
6. Riders outside 10 km are never used.
7. A tie on in-hand count is broken by session count, then distance, then rating.
8. Starting duty triggers assignment of nearby pending orders (Phase B).
Plus a unit test for the new ranking (`pickBestFromCandidates`) and the new setting parser.
---
## 4. Rollout
1. Deploy **Phase A** with `MILER_MAX_ACTIVE_BOOKINGS=20`. For one day, compare orders per rider (should be within 1 of each other among riders in the same area).
2. Deploy **Phase B**.
3. If riders are sent too far, add a cap such as "prefer balance, but not more than X km further than the nearest eligible rider" (`ASSIGNMENT_BALANCE_MAX_EXTRA_KM`).
---
## 5. Unchanged
The 10 km radius, the live-GPS rule, cancel freeing the rider, the pending-order sweeper, `claimBooking`, manual assignment (`AssignMilerToBooking`), hub batch assign (`HubBatchAssign`, its own cap of 5), the express dispatch agent, and the customer-app flow.
---
## 6. Open points (decide before or during the work)
- **Distance vs balance:** do you want the extra-km cap from rollout step 3 from day one?
- **The AI endpoint** (`/api/v1/doormile/decide-assignment` on `routemate.workolik.com`) is missing. Once restored, it will choose only among the least-loaded riders (Phase A guarantees this).
- **Hub batch assign** has its own greedy logic and cap (5). Should it use the same balancing later?