docs(12): add code review fix report

.planning/phases/12-orders-invoices-company-matching/12-REVIEW-FIX.md

@@ -0,0 +1,63 @@
+---
+phase: 12-orders-invoices-company-matching
+fixed_at: 2026-07-11T11:30:00Z
+review_path: .planning/phases/12-orders-invoices-company-matching/12-REVIEW.md
+iteration: 1
+findings_in_scope: 5
+fixed: 5
+skipped: 0
+status: all_fixed
+---
+
+# Phase 12: Code Review Fix Report
+
+**Fixed at:** 2026-07-11T11:30:00Z
+**Source review:** .planning/phases/12-orders-invoices-company-matching/12-REVIEW.md
+**Iteration:** 1
+
+**Summary:**
+- Findings in scope: 5 (2 Critical, 3 Warning; Info findings excluded per fix_scope)
+- Fixed: 5
+- Skipped: 0
+
+## Fixed Issues
+
+### CR-01: `partner_cost` / `partner_cost_total` columns silently truncate real PAX8 cost precision
+
+**Files modified:** `migrations/095_pax8_order_items_partner_cost_numeric.sql` (new)
+**Commit:** `591fc5c`
+**Applied fix:** Added new migration 095 (093 is a committed migration and was not edited) widening `pax8_order_items.partner_cost` and `partner_cost_total` from `NUMERIC(12,2)` to `NUMERIC(14,4)`, matching the precision already used for `quantity` (migration 094). Applied directly to the running dev Postgres container (`pulse-postgres`, per project convention for existing volumes) via `docker exec -i pulse-postgres psql -U pulse_user -d pulse_autotask < migrations/095_....sql`, and verified via `\d pax8_order_items` that both columns now report `numeric(14,4)`.
+
+### CR-02: `company_matches` "review" counts fabricate deletion counts in the Sync History UI
+
+**Files modified:** `lib/services/pax8-sync-service.ts`, `lib/services/pax8-sync-service.test.ts`, `lib/types/pax8.ts`
+**Commit:** `bddf612`
+**Applied fix:** Added a dedicated `flaggedForReview?: number` field to `Pax8EntitySyncResult`. `syncCompanyMatches()` now reports `tombstoned: 0` (no soft-deletes are ever performed by matching) and surfaces the ambiguous/no-candidate review count separately via `flaggedForReview`, plus a `[Pax8Sync] Company match: N flagged for manual review (...)` log line. `fullSync()`'s summary log line now also reports the total flagged-for-review count. Updated the existing unit test (`syncCompanyMatches delegates to matchPax8Companies...`) which previously asserted the buggy `tombstoned === 1` behavior — it now asserts `tombstoned === 0` and `flaggedForReview === 1`. All 5 tests in `pax8-sync-service.test.ts` pass.
+
+### WR-01: A single failing record aborts the entire remaining batch for that sync run
+
+**Files modified:** `lib/services/pax8-sync-service.ts`
+**Commit:** `cb8ae85`
+**Applied fix:** Wrapped the per-row upsert body in its own try/catch in all four entity syncs (`syncCompanies`, `syncSubscriptions`, `syncProducts`, `syncOrders`), including a separate per-item try/catch nested inside the per-invoice try/catch in `syncOrders` (invoice header failure and item-fetch/item-upsert failure are isolated independently). Each failed row is logged (`console.error`, non-fatal) and the loop continues; the id is still recorded as "seen" so a transient write failure doesn't get misread as "gone from PAX8" by the tombstone step. Tombstoning now always runs over whatever was actually seen this run, regardless of mid-loop row failures. Each entity result now reports `success: false` with a summary `error` message (count of failures + first error) when any row failed, while still returning the partial `upserted`/`tombstoned` counts from the rows that did succeed — previously a single throw discarded all counts for the entire method and skipped tombstoning entirely. All 5 tests in `pax8-sync-service.test.ts` pass.
+
+### WR-02: `matchPax8Companies()`'s fixed `limit=1000` + `ORDER BY name` can permanently starve later companies
+
+**Files modified:** `lib/services/pax8-company-matcher.ts`
+**Commit:** `76a652d`
+**Applied fix:** Applied both halves of the review's suggested fix: (1) raised the default `limit` from 1000 to 10000 (generous headroom above the validated 118-company scale); (2) replaced the static `ORDER BY name` cursor with `ORDER BY COALESCE(r.detected_at, c.matched_at, 'epoch'::timestamptz) ASC`, joining the open (`resolved_at IS NULL`) `pax8_company_match_review` row so companies are re-scored in order of "least recently considered" rather than alphabetically — a permanently-open review row now sinks behind any row that hasn't been reconsidered as recently, instead of camping in the same alphabetical slot forever. All 6 tests in `pax8-company-matcher.test.ts` pass.
+
+### WR-03: `Pax8Client.getToken()` does not validate the token response shape
+
+**Files modified:** `lib/services/pax8-client.ts`
+**Commit:** `1bb2b8b` (plus follow-up `be6f07b` fixing a TS narrowing error the initial edit introduced — `npx tsc --noEmit` flagged `this.accessToken` as `string | null` on the return statement; fixed by capturing the validated token into a local `const token: string` and returning that instead of re-reading the mutable instance field)
+**Applied fix:** Applied the review's suggested fix essentially verbatim: after parsing the token response body, throw `PAX8 token response missing access_token/expires_in` if `access_token` is falsy or `expires_in` isn't a number, before ever assigning to `this.accessToken`/`this.tokenExpiry`. This closes the silent-`undefined`-token / `NaN`-expiry failure mode described in the finding. All 12 tests in `pax8-client.test.ts` pass.
+
+## Skipped Issues
+
+None — all 5 in-scope findings were fixed.
+
+---
+
+_Fixed: 2026-07-11T11:30:00Z_
+_Fixer: Claude (gsd-code-fixer)_
+_Iteration: 1_