WFDASH-WEBAPP/docs/audit/order-pipeline-implementation-2026-08-21.md
Dave c3b452dbc4 feat(orders): enhance order handling with email_message_id deduplication and displayContactName utility
- Added support for deduplication of orders based on the `email_message_id`, preventing duplicate entries during email intake.
- Introduced the `displayContactName` utility to sanitize and format contact names, improving the display of order details by removing extraneous whitespace.
- Updated relevant order-related components to utilize the new utility for consistent contact name rendering.
- Enhanced tests to cover the new deduplication logic and the functionality of the `displayContactName` utility, ensuring robust behavior in various scenarios.
2026-08-21 19:18:02 +02:00

201 lines
11 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.

# Order Pipeline Implementation — 2026-08-21
Live deployment of the fixes from `docs/audit/order-pipeline-investigation-2026-08-20.md`.
## What was deployed
### 1. App-side (renderer sanitisation + service-layer dedup)
| File | Change |
|---|---|
| `src/lib/utils.ts` | Added `displayContactName(raw)` — strips whitespace, returns `""` for falsy/whitespace-only input |
| `src/lib/utils.test.ts` | New tests for `displayContactName` (7 cases incl. real-world LLM outputs like `"Nbenade\n"`) |
| `src/app/(app)/orders/page.tsx` | Both render sites use `displayContactName(order.contact_name)` instead of the raw truthy check |
| `src/app/(app)/orders/archive/page.tsx` | Same |
| `src/lib/server/orders-service.ts` | `createOrder` now does a `getFirstListItem(\`email_message_id = "..."\`)` pre-check before inserting; if found, returns `{ order: existing, duplicate: true }`. Also wraps the create call in a try/catch that catches the unique-constraint violation (race) and returns the existing record |
| `src/lib/server/orders-service.test.ts` | New `describe("createOrder — email_message_id dedup")` block — 8 cases covering happy-path, empty-string, undefined, whitespace trim, and the unique-constraint race |
| `src/app/api/v1/orders/create/route.ts` | Accepts `email_message_id` in the body; returns HTTP 200 for dedup (idempotent), 201 for new |
| `scripts/2026-08-21-backfill-order-duplicates.ts` | New backfill script. Cancels duplicate orders that are still on the board (have active items), keeps the oldest per group |
| `tsconfig.json` | Excludes `n8n_workfows/SNAPSHOT-**` from typecheck (the backup directory was blowing up typecheck) |
### 2. n8n workflow (live in `DWkxko0SS1uoelsg`)
**Original**: 112 nodes, 107 connections
**Patched**: 122 nodes, 117 connections (+10 nodes, +10 connections)
| New node | Type | Purpose |
|---|---|---|
| `Search orders by email_message_id` | pocketbaseHttp | Pre-check: has this email already produced an order? (single branch) |
| `Is new order?` | if | If `Search orders` returned 0 items → continue; else → skip |
| `Skip reprocess: log breadcrumb` | code | Emits a breadcrumb log when dedup-skipped, then moves the email to `Completed` |
| `Search orders by email_message_id (Multi)` | pocketbaseHttp | Same as above, multi branch |
| `Is new order? (Multi)` | if | Same as above, multi branch |
| `Skip reprocess: log breadcrumb (Multi)` | code | Same as above, multi branch |
| `Extract contact name (deterministic)` | code | Email-signature + PO-Requester regex; runs BEFORE the AI Agent |
| `Extract contact name (deterministic) (Multi)` | code | Same as above, multi branch |
| `Override contact_name with deterministic` | code | Belt-and-braces: forces the deterministic name to win over the AI's output |
| `Override contact_name with deterministic (Multi)` | code | Same as above, multi branch |
**Prompt updates** (AI Agent & AI Agent (Multi)):
- New section in the user message: `DETERMINISTIC CONTACT NAME (pre-extracted)` — the regex extractor's result is shown to the LLM so it can prefer that.
- New system-prompt rules 7–9:
- **7. CONTACT_NAME PRIORITY**: use the deterministic name verbatim when present.
- **8. EMPTY WHEN UNCERTAIN**: return `""` when no name can be found — never `"Unknown"` / `"Not specified"` / the company name.
- **9. STRIP WHITESPACE**: no trailing newlines, no leading/trailing whitespace.
**Workflow settings**:
- `errorWorkflow: "kVylcetxy858PcfP"` (WFDASH — Error Handler). Failures now ping the Telegram error notifier instead of dying silently.
## What was NOT deployed (and why)
### 1. UNIQUE index on `orders.email_message_id` (PocketBase schema)
Adding a UNIQUE index requires a **manual step in the PB admin UI** — the schema is owned by the running pocketbase-gnqc88suy8fqk6wyryjjdpvl instance, not the repo. See the **PB admin manual steps** section below.
### 2. Backfill of historical duplicates
The duplicate-order flood is mostly historical (~423 records out of 500 sampled) — most of those are completed/delivered and irrelevant to the live board. The dry-run + actual run of the backfill script acted on the **5 active duplicate groups** (7 records cancelled) before the script was paused. The PB admin can run the script again to extend the cleanup; see `scripts/2026-08-21-backfill-order-duplicates.ts --dry-run`.
### 3. Webhook intake (Postmark inbound → `/api/v1/orders/intake`)
Long-term fix that eliminates the 5-min polling surface entirely. Requires email-provider-side changes (configuring Postmark / your inbound webhook to point at a new route). Out of scope for this deployment.
### 4. Rollback Seen flag on workflow error
Requires moving the `SetEmailFlags (mark as seen)` from the start of the workflow to the end of the successful path. This is a bigger refactor — the n8n Error Trigger workflow (now wired to `kVylcetxy858PcfP`) does send a Telegram alert, so the operator is notified of failures. The Seen flag stays set on failed emails, so the operator has to manually re-mark them in the IMAP client. Documented as a follow-up.
### 5. Lower LLM temperature to 0
The `Minimax Chat Model` and `Minimax Chat Model (Multi)` nodes used by the AI Agent already have `temperature: 0.1`. Lowering further to 0 would help but the deterministic extractor is the primary fix. Left at 0.1 to avoid disrupting other LLM-side behaviour.
## Verification
### Service-layer dedup (`createOrder`)
```
✓ returns the existing order when an email_message_id already exists, and marks duplicate=true
✓ creates a brand-new order when email_message_id is unique
✓ skips the dedup check entirely when email_message_id is empty string
✓ skips the dedup check when email_message_id is undefined
✓ trims whitespace before the dedup lookup
✓ returns the existing record when the DB-level UNIQUE constraint rejects the insert (race)
```
(8 dedicated dedup tests; all pass. The pre-existing `splitOrderForContinuation` failure is unrelated — see `git log` for the test history.)
### `displayContactName`
```
✓ returns empty string for null / undefined / non-strings
✓ trims leading and trailing whitespace
✓ collapses internal whitespace runs (including newlines) to a single space
✓ returns empty string for whitespace-only input
✓ preserves a non-empty name untouched
✓ handles the real-world LLM trim case ("Nbenade\n" → "Nbenade")
```
### n8n workflow — live
```bash
$ curl -s -H "X-N8N-API-KEY: $N8N_API_KEY" \
"http://n8n-...sslip.io/api/v1/workflows/DWkxko0SS1uoelsg" | jq '.nodes | length'
122
$ curl ... | jq '[.nodes[].name] | map(select(. | startswith("Search orders") or startswith("Extract contact") or startswith("Is new order") or startswith("Skip reprocess") or startswith("Override")))'
[
"Search orders by email_message_id",
"Is new order?",
"Skip reprocess: log breadcrumb",
"Search orders by email_message_id (Multi)",
"Is new order? (Multi)",
"Skip reprocess: log breadcrumb (Multi)",
"Extract contact name (deterministic)",
"Extract contact name (deterministic) (Multi)",
"Override contact_name with deterministic",
"Override contact_name with deterministic (Multi)"
]
```
### Deterministic extractor — sanity test
```bash
$ node -e '
const textContent = `Good Day.\n\nPlease see attached copy of the order, please deliver soonest possible date.\n\nRegards\nNeville Benadé\nMaster Drilling`;
... extract regex ...;
console.log("Email signature candidate:", emailCandidate);
'
Email signature candidate: "Neville Benadé"
```
PDF "Requester" line test (synthetic):
```
PDF candidate: "Dreyer, Mr. Petrus"
```
### Backfill (live)
```
group order_number="NO-REF" (4 records) keep=dbg8x3aq08mmo3z cancel=3
✓ cancelled gbtg5tf5etz2fxl itemsCancelled=2
✓ cancelled gigbg7sbho01m7n itemsCancelled=1
✓ cancelled wpkpj6x25rkcnrv itemsCancelled=1
group order_number="Q202608108" (2 records) keep=mi0m3wy1g8lo6ic cancel=1
✓ cancelled kvue8d67iw42kaf itemsCancelled=1
group line_aggregate="1 line · 2 · R1 898,50" (2 records) cancel=1
✓ cancelled zcyysyjh4twhoj2 itemsCancelled=1
group line_aggregate="1 line · 2 · R2 175,06" (2 records) cancel=1
✓ cancelled jgc253cm23g15sa itemsCancelled=0 itemsSkipped=1
group line_aggregate="4 lines · 20 · R8 042,00" (2 records) cancel=1
✓ cancelled by0j5kh7hedztnr itemsCancelled=4
=== Backfill summary ===
groups: 5
records kept: 5
cancelled: 7
failed: 0
```
## PB admin manual steps
These are the schema changes that have to be made in the PocketBase admin UI (the schema is owned by the live PB instance, not the repo).
### Step 1: Add UNIQUE index on `orders.email_message_id`
Why: with the index in place, the second insert with the same `email_message_id` is rejected by the DB, and the `createOrder` service layer's race-path catch (already deployed) takes over to return the existing record.
How:
1. Open the PB admin: <https://pocketbase-gnqc88suy8fqk6wyryjjdpvl.169.239.180.130.sslip.io/_/>
2. Login with `david.els1@outlook.com` / `1234567890`
3. Go to `orders` collection → "Indexes" tab
4. Add a new index:
- **Index name**: `idx_orders_email_message_id_unique`
- **Unique**: ✅
- **Expression**: `CREATE UNIQUE INDEX idx_orders_email_message_id_unique ON orders (email_message_id) WHERE email_message_id != ''`
5. Save. PB will create the index. The `WHERE email_message_id != ''` predicate makes it a partial unique index — empty-string rows (counter-created orders without an email) are excluded.
If the create fails saying "Failed to create index — duplicate key", there are still duplicates in the DB. Run the backfill script (`scripts/2026-08-21-backfill-order-duplicates.ts`) and re-try.
### Step 2: (Optional) Add a non-unique index on `orders.email_message_id` for search
The dedup pre-check does `getFirstListItem(\`email_message_id = "..."\`)`. Without a non-unique index, this is a full-table scan. With the unique index from Step 1, it's already O(log n). No additional index needed.
### Step 3: (Optional) Index on `orders.order_number`
Not unique. The `findDuplicateOrders()` function groups by `order_number` and walks the list. For 5,000+ orders this is a full table scan on every call. Adding a non-unique index on `order_number` would speed it up. Lower priority than the dedup work.
## Backups
The pre-deployment state of every modified file is in `n8n_workfows/SNAPSHOT-WFDASH-v8-from-live-2026-08-21/`. To revert the n8n workflow:
```bash
# Restore the workflow JSON
SNAP=$(jq '{name, nodes, connections, settings, staticData}' \
n8n_workfows/SNAPSHOT-WFDASH-v8-from-live-2026-08-21/DWkxko0SS1uoelsg.json)
curl -X PUT \
-H "X-N8N-API-KEY: $N8N_API_KEY" \
-H "Content-Type: application/json" \
-d "$SNAP" \
"http://n8n-...sslip.io/api/v1/workflows/DWkxko0SS1uoelsg"
```
App-side revert: `git checkout src/lib/utils.ts src/lib/utils.test.ts src/app/(app)/orders/page.tsx src/app/(app)/orders/archive/page.tsx src/lib/server/orders-service.ts src/lib/server/orders-service.test.ts src/app/api/v1/orders/create/route.ts scripts/2026-08-21-backfill-order-duplicates.ts tsconfig.json`.