mirror of
https://github.com/Tria-plc/edr-platform.git
synced 2026-08-26 18:42:49 +00:00
186 lines
11 KiB
Markdown
186 lines
11 KiB
Markdown
# Scenario ↔ engine reconciliation (S1–S40)
|
||
|
||
Verified against the API source while writing the Group 1 specs. Every claim
|
||
below carries a `file:line`; re-check them before trusting this file, it is a
|
||
snapshot of the code as of the freight_feature/usermanagement branch.
|
||
|
||
The point of this file: several scenarios in the original 40-case document
|
||
describe behaviour the engine does **not** implement. Those are not spec bugs
|
||
to code around — they are either product gaps worth a ticket, or scenarios
|
||
whose premise needs restating. Writing a green test against a premise the code
|
||
contradicts is worse than having no test.
|
||
|
||
## Capacity: 53 vs 54
|
||
|
||
The scenarios are written for a **53-wagon** train. A loco-pair schedule cannot
|
||
hold 53 on this corridor: `syncScheduleMaxWagons` recomputes `max_wagons` as
|
||
`floor(locoLength / shortest active wagon length)`, and `seed-import-corridor.sql`
|
||
deliberately pins that at `floor(760 / 13.966) = 54`.
|
||
|
||
A **built train** is exempt — `booking-batch.service.ts:4152`:
|
||
|
||
const maxWagons = physicalWagons ?? capacityLimits(loco).base.wagons;
|
||
|
||
So Group 1 runs on `TRN-G1-1`, a 53-wagon built consist (`seed-g1-train.sql`).
|
||
Note `train-capacity.util.ts:107` calls 53 "the marshalling figure" and says the
|
||
slot count is "never a fixed 53" — the number is real, it just has to come from
|
||
a consist rather than from locomotive length.
|
||
|
||
## Confirmed — scenario matches the engine
|
||
|
||
| Scenario | Engine fact | Where |
|
||
| --- | --- | --- |
|
||
| S1/S5 cascading promotion | `fillFromWaitingList` loops up to 10 rounds until a pass reserves nothing | `booking-batch.service.ts:2788` |
|
||
| S4/S6 split offers | `booking_batch_offers`, `status` defaults `OFFERED`, `offered_wagons` | `booking-batch-offer.entity.ts:27,46,73` |
|
||
| S9 heavy VGM | NW5 tare **22.4T** → 2×28 + 22.4 = 78.4T gross, exactly as the scenario computes | `train-capacity.util.ts:16,82` |
|
||
| S11 split ignores tolerance | tolerance "spendable only by admitting a booking whole, never by a split" | `train-capacity.util.ts:58`, `booking-batch.service.ts:4109` |
|
||
| S13 holds occupy space | reserved = `SELECTED_FOR_BATCH`/`AWAITING_PAYMENT`, subtracted from capacity until the deadline lapses | `bookings.repository.ts:1372`, `booking-batch.service.ts:4487` |
|
||
| S18 oversized export rejected | 409 at `requestOperation` with a sized message | `booking-transition.service.ts:1016`, `booking-batch.service.ts:900` |
|
||
| S25 per-station arrival | checkpoint at an intermediate yard auto-unloads bookings destined there | `booking-journey.service.ts:261` |
|
||
| S30 cancel snapshot | `train_schedules.wagon_allocation_snapshot` jsonb, frozen before wagons are released; holds wagon numbers + per-slot weights | `train-scheduling.service.ts:3984`, `4647` |
|
||
| S31 wagon transfer | statuses `PENDING/PARTIALLY_FULFILLED/FULFILLED/CLOSED_SHORT/CANCELLED` | `packages/types/src/freight/index.ts:353` |
|
||
| S31 movement ledger | `wagon_movements`, kind `EMPTY_REPOSITION`, carries `transfer_request_id` | `wagon-movement.entity.ts:16,49,55` |
|
||
| S37 risk history | `clearance_milestones.metadata` → `riskLevel` + append-only `riskHistory` (oldest-first) | `clearance-milestone.entity.ts:38`, `clearance-milestone.service.ts:250` |
|
||
| S38 max 2 per truck | `MAX_CONTAINERS_PER_TRUCK = 2`, error `A truck carries at most 2 containers` | `truck-load.util.ts:5,36` |
|
||
| S38 duplicate container | 409 `Container X is already loaded onto another truck` | `truck-load.util.ts:52` |
|
||
| S40 re-price parks the booking | batch pool is `status = 'PAID'` exactly, so `PRICE_CHANGED_PENDING_CONFIRM` holds zero capacity | `bookings.repository.ts:1136` |
|
||
|
||
## Corrected — same intent, different mechanism
|
||
|
||
**S7 government.** The scenario says government "jumps the queue" via an
|
||
institution field and a +50,000 priority. Two corrections:
|
||
- the bonus is real (`GOVERNMENT_PRIORITY_BONUS = 50_000`,
|
||
`rule-engine.service.ts:258`) but keys off `bookings.is_government`, set from
|
||
`contracts.is_government` — there is no institution lookup;
|
||
- government does not merely outrank, it **preempts**: it displaces the
|
||
lowest-priority already-reserved commercial booking and rides unpaid
|
||
(`preemptForGovernment`, and see `flows/government_preemption.cy.ts`).
|
||
Created via `POST /bookings` + `POST /bookings/:id/government-expedite`
|
||
against a `kind='government'` company, not through the contract wizard.
|
||
|
||
**S8 priority tiers.** `USD_PAYER` / `RAIL_AND_FORWARDING` no longer exist as
|
||
priority types — migration `1783000000000-ReplacePriorityRulesWithPriorityConfigs`
|
||
replaced `priority_rules` with `priority_configs`, whose `type` is only
|
||
`WAGON | CURRENCY | CUSTOMS`, scored by wagon-count range
|
||
(`priority-config.entity.ts`, applied at `rule-engine.service.ts:247`). The
|
||
scenario's ordering intent survives as: a CURRENCY(USD) config, a CUSTOMS
|
||
config, and a plain booking that matches neither.
|
||
|
||
**S3 "day stays open".** Best asserted through
|
||
`GET /bookings/:id/day-availability?date=` → `{ fits, freeWagons, trainsForDay }`
|
||
(`booking-transition.service.ts:1078`), which is what the portal calendar reads.
|
||
|
||
## Contradicted — the engine does NOT do this
|
||
|
||
These need a product decision before a test can be written honestly.
|
||
|
||
**S26 — mid-corridor intercity is NOT blocked.** The scenario expects
|
||
Dire Dawa → GMP (both Ethiopian) to be rejected as disabled intercity. It is
|
||
the opposite: DOMESTIC is a first-class direction derived from yard countries
|
||
(`bookings.service.ts:270`), and the guard rejects *non*-Ethiopian endpoints —
|
||
`'Intercity bookings only run between Ethiopian yards'`
|
||
(`bookings.service.ts:218`). The only related rejections are
|
||
`'Intercity bookings cannot pin a date or schedule'` (`:771`) and
|
||
`'No route passes through this origin and destination in order'`.
|
||
→ Either the scenario is stale, or intercity was meant to be disabled and is
|
||
not. Ticket, not a test.
|
||
|
||
**S29 — there is no out-of-order checkpoint guard.** `recordCheckpoint`
|
||
(`train-scheduling.service.ts:3681`) validates only: schedule exists, status is
|
||
DISPATCHED, and the station is on the route. Nothing compares `sequenceNo`
|
||
against the highest already logged, so "Arrived Adama" before "Passed Meiso" is
|
||
accepted. `currentSequenceNo` is a `Math.max` (`:3643`) so the timeline does not
|
||
visibly regress — which *masks* the real damage: the position fix at `:3741`
|
||
moves the locomotives, every wagon on the schedule, and the built train to that
|
||
station's yard. A stray backward checkpoint silently relocates rolling stock.
|
||
→ Real bug. Worth a spec that documents current behaviour as `.skip` plus a
|
||
ticket, rather than an assertion that pretends the guard exists.
|
||
|
||
**S36 — `importReleaseGranted` is NOT gated on the second duty.** The scenario
|
||
expects release to stay false until the second duty settles. `importReleaseGranted`
|
||
is computed from the `IMPORT_RELEASE_GRANTED` milestone
|
||
(`booking-clearance.service.ts:368`), which completes purely by uploading a file
|
||
with fieldname `import_release`. `completeByDocTrigger`
|
||
(`clearance-milestone.service.ts:430`) performs **no** precondition check, and
|
||
`assertPriorCompleteOnMilestones` only walks *pre-booking* milestones
|
||
(`clearance-workflow.service.ts:119`) — so all 17 post-`DO_COLLECTED` codes,
|
||
including the whole `T1_CLOSED → RISK_ASSIGNED → SECOND_DUTY_* →
|
||
IMPORT_RELEASE_GRANTED` tail, are unordered.
|
||
→ Release can be granted with `SECOND_DUTY_PAID` still PENDING. Real gap.
|
||
|
||
**S40 — hazardous quantity is NOT rejected, it is silently clamped.** The
|
||
scenario expects `hazardousQuantity=12` on a `quantity=10` line to be rejected.
|
||
The DTO has `@Min(0)` and no `@Max` (`create-booking.dto.ts:60`), and the
|
||
repository clamps to `0..quantity` (`bookings.repository.ts:217`): the booking
|
||
is created 201 with the value truncated, no warning. Reefer behaves identically.
|
||
Note `returnQuantity` — same layer, same shape of data — *does* throw
|
||
(`contract-booking.service.ts:1740`), so the pattern exists and these two just
|
||
do not use it.
|
||
→ Real gap. A test asserting rejection would fail today.
|
||
|
||
**S40 — reefer is NOT derived from container type.** The scenario expects
|
||
`reeferQuantity` forced to 0 for DRY types. No such logic exists; a DRY type
|
||
with `reeferQuantity > 0` is an explicitly supported state and applies the
|
||
surcharge anyway (`booking.entity.ts:384`, `booking-pricing.service.ts:402`).
|
||
|
||
## Flag-dependent — assert the flag, or the test is vacuous
|
||
|
||
**S13/S14/S18 "export never splits"** holds only while
|
||
`FREIGHT_EXPORT_SPLIT !== "true"` (`booking-batch.service.ts:394`). With the
|
||
flag on, `isSplitEligible` admits EXPORT (`:2558`) and `tryExportPartialOffer`
|
||
(`:1240`) runs. The export specs must assert the flag is off, or they silently
|
||
stop testing whole-or-nothing the day someone flips it.
|
||
|
||
## Known type hole (not scenario-blocking)
|
||
|
||
Last-mile and first-mile billing write the literal `'last_mile'` / `'first_mile'`
|
||
cast past the type checker (`last-mile-invoice.service.ts:41`), while
|
||
`Freight.InvoiceSource.LastMile` is `"lastmile"`. `invoices.source` is a plain
|
||
varchar with no constraint, so both spellings persist. Writes and reads agree
|
||
within each module so billing works — but S39's "invoice source LASTMILE"
|
||
assertion must match `'last_mile'`, not the enum value.
|
||
|
||
## Two engine changes the committed specs predate
|
||
|
||
Both were found by running the suite, and both broke EVERY scenario until
|
||
fixed. They are recorded here because neither is visible from the scenario
|
||
document — only from the API source.
|
||
|
||
**1. Every contract booking is born in the clearance gate.**
|
||
`contract-booking.service.ts:211` — *"EVERY contract booking clears per booking
|
||
now — both contract kinds, both paths, intercity included."* A booking is
|
||
created in `AWAITING_DOCUMENTS` regardless of whether customs clearance is
|
||
enabled, so `bookContainers` followed by `acceptOperation` always 409s with
|
||
`Cannot perform this action on status "AWAITING_DOCUMENTS". Allowed:
|
||
OPERATION_REQUEST_PENDING`.
|
||
|
||
The gate is upload → GL approve → finalize → customer proceeds with the day.
|
||
`clearToOperationRequestPending` (import-utils) runs it; `bookAndClear`
|
||
(g1-utils) wraps book + clear + accept and is what the g-specs use.
|
||
|
||
NOTE: the pre-existing corridor specs (e.g. `import_full_train.cy.ts`) still
|
||
call `acceptOperation` directly and fail for this reason — 3 passing / 10
|
||
failing when last run. They predate the gate and need the same treatment.
|
||
|
||
**2. An unpaid hold cannot expire without a reachable payment gateway.**
|
||
Before expiring a reservation the engine asks the gateway whether a late
|
||
payment landed (`booking-batch.service.ts:3484-3506`), and treats ANY error as
|
||
`unverifiable: true` — deferring the expiry rather than risk expiring a
|
||
customer who paid:
|
||
|
||
[BATCH] expire deferred for BK-… — settlement unverifiable at the
|
||
gateway; retrying next settle tick
|
||
|
||
`PAYMENT_API_URL` defaults to the real `https://paymentcallback.triaplc.com`
|
||
(`payment-client.service.ts:25`), unreachable from e2e, so every expiry
|
||
deferred forever. Six scenarios turn on an expiry: G1·S1, G1·S5, G1·S6,
|
||
G3·S16, G5·S22, G5·S24.
|
||
|
||
Fixed with a stand-in service — `e2e/freight/payment-mock/server.js`, wired as
|
||
`payment-mock-e2e` in `docker-compose.e2e.yaml` with
|
||
`PAYMENT_API_URL: http://payment-mock-e2e:4500`. It answers
|
||
`POST /payments/reconcile` with `{paid:false, unverifiable:false}` so the
|
||
engine gets a definite "no payment exists" and expires the hold as designed.
|
||
A stack that does NOT point PAYMENT_API_URL at a reachable service will hang
|
||
on every expiry assertion.
|