Skip to content

Design: Review V2 line-item override (quantity & unit price)

Status: APPROVED (/autoplan, 2026-06-14) — premise “build as specified”; 10 must-fixes folded in; 3 taste decisions resolved (all recommendations taken). See review report at end. Owner: David Last updated: 2026-06-14 Surface: apps/webapp order review (Review V2 cockpit) Builds on: Review V2 (review-v2-multi-tab-excel.md), ENG-277 optimistic-lock + price provenance


Legacy review let a CSR edit any line’s quantity and unit price as free-form inputs. Review V2 renders both as read-only display — there is no override path. That removed a legitimate, if narrow, capability:

  • OCR/extraction misreads a quantity (e.g. 320 vs 300) and the corrected value isn’t a matcher decision.
  • A negotiated one-off price that is neither the ERP tier price nor a PO price.
  • A unit-of-measure / pack mismatch the matcher and tier resolver cannot model.

Today the only way to “fix” these is a support-side DB edit or routing the order back as an error — both worse than a controlled, audited in-UI override.

The goal: restore override durably, without adding complexity to the three core flows (auto-validate, discounts, submit).

Why V2 dropped it (context, not a mistake)

Section titled “Why V2 dropped it (context, not a mistake)”

V2 reframed review from editing a form to resolving matches. Free-form qty/price editing was dropped for three concrete reasons:

  1. Free price edits raced the async pricing writeback. persistAutoPricedMatches writes resolved ERP-tier prices back to unit_price under a row_version CAS guard specifically so it never clobbers an operator — see persist-auto-priced-matches.ts:27 and the CAS at :583. Legacy’s bundled whole-order PUT didn’t participate cleanly in that protocol → lost-update risk. ENG-277 added row_version and the immutable extracted_unit_price to close it.
  2. Price became a structured decision. When PO price ≠ ERP tier, auto-validate emits a validationError carrying details.erpTierPrice; the operator accepts the tier price rather than typing one. That is already a constrained override — see accept-gross-price.ts:137.
  3. Allowances moved to their own channel. Discounts/allowances are first-class custom-line items resolved via DiscountMatchPanel, removing a major reason CSRs used to fudge price.

The casualty: the general override branch was dropped, leaving exactly one allowed move (accept ERP tier price).

accept-gross-price is a price override — it sets:

.set({ unitPrice: grossPrice, validationError: null, isValidated: true, isAutoValidated: false, isEdited: true })

…hardcoded to value = erpTierPrice and gated to items already in price-review. Generalizing that single endpoint gives durable override almost for free, riding rails that already exist:

RailAlready presentSource
Immutable original priceextracted_unit_price (scale 4)schema:495
Optimistic lockrow_version + CAS guard in writebackschema:509
Provenance enumvalidation_source = worker | user | auto_validateschema:487
Submit reads resolved valuespayload built from DB quantity/unitPricesubmit.ts (~252–266, 643)
AuditSupportAccessAudit.logAccessalready called in accept-gross-price

⚠️ Review correction (C1/H1/M1): three of these “rails” are unfinished substrate, not finished rails:

  • Submit does NOT carry the override. submit.ts:641-644 builds ErpOrderInput.items as {erpItemId, quantity, price, unitOfMeasure} only. wherefour-service.ts:525-532 drops the per-line price when preferTierOverOverride && tierAvailable && hasPrice && !explicitOverride (explicit = item.overrideUnitPrice === true). For CK (the first rollout target — see the flag’s own comment naming “CK and similar”), an overridden price is silently re-priced to tier. Must wire overrideUnitPrice end-to-end (C1).
  • validation_source is write-only. It is written in one place and read by no auto-validate path, so “re-validate skips user lines” does not exist yet (H1).
  • price_decisions is the real audit table. price-decisions.ts already has decision_stage='manual_override', source='manual_override', actor='operator', reason (“Required for manual_override”), chosenPrice, priorDecisionId — purpose-built for exactly this (M1, taste decision T1).

Add one terminal line-resolution action — “Override” — alongside Confirm / Skip / Mark error / Remove, backed by a single new endpoint.

  1. extracted_quantity (new column). There is an immutable extracted_unit_price but no quantity equivalent, so a quantity override would lose the original. Add extracted_quantity integer (nullable), written once at extraction time and never overwritten — symmetric with extracted_unit_price.
    • Migration shape (M3 — match the extracted_unit_price precedent, do NOT inline-backfill). ENG-277 added extracted_unit_price as an instant DDL-only ADD COLUMN IF NOT EXISTS (migration 0062) with a separate idempotent backfill script (backfill-extracted-unit-price.ts + test). Mirror that: migration = ALTER TABLE extracted_order_items ADD COLUMN IF NOT EXISTS extracted_quantity integer; (nullable, instant), numbered 0082 (rebased onto staging; latest base is 0081). A whole-table UPDATE … SET extracted_quantity = quantity inside the DDL is not instant and risks wedging the migration ledger (active drift per PR #1668/0079). Treat NULL extracted_quantity as ”= current quantity” in the UI for legacy rows.

No other schema changes: override reuses unit_price, quantity, validation_source, row_version, is_edited, validation_error.

API: POST /api/extracted-orders/[id]/items/[itemId]/override

Section titled “API: POST /api/extracted-orders/[id]/items/[itemId]/override”

Accepts { quantity?: number, unitPrice?: string, reason?: string, expectedRowVersion: number }.

Behavior (mirrors accept-gross-price, strictly safer):

  • Per-customer authz (revised — see update note below). Override is opt-in per customer via the connection-scoped review_v2_line_override flag (connection-flag.ts), default off. When enabled, a customer’s own CSRs (org users) may override only on their own org’s orders; Ordermatic support-admins keep cross-org break-glass access (audited). The flag is enforced server-side, not just in the UI.
  • Input bounds (H2/F4 — server zod + client mirror). quantity: positive integer (column is integer; reject 0, negative, non-integer, blank). unitPrice: numeric, > 0, ≤ scale-4 (matches extracted_unit_price); reject negative/blank. Deviation guard: when an ERP tier price is known and |unitPrice − erpTierPrice| / erpTierPrice exceeds a band (e.g. 25%, configurable), require an explicit confirm and a non-empty reason. Soft-cap absurd values (qty > 100k, price > $100k) with confirm, not hard block. No-op overrides (qty & price unchanged) rejected.
  • CAS on row_version (WHERE row_version = expectedRowVersion, then row_version + 1).
  • Sets the supplied quantity/unitPrice, validation_source = 'user', is_edited = true, is_validated = true, clears validation_error.
  • Writes extracted_quantity/extracted_unit_price only if still null (preserve original).
  • Carry the override to the ERP (C1 — the must-fix). Persist an operator-override signal (reuse validation_source = 'user' or add override_unit_price boolean) and, in submit.ts:641-644, set overrideUnitPrice: true on the ErpOrderItemInput for overridden lines (the bootstrap OrderItem[] map at :252-266 must carry it too, or submit re-reads extracted_order_items). Without this, CK books at tier and the override is a silent no-op.
  • Audit (M1 / taste T1). Doc default is SupportAccessAudit.logAccess (action: 'api_override_line_item'). Recommended: also write a price_decisions row (decision_stage='manual_override', source='manual_override', actor='operator', reason, chosenPrice, priorDecisionId) — no migration needed, the enums exist, and it’s the table downstream reconciliation reads.
  • 409 on row_version mismatch → client refetches and re-presents (see UI F5).
  • Sibling CAS fix (M2 — now required, not optional). accept-gross-price.ts AND validate-at-tier-price.ts update price/isValidated without bumping row_version, violating the invariant the writeback CAS depends on (schema:506-507). Shipping a CAS-correct override next to two CAS-blind siblings undermines the safety argument. Patch both in the same PR.
  • Placement (taste T2). Doc default: a new ghost button + shortcut in the ItemDetailPane action bar at ReviewCenterPane.tsx (~977–1050). Design recommendation: Override is an edit that keeps you on the line, not a terminal action that advances focus like Skip/Error/Remove/Confirm — and a 5th button forces the bar to wrap on laptops. Prefer an inline “Edit” pencil affordance on the Quantity / Unit price rows in the “FROM DOCUMENT” block (where the edited values live); action bar stays 4 actions.
  • Appearance conditions (F2/F3). Suppress on discount/custom lines (isCustomLineItem(item) — they own their own panel). On a price-review line, “Accept tier price” stays primary; Override is the subordinate “neither value is right” escape hatch.
  • Opens a small inline editor (qty and/or price + reason), not a full form.
  • States (F4 — CRITICAL, mirror DiscountMatchPanel): saving (disable Save, block re-submit), non-409 failure (revert optimistic update, inline error, keep editor open with typed values), success. 409 (F5): surface what changed (“price auto-updated to $X while editing”), preserve typed input, let CSR re-apply deliberately — do not silently swap values under an open editor.
  • Keyboard (F10): trigger shortcut O or E (S/X/Enter/Backspace/J/K/⌘K taken) added to the Kbd legend; the editor owns Enter=Save / Escape=cancel via onKeyDown + stopImmediatePropagation (RemoveLineConfirm precedent at ReviewCenterPane.tsx:271-277) so a background auto-validate that re-arms the parent listener can’t hijack keys. The global handler already early-returns on isInput, so typing a qty won’t fire Skip. Focus the qty field on open; return focus to the line on close.
  • Provenance + revert (F12 — required). Badge on the value rows it modifies: “Overridden: 320 → 300 (name · reason)” sourced from extracted_* vs current (show the arrow only when extracted_* !== current; handle null extracted_quantity; attribute the real actor, not “you”). Every other resolution here is reversible (Remove restores; discount has “Clear pin”) — the badge needs “Revert to original” restoring from extracted_quantity/extracted_unit_price and resetting validation_source. Cheap (originals are preserved) and closes the fat-finger loop.
  • Numeric inputs use inputMode="decimal"/"numeric"; editor fields stack under narrow @container (desktop-CSR-first, so “don’t break” on mobile).
  • Gated behind the connection-scoped review_v2_line_override flag (per customer, default off). Enable per ERP connection (a customer may have several, e.g. prod vs sandbox). The UI reads the same connection flag the server enforces.

Override is a terminal resolution state feeding the same columns the core flows already consume:

  • Auto-validate — unchanged (H1 resolved by retraction). Override is an alternative resolution, not a matcher step. The original “re-validate skips validation_source='user' lines” mechanism was never wired (the column was write-only) and is unnecessary: an override sets is_validated = true, and the auto-validate candidate selection already includes only un-validated lines, so an overridden line is never re-derived. The validation_source='user' stamp is kept for provenance/audit, not for skip logic.
  • Discounts — unchanged. They remain their own line type via DiscountMatchPanel; override targets product lines and quantity corrections only.
  • SubmitNOT unchanged (C1). It must set overrideUnitPrice (see API), or CK books at tier. Also a documentation correction (M4): the submit gate blocks on validationError.type === 'needs_review' (isNeedsReviewValidationError), not on isValidated — an override unblocks the line by clearing validation_error. Note there are two gate implementations (bootstrap submit.ts:236-249 and reload :343-364); an override must clear validation_error for both (it does). Add a test for the bootstrap path (v2 commonly hits it).
  1. Migration 0082: instant DDL ADD COLUMN for extracted_quantity + override_unit_price (no inline backfill) + separate idempotent backfill script (M3).
  2. Wire overrideUnitPrice through submit→WhereFour + the submit-integration test against a preferTierOverOverride connection (C1). Nothing ships before this is green — it is the gate on delivering any value to CK.
  3. Bump row_version in accept-gross-price AND validate-at-tier-price (M2 — required).
  4. H1 resolved by retraction: overridden lines carry is_validated = true, already excluded by the auto-validate candidate selection — no auto-validate predicate change needed.
  5. Ship endpoint + UI behind review_v2_line_override (off), support-admin-gated.
  6. Enable for platform support-admins, then CK POC org, then GA.
  • API unit: override sets values + validation_source='user' + bumps row_version; 409 on stale expectedRowVersion; original extracted_* preserved on second override; auth/org-scope + support-admin audit row written.
  • Race: override interleaved with persistAutoPricedMatches — CAS guarantees exactly one winner, no silent overwrite.
  • Submit integration (CRITICAL GAP today): assert the outbound payload’s override_unit_price survives for an operator-overridden line on a preferTierOverOverride=true connection — the chain extracted_order_items → submit.ts ErpOrderInput → erp-submission remap → WhereFour adapter. Note submit.test.ts currently mocks Temporal entirely, so the payload is never asserted — that mock is exactly why C1 slipped; this test must assert the real body. Also: overridden line passes BOTH gates (bootstrap + reload); price-review blocker still fires for other unresolved lines.
  • Input bounds (H2): reject non-integer/zero/negative qty, negative/zero/blank price; deviation guard fires + requires reason past the band; no-op override rejected.
  • Audit: override writes the chosen audit row(s) (SupportAccessAudit and/or price_decisions per T1); reviewer (non-admin) path audited if scope is widened.
  • Component: editor states (saving / failure-revert / 409 surface), keyboard (Enter=Save trapped, Backspace doesn’t Remove), provenance badge from extracted_* incl. null, Revert to original.
  • Regression: auto-validate and discount-resolution suites unchanged and green.
  • Permission scope: RESOLVED — per-customer (connection flag). Customer CSRs override on their own org’s orders when enabled; support-admins keep cross-org break-glass. Support overrides log to SupportAccessAudit; customer price overrides are audited via price_decisions (actor=operator). UI affordance follows the customer flag, so support break-glass when a customer has it OFF is API-level only.
  • Re-validate semantics: the skip must be implemented (H1), not assumed. Lean: skip validation_source='user' lines, with an explicit “re-match anyway” affordance.
  • Reason required? See taste decision T3.

Run: /autoplan, 2026-06-14 · branch claude/serene-bell-6640b4 · base main (PRs target staging) Scope: CEO ✓ · Design ✓ (UI scope) · Eng ✓ · DX skipped (no developer-facing surface; internal Astro routes + CSR UI only) Voices: [subagent-only] — Codex was entitlement-blocked on this host (ChatGPT-account login rejects gpt-5 / gpt-5.4 / gpt-5-codex). Second voice = direct code verification of every load-bearing claim. Fix codex model access to restore dual-voice. Premise gate: confirmed “build as specified” (unified qty+price editor, generic audit acceptable), F1 non-negotiable.

DimensionVerdictNote
Right problem?CONFIRM (owner)Premise unevidenced by data, but owner has CK ground truth and confirmed build.
Does the override reach the ERP?FAIL → must-fixC1: dropped to tier for CK. Verified in submit.ts + wherefour-service.ts.
”Rails already exist”?PARTIALvalidation_source write-only (H1); submit override path absent (C1); price_decisions ignored (M1).
Pricing integrity / trustFAIL → must-fixH2: no qty/price bounds; fat-finger reaches customer ERP.
Concurrency safetyPARTIALM2: two sibling endpoints don’t bump row_version.
Migration safetyOK w/ fixM3: DDL-only + separate backfill, number 0082.
UX completenessPARTIALF4 (states), F7 (inline validation), F12 (revert) missing.

Must-fixes (auto-decided — folded into the doc above)

Section titled “Must-fixes (auto-decided — folded into the doc above)”
#SevFixPrinciple
C1CRITICALWire overrideUnitPrice through submit→WhereFour + integration test on a preferTierOverOverride connectionP1 completeness
H1HIGHImplement (or retract) the validation_source='user' re-validate skipP5 explicit
H2/F4-engHIGHServer+client input bounds (qty>0 int, price>0 scale-4) + deviation guardP1 completeness
M2HIGHBump row_version in accept-gross-price + validate-at-tier-price (required)P2 boil-the-lake
F4-uxCRITICALEditor saving/failure/409 states (mirror DiscountMatchPanel)P1 completeness
F7-uxCRITICALInline input validation before SaveP1 completeness
M3MEDMigration 0082: instant DDL + separate idempotent backfillP5 explicit
M4MEDCorrect submit-gate mechanism text; test bootstrap gateP5 explicit
L3MEDExplicit hasPlatformSupportAdminRole gate on the new endpointP5 explicit
F12-uxHIGH”Revert to original” from extracted_*P1 completeness

Taste decisions (RESOLVED — approved as-is, all recommendations taken)

Section titled “Taste decisions (RESOLVED — approved as-is, all recommendations taken)”
  • T1 — Audit substrate → price_decisions row (decision_stage='manual_override', source='manual_override', actor='operator', reason, chosenPrice, priorDecisionId). Keep SupportAccessAudit only as the cross-org access log.
  • T2 — Override placement → inline “Edit” pencil on the qty/price rows in the FROM DOCUMENT block; the action bar stays 4 actions (no 5th button).
  • T3 — Reason field → mandatory on the deviation / price-to-$0 path; optional for pure quantity corrections.
  • T4 — Revert affordance → included (must-fix F12).
  • Quantifying override demand (support-DB-edit + error-route counts) — owner declined the data-pull; revisit if adoption is low.
  • Splitting price-override from quantity-correction into distinct trust tiers (CEO F5) — owner chose the unified surface.
  • Generalizing accept-gross-price to “accept any resolved price” (CEO F8 alternative) — not pursued.