Skip to content
Ordermatic Docs

Simplify cross-ERP pricing config — multi-phase plan

Problem

Section titled “Problem”

Pricing config leaked P21-specific shape into the generic cross-ERP contract, and the generic gold table (erp_effective_prices) is underused and lossy. Both were fine with one real ERP; they won’t scale to “dozens more.”

Verified findings (this session, cross-checked against live production Postgres/Typesense for WhereFour/CK, P21/Genfit, and Eclipse E4/VMS, plus a direct grep sweep + Explore-agent pass over the application layer):

Correction log (post-review against main @ b4bbd2016): two claims below were wrong and have been fixed in place — the bronze_pricing.py “landmine” description was imprecise (a guard exists; see the corrected description), and auto-validate-with-price-check.ts (cited in the original blast-radius table) does not exist on main and has been removed. All file paths below were re-verified with git show origin/main:<path> from this branch (branch point == origin/main tip, confirmed via git merge-base), not against the shared/multi-tenant checkout directory used by other concurrent sessions. See “Where the ENG-545 work actually lives” below for the previously-uncommitted-code issue.

  • price1–price10 is Prophet21’s native field names, ported verbatim into the shared ProductPricing contract (apps/dagster/erp_pipeline/contracts/product.py:15-33). Every other ERP either hard-codes NULL for them (WhereFour: apps/dagster/dbt/models/bronze/raw_products.sql:36-50, comment “WhereFour’s /inventory API exposes no price field”), sets them to None with a comment admitting nothing populates them (Acumatica: apps/dagster/erp_pipeline/transformers/acumatica_products.py:99-109), or has no equivalent concept at all (VMS: pricing is 100% customer-specific, no generic list price). apps/dagster/dbt/models/bronze/raw_product_pricing.sql:14-23 documents this as a known-narrow special case in its own docstring. Also confirmed: apps/dagster/dbt/models/silver/stg_products.sql:126-169 (the silver layer) independently re-copies all 10 fields via COALESCE(NULLIF(pp.price1, 0), p.pricing__price1) — omitted from the original blast-radius table, added below.
  • erp_effective_prices (Postgres) is the right general shape on paper — keyed on (connectionId, erpCustomerId, erpItemId), already has uom/locationId/qtyBucketMin/precedence/validFrom/validTo — but only WhereFour writes to it. P21 writes zero rows, ever (refresh_effective_prices_for_connection hardcodes a wherefour_inventory_id payload-key check that P21 rows can never satisfy). qtyBucketMin/uom/locationId have zero writers anywhere in the codebase — confirmed by direct grep, schema-only dead columns. (The “now generalized via Eclipse E4” claim in the original draft referred to branch-local work not yet committed — see below; on main today this function is still WhereFour-only.)
  • pricing_resolution_strategy (ERP_RESOLVES/LOCALLY_RESOLVES/HYBRID, apps/dagster/erp_pipeline/contracts/pricing.py:18-23) already exists and is never read anywhere in the webapp runtime — confirmed by grep, zero hits in apps/webapp/src.
  • erp_pricing_rule_assignments (Postgres table) is referenced only in docstrings and a SELECT COUNT(*) health-check (pricing_materialization.py:265-298, pricing_normalization.py) — never actually populated or consulted for a pricing decision. Confirmed dead.
  • bronze_pricing.py — corrected description. Line 92 has a real guard: if not is_entity_supported(connection_id, "product_pricing", erp_type): ... return (skip, no-op). My original claim that this call is “unconditional, regardless of erp*type” was wrong — I’d captured lines 95-125 in my first pass and missed the guard sitting on line 92. The real, narrower bug: the guard checks the product_pricing capability flag, not erp_type == "prophet21" identity — so any connection whose capabilities declare product_pricing=True reaches the unconditional Prophet21SyncOrchestratorV2(connection_id) at line 114, regardless of which ERP it actually is. This is not hypothetical: apps/dagster/erp_pipeline/capabilities.py:189 already sets product_pricing=True for "acumatica" (comment: *“SalesPricesInquiry available (AR202000)”_), while Acumatica’s actual pricing model (pricing_resolution_strategy="hybrid", line 192) is meant to be resolved by a completely different, already-built-but-unwired resolver (acumatica/pricing.py:145-390). Any Acumatica connection that reaches this asset today would incorrectly run Prophet21’s orchestrator against it. Fix scope is narrower than originally stated but the bug is real and live now, not future-hypothetical.
  • WhereFour’s gold materialization throws away tier identity. The scraped tier name (“Harris Teeter, Inc - Indian Trail”) only survives as an FK-hop into raw JSON — gold stores a bare unitPrice. This is why a separate tier_name_bridge exists — getCustomerTierPricesByName in apps/webapp/src/services/erp-query/postgres-effective-price-service.ts:304-330 (not pricing-calculator.ts, which only calls it) — a live re-read of raw rows to recover identity gold already lost. Bonus finding while re-verifying this: the bridge is gated by isClevelandKitchenCompany(conn?.company) — hardcoded to one customer by name, not “is this a WhereFour connection,” reinforcing the broader thesis that customer-specific special-casing has crept into what should be generic pricing infrastructure.
  • VMS’s mined prices have real, measured uncertainty with nowhere to live. 39% of repeat (customer, item) pairs show meaningfully different historical prices; “last price predicts next order” backtests at 83–89% accuracy. Gold’s single unitPrice column can’t distinguish this from WhereFour’s hard contract price or P21’s price-level lookup — all three are the same epistemic category in the schema despite being very different kinds of fact.

Where the ENG-545 work actually lives

Section titled “Where the ENG-545 work actually lives”

The review correctly flagged that eclipse_e4_pricing_rules_sync.py, the refresh_effective_prices_for_connection generalization, and the VMS variance stats don’t exist on main — because they were never committed. I built and validated all of it earlier this session (27 passing unit tests, validated against real production Enterprise32/MSSQL data, real Typesense/ground-truth cross-referencing) but left it as uncommitted working-tree changes in this worktree instead of committing it. That was a process failure on my part, not a fictional citation — the code is real, it just wasn’t anywhere reviewable. I’m committing and pushing it now (branch claude/eloquent-napier-984f5f, PR to follow) so Phases 2 and 4 are re-reviewable against an actual base rather than my say-so.

Verified application-layer blast radius for deleting price1–price10

Section titled “Verified application-layer blast radius for deleting price1–price10”

Ran a direct grep sweep plus a dedicated Explore-agent pass (webapp components/pages, apps/internal, eval fixtures, type definitions, GraphQL/tRPC). Full result: zero UI components render a 10-price-tier picker, zero GraphQL/tRPC schemas declare these fields separately — the entire blast radius is backend data-plumbing and one resolver function.

File What it does Effort
apps/webapp/src/utils/pricing-calculator.ts:10-27, 44-100 The actual resolution logic. calculateCustomerPrice() computes fieldName = sourcePriceName.toLowerCase().replace(' ','') (e.g. "price6") and does product.pricing[fieldName] — already a dynamic bracket-notation lookup, not hardcoded branches. Also: basePrice === 0 is explicitly treated as “tier unset, not free” (ponytail comment, line 69-72) — this business rule must survive unchanged. Low — one line changes (product.pricing[fieldName] → product.pricing.tiers?.[fieldName]) if the new container keeps the same "price1".."price10" string keys
apps/webapp/src/services/search/typesense-search-service.ts:86-96, 1117-1131 Canonical Typesense-doc → TypesenseProduct conversion (the root of the propagation below) Low — one conversion point
apps/webapp/src/services/erp-query/typesense-query-service.ts:78-89 A second, redundant doc → object conversion (DRY violation worth collapsing into #2 while we’re in here) Low
apps/webapp/src/utils/matching-helpers.ts:540-579 TierPriceFields type + productTierPrices() — already generically “collect all positive tier prices, drop zeros/absent”; good existing design, just needs its input type updated Low
apps/webapp/src/utils/fuzzy-matching.ts:245-254, 401-410 Two explicit 10-field copy-sites Low
apps/webapp/src/utils/cross-reference-lookup.ts:64-73 One explicit 10-field copy-site Low
apps/webapp/src/pages/api/erp/parts/auto-validate.ts:340-349 One explicit 10-field copy-site Low
apps/webapp/src/pages/orders/pdf/[documentId]/review.astro:293-304 Display-only, no dedicated UI form Low
packages/eval/src/runners/auto-validation-runner.ts:347-356 Eval harness copy-site Low
apps/dagster/dbt/models/silver/stg_products.sql:126-169 Missed in the original draft. The silver dbt model independently re-copies all 10 fields, preferring a location-level override over the root/canonical value: price1 := COALESCE(NULLIF(pp.price1, 0), p.pricing__price1) × 10. Needs the same container change on both sides of the COALESCE. Low — mechanical, but dbt Jinja/SQL not TS, so it’s a distinct edit surface from everything else in this table
apps/webapp/src/utils/pricing-calculator.test.ts, fuzzy-matching.part-mapping.test.ts, auto-validate.test.ts Fixtures/assertions hardcode price1/price7 values Low — fixtures need updating, test logic doesn’t change
packages/integrations/src/prophet21/transaction-api.ts:24-33, 137-146, 225-290, 402-411 P21-native SOAP/REST client — P21’s real API genuinely has 10 price levels. Keep reading Price1-Price10 from P21 here; only the output shape this adapter hands to the generic contract changes. Low (adapter boundary only)
apps/webapp/src/services/erp/implementations/prophet21-service.ts:1962, 2068 P21 SOAP submit — only ever sends price1 (P21’s primary sell price), not all 10 None — unaffected
apps/dagster/erp_pipeline/sources/prophet21/client.py:277-337 Prophet21Client.get_item_price_levels() — parses P21’s real Price1..Price10 API response None — this is the legitimate source, stays as-is

Bottom line, corrected: 11 generic-layer files (TS) + 1 dbt SQL model need a mechanical container change (flat fields → one dict, same key strings), 3 test files need fixture updates, and 3 P21-native files are correctly untouched. (The original draft listed a 12th file, auto-validate-with-price-check.ts — confirmed via git show origin/main:... that it does not exist on main; dropped.) No UI risk, no API-contract risk. This is a real but low-risk migration — the risk is entirely in sequencing it safely against live P21/WhereFour traffic, not in the size of the diff.

Line-range note: file:line citations above were spot-verified against origin/main @ b4bbd2016 at write time; treat exact line numbers as approximate if the file has moved since — re-grep before editing, don’t trust the numbers blindly.

Phase 1 — Safe, zero production risk (do first, proves the thesis cheaply)

Section titled “Phase 1 — Safe, zero production risk (do first, proves the thesis cheaply)”
  1. Fix the bronze_pricing.py:114 landmine (corrected scope). The existing is_entity_supported(..., "product_pricing", erp_type) guard (line 92) checks a capability flag, not ERP identity — add an explicit erp_type == "prophet21" check (or route through a per-ERP factory, the way erp_source_factory.py already does for sync orchestrators) before instantiating Prophet21SyncOrchestratorV2. This is not risk-free-because-hypothetical: Acumatica’s capabilities entry already has product_pricing=True (capabilities.py:189), so this fixes a bug that would fire on the next Acumatica connection that reaches this asset, not just a future one. Still zero risk to the current working P21 path — P21 connections keep hitting the exact same code they do today.
  2. Drop erp_pricing_rule_assignments. Confirmed zero real writers/readers (only a docstring and a SELECT COUNT(*) health-check). Drizzle migration to drop the table; update pricing_materialization.py’s count_pricing_source_rows to stop querying it (or return a static 0 for that half of the tuple, whichever keeps the health-check’s callers happy with minimal change).
  3. Delete the pricing_normalization.py docstring/comments referencing the never-filled silver ETL, or leave them but stop calling it aspirational — cosmetic, but keeps the “how gold works” mental model honest for the next engineer.

Perceived value: removes a live footgun (#1) and a maintenance-confusing dead abstraction (#2/#3) with essentially zero chance of regressing anything currently working. Good first PR to build confidence before touching schemas that live customers depend on.

Phase 2 — Safe, additive gold-table columns (no reader changes yet)

Section titled “Phase 2 — Safe, additive gold-table columns (no reader changes yet)”
  1. Add three nullable columns to erp_effective_prices (Drizzle migration, generated not hand-written per AGENTS.md):
    • source_label: text — human-readable identity of the pricing construct (WhereFour: tier name; P21: price-level name, once P21 starts writing here; VMS: null, no equivalent concept).
    • confidence: numeric(3,2) + observation_count: integer — nullable; VMS populates from order-price variance (coefficient of variation across a (customer, item)’s historical orders), everyone else leaves null (meaning “certain, don’t ask”).
  2. Update WhereFour’s pricing_materialization.py refresh_effective_prices_for_connection to populate source_label from the tier-scrape rows’ raw_payload.tier_name going forward (one more SELECT expression in the existing CTE — no backfill required; old rows just have source_label=null until the next routine materialization cycle re-runs, which already happens). — done, #2089.
  3. Update eclipse_e4_pricing_rules_sync.py to populate confidence/observation_count from per-(customer, item) order-price variance. — done, #2089.

Perceived value: unlocks the ability to eventually delete tier_name_bridge (a live workaround for gold’s current data loss) and gives auto-validate a generic, ERP-agnostic way to threshold (“flag for review if confidence < 0.9”) instead of hardcoding VMS-specific logic later. Zero risk: additive nullable columns, no existing reader changes, nothing consumes the new fields yet so there’s nothing to regress.

Phase 3 — Medium risk: delete price1–price10, expand-contract style (P21/Genfit is live — no hard cutover)

Section titled “Phase 3 — Medium risk: delete price1–price10, expand-contract style (P21/Genfit is live — no hard cutover)”

Do not cut over in one PR. P21/Genfit has real customers depending on this path today. Use expand → migrate → contract:

3a. Expand (additive, safe on its own).

  • Add pricing.tiers: Record<string, number | null> (keeping the exact same string keys "price1".."price10" the codebase already computes via sourcePriceName.toLowerCase().replace(' ','') — this is the detail that makes the riskiest function, calculateCustomerPrice, a near-zero-logic change) to: the dagster ProductPricing contract, the Typesense products schema, and the webapp’s TypesenseProduct/ProductPriceInfo types — alongside the existing flat price1-price10 fields, not instead of them.
  • Dual-populate: P21’s product transformer + sync_product_pricing() write both the flat fields (unchanged) and the new tiers dict (mechanical, from the same source data).
  • Ship this with zero reader changes. Nothing consumes tiers yet.

3b. Migrate readers (one file at a time, each independently shippable/testable).

  • pricing-calculator.ts: product.pricing[fieldName] → product.pricing.tiers?.[fieldName] (preserve the basePrice === 0 → null “unset, not free” rule exactly — this is the one piece of business logic in the whole blast radius that a refactor could silently break).
  • typesense-search-service.ts / typesense-query-service.ts: change the doc→object conversion to read/pass through tiers instead of reconstructing 10 named properties (collapses two redundant conversion sites into a shared helper while here — real simplification, not just a rename).
  • matching-helpers.ts’s productTierPrices(): iterate Object.values(p.tiers ?? {}) plus list_price, instead of 10 explicit push() calls — shorter and handles the dict generically.
  • fuzzy-matching.ts, cross-reference-lookup.ts, auto-validate.ts, review.astro, packages/eval/.../auto-validation-runner.ts: each becomes { tiers: source.pricing?.tiers, list_price: source.list_price } instead of a 10-line explicit map.
  • apps/dagster/dbt/models/silver/stg_products.sql: same container change on both the pp.price1..10 and p.pricing__price1..10 sides of each COALESCE(NULLIF(...), ...) expression.
  • Update the three test files’ fixtures to the new shape; assertions don’t need logic changes (per the Explore agent’s finding — they check computed prices, not field names, except fuzzy-matching.part-mapping.test.ts:91 which asserts on .pricing.price1 directly and needs updating to .pricing.tiers.price1).
  • packages/integrations/src/prophet21/transaction-api.ts: keep reading Price1-Price10 from P21’s real API (this is correct, native P21 shape); change only the return type of getItemPriceLevels()/getCustomerPricingStructure() to build the tiers dict instead of 10 named properties, at the adapter boundary.

3c. Verify zero live readers of the flat fields, then contract: remove price1-price10 from the contract/schema/types in a final PR. Grep-clean confirmation before merging (grep -rn "\.price[0-9]\b" across apps/webapp, apps/dagster, packages should return only the P21-native SOAP/transaction-api files that legitimately keep the literal names for talking to P21 itself).

Perceived value: removes ~10 dead columns’ worth of schema surface from every non-P21 ERP forever, collapses two redundant Typesense-doc conversion sites into one, and turns a “10 hardcoded fields” contract into “one dict P21 populates and nobody else has to think about.” Risk is real but bounded and reversible at every step — expand/migrate/contract means P21 keeps working unmodified through 3a and 3b; only 3c is a point of no return, and it’s gated on a grep-clean check.

Phase 4 — Higher risk, highest structural value: make pricing_resolution_strategy the real dispatch key

Section titled “Phase 4 — Higher risk, highest structural value: make pricing_resolution_strategy the real dispatch key”

Currently resolveCustomerPriceWithCache unconditionally tries gold → WhereFour tier-name-bridge → P21 price-tier fallback for every ERP regardless of fit. Refactor it to branch on the connection’s pricing_resolution_strategy first:

  • ERP_RESOLVES (P21-style: real native per-customer price + a customer-side pointer) → resolve via the ERP-specific calculator only; never materialize into erp_effective_prices (stop scheduling gold materialization for these connections — it produces zero rows today and always will under this model).
  • LOCALLY_RESOLVES (WhereFour/VMS-style: no native API, mined/scraped, precomputed) → the only strategy that reads erp_effective_prices, using source_label/confidence from Phase 2. Once source_label is confirmed populated and the discount resolver’s SKU-bridge is updated to read it (stage behind the existing price_tier_review_enabled flag pattern already used for this exact class of pricing-authority risk, per pricing-authority-signoff-plan), delete tier_name_bridge (getCustomerTierPricesByName, apps/webapp/src/services/erp-query/postgres-effective-price-service.ts:304-330) — a real subtraction, not just a refactor.
  • HYBRID (Acumatica-style: live hierarchical resolve with break-quantity tiers, already built in acumatica/pricing.py:145-390, currently unwired) → resolve via the ERP’s own resolver at request time, optionally writing a cache-only gold snapshot at low precedence so it never wins over a live re-resolve.

Why this is the actual scale lever: a future ERP with a simple flat catalog declares ERP_RESOLVES + list_price and needs zero pricing-pipeline code. A future ERP needing order-history mining (like VMS) declares LOCALLY_RESOLVES and reuses the exact erp_pricing_rules_raw → erp_effective_prices pipeline — see the ENG-545 branch work (pending PR, see “Where the ENG-545 work actually lives” above) that generalizes refresh_effective_prices_for_connection’s item-key lookup via COALESCE so a second LOCALLY_RESOLVES ERP can share it without a schema change. No future ERP should ever need a new pricing table — only a strategy tag and, if LOCALLY_RESOLVES, one source-specific sync op.

Perceived value: highest of all four phases — this is what actually stops “dozens more ERPs” from each needing bespoke pricing-resolution code. Risk: highest of all four phases too — it changes the runtime resolution order for both live P21 and live WhereFour connections simultaneously. Do this last, behind a flag, with the tier-name-bridge deletion staged separately and only after source_label has been live long enough to trust it.

Phase 5 — Separate track, low risk: VMS prompt_config visibility in Langfuse

Section titled “Phase 5 — Separate track, low risk: VMS prompt_config visibility in Langfuse”

Not pricing, but same “config management” theme and low-hanging:

  • Keep erp_connections.promptConfig (Postgres) as the runtime source of truth — hardening.py iterates company_info.aliases as a structured Python list at extraction-hardening time; flattening that into a rendered Langfuse prompt body would break it (no machine-readable data left for the seller-name-filter logic to iterate).
  • Add one create_prompt() call to the existing render pipeline (get_pdf_extraction_prompt_from_config) that mirrors the rendered prompt into a Langfuse-labeled version (e.g. vms-labeled) purely for audit/diff visibility, after each render — the same pattern the existing DSPy org-overlay system (resolve_dspy_org_overlay, langfuse_prompts.py:295-357) already uses.
  • Do not wire this mirror back into the runtime read path.

Perceived value: satisfies “we need visibility into Langfuse” without the structural risk of migrating structured per-connection data into a prose-templated prompt system built for a different cardinality of override.

Section titled “Recommended sequencing”
  1. ✅ Phase 1 (bronze_pricing landmine fix + drop erp_pricing_rule_assignments) — merged, #2088.
  2. Phase 2 (gold source_label/confidence/observation_count columns) — #2089 (2.1/2.2 schema + WhereFour source_label, then 2.3 VMS confidence/observation_count population once #2086 landed).
  3. Phase 5 (Langfuse mirror) — independent track, ship whenever convenient.
  4. Phase 3 (price1-10 expand/migrate/contract) — three PRs minimum (3a, 3b, 3c), each independently reversible until 3c.
  5. Phase 4 (strategy-dispatch refactor + tier_name_bridge deletion) — last, behind a flag, only after Phase 2’s source_label has been live and trusted for at least one full materialization cycle on WhereFour.