docs(orders,currency): order-time FX rate snapshot + reporting-currency stamping

[norbert-kulus-blockydevs]

Docs-only: ADR-040 + the implementation plan for #2049. No production code, no migration, no libs/ or apps/ change — the ADR is Proposed and the plan is what the implementation PR executes.

What the ADR decides

At ingestion, stamp every order with the amount it represents in a reporting currency, plus an immutable reference to the rate used — so analytics can report one figure per period instead of an un-summable pile of per-currency totals. Read-time conversion is rejected: only the rate that applied at placement is defensible, and it must survive every re-ingestion.

The substantive question is converted into what?, and the answer changed during review. Three things were being conflated — the currency the buyer paid in (a fact about the order), the currency a shop prices in (a fact about a connection, config.currency / #362), and the currency we report in (a choice made by the business). Deriving the third from the first two is a category error, and per-connection values produce an estate with no single total, which is the problem the feature exists to solve. So:

1. One system-level setting, resolved settings row → OL_REPORTING_CURRENCY → 'EUR', on the ai_provider_active_setting singleton shape. Validated at save time; renders as EUR (default) until explicitly set. config.currency keeps its meaning and is not consulted.
2. The rate source is a function of the reporting currency — PLN → NBP, EUR → ECB, one call per order that needs one. Because NBP quotes everything against PLN and ECB quotes EUR against everything, every pair is direct or a single documented inversion; no pivot arises for either supported value.
3. Convert only when the order's currency differs. The equal case costs no I/O and no rate row.
4. A shared append-only exchange_rates registry keyed (source, from, to, rateDate) — 500 EUR orders on one day resolve to one row. Direction is a stated invariant: a stamp is always total × rate, never a division.
5. The resolution is snapshotted at the first stamp attempt, so the retry job and the reconcile sweep stamp as of ingestion rather than re-resolving.
6. The stamp is a narrow conditional UPDATE, never the persistOrder upsert path, so re-ingestion cannot move a reported figure.
7. Port + registry + rules in libs/core/src/currency/ with no outbound HTTP; every adapter in a new @openlinker/integrations-fx package — the AiCompletionPort / @openlinker/integrations-ai split, and not conditioned on whether a source needs a credential.
8. Changing the setting is forward-only. The PUT reports how many rows already carry a stamp so the two-era split is accepted knowingly; restatement is filed as [TASK] CORE - Restate already-stamped orders when the reporting currency changes #2096.

Invoicing computes its own rate and does not consume this stamp — they differ on date (placement vs the art. 19a tax point, which for a prepaid marketplace order is the payment instant no shipped source persists), target (always PLN statutorily) and derivation (a statutory rate must be a directly published table-A quote). FA(3) draws the same line: KursWaluty is scoped to dział VI ustawy, KursUmowny / WalutaUmowna to "nie dotyczy przypadków, o których mowa w dziale VI".

Changed from the first draft of this PR

The original ADR argued the opposite on three of these. It has been rewritten, not patched:

Was	Now
A four-rung connection-derived ladder; a global setting "explicitly rejected"	One system-level setting; the ladder is the rejected alternative
Connection.config.fx + readFxConfig + a ConnectionPort read in the FX path	Deleted — the rule and source are code constants, the currency is system-level
NBP provider inside libs/core, "the first outbound HTTP call in libs/core", bounded by a credential-based split rule	Both adapters in @openlinker/integrations-fx; the split rule is dropped entirely
"whoever implements KursWaluty must consume this stamp"	Reversed — invoicing computes its own; three-axis reason + the FA(3) scoping
Five order_records columns	Six — fxIntendedCurrency carries the first-attempt snapshot
The WooCommerce placedAt mapping folded in	Split back out, with the objection sharpened rather than dropped (#2097)

The plan follows the same shape: the ladder step is deleted, Phase 1 splits into a core half and the new package, a settings stack and the intent snapshot are added, and Phase 4's three connection-setup fields and Allegro OAuth thread are replaced by a settings tile. Roughly 60% of the document changed; the verified line-number references and in-repo precedents that were unaffected are untouched.

Two things a reviewer should look at directly

The group-integrity CHECK had to change. Its first arm required fxRule IS NULL, but the intent claim writes fxRule while reportingCurrency is still NULL — so the pre-revision constraint would have rejected every intent row and made the snapshot unimplementable.

fxStampedAt IS NULL stops meaning "unstamped" once a deferred row exists. The plan carries a five-state table for the column group, because a consumer using the wrong predicate silently conflates "in flight" with "unstampable".

Numbering

ADR-040 stays. main carries 035–038 + 041 with 039 / 040 / 042 open; main's own reserved-numbers note already reads "040 by #2050"; and #2066 claims 043 / 044 / 045, so renumbering to 043 would create the collision rather than avoid one. This PR's merge resolves the README index conflict by keeping both rows (040 here, 041 from main) and corrects the reserved-numbers note, which was stale in three places. Mechanical guarding is #2082.

Not in this PR

• The implementation itself (that is Order-time FX rate snapshot + reporting-currency stamping #2049's own PR, executing this plan).
• Restatement of already-stamped orders — [TASK] CORE - Restate already-stamped orders when the reporting currency changes #2096.
• The WooCommerce placedAt mapping — [BUG] Integration - WooCommerce order source does not populate placedAt (blocks FX stamping, leaves invoices without a saleDate) #2097. Consequence accepted and documented: a foreign-currency WooCommerce order stays terminal-unstamped, and the demo therefore shows an unstamped count rather than a conversion.
• The docs/architecture-overview.md § Currency section — added by the implementation PR; the ADR now says so rather than asserting it exists.
• The [IMPL] Backend — sales & channel aggregates endpoint (revenue, orders, AOV + median, units) #1987 / [IMPL] Backend — top-products endpoint with inline per-channel split #1988 acceptance-criteria edit, which is the epic owner's call — noted on [PRODUCT-DESIGN] Business analytics surface at /analytics — sales layer + cross-channel layer #1976.

Known blocker for the implementation, not for this PR

ECB's daily XML feed carries only the latest day, so it cannot serve a prev-business-day rule. The historical/SDMX endpoint's response shape and its non-publication-day behaviour (404 vs empty series vs nearest prior day) need the same treatment the NBP section got before EcbExchangeRateAdapter is written. Flagged in the plan as a Phase 1b blocker; every other phase is unblocked.

Test plan

Docs-only. prettier --check passes on all three files. No code, so lint / type-check / test have nothing to exercise; the plan's own § 9 carries the test contract the implementation PR is held to, including the manual migration:run → revert → run round-trip that CI cannot cover (nothing in CI or the harness runs a migration).

Refs #2049

[piotrswierzy]

Tech-lead review — ❌ Request changes (bookkeeping, not substance)

Docs-only: three files, ~1,500 additions, zero production code — the new ADR (Proposed), the ADR README index row, and the implementation plan. No migration, no libs//apps/ changes, so I applied the backend checklist to the design the plan commits to rather than to code.

The FX design itself is unusually good — see the verification list at the bottom. The blocker is numbering.

BLOCKING

1. ADR-number collision on 040. I verified this independently of the review: main's highest ADR is 038, and two open PRs both claim 040:

• this PR → 040-order-time-fx-stamping-connection-derived-reporting-currency.md
• #2056 → 040-fiscalisation-capability.md

Both also add an ADR-040 row at the same README table position, so whoever merges second silently overwrites the other's row and ships a duplicate number into an append-only record. This is the docs equivalent of the migration-timestamp collision that already bit #2022/#2037 this week — each PR passes in isolation, only the second is wrong, and nothing automated catches it.

The allocation is inconsistent across the whole cluster, not just here: #2056's README note says "039 is reserved … authored in #2051 / PR #2055 … Allocate 041 for the next new ADR", while #2055 actually ships 041-sales-document-routing-policy.md; meanwhile this PR's body attributes 039 to #1985's analytics read model. So 039 has two claimants and 040 has two claimants.

Please settle the allocation for #1985 / #2050 / #2055 / #2056 in one place — a reservation table in docs/architecture/adrs/README.md is the natural home, and #2056 already drafted one — then renumber. Mechanical, but it has to happen before merge: the README's own ADRs are append-only rule makes a post-hoc renumber the expensive option.

IMPORTANT

2. The PR body describes a different design from the merged artifacts. The body names …connection-derived-**base**-currency.md, a three-rung ladder starting at Connection.config.currency, baseCurrency columns, four nullable columns, and migration 1833000000000. The ADR ships a four-rung ladder (explicit config.fx.reportingCurrency first) with the reporting-currency rename (and a good rationale for it), and the plan pins migration 1834000000000 with five columns. The body is the first thing a reviewer reads and currently teaches the superseded design.

3. Rung 3 makes the stamped reporting currency depend on when stamping happened. The per-order figure is immutable, but the plan's own test matrix asserts "retry stamps a different reporting currency than the inline path would have" as expected behaviour. So two orders placed in the same minute on the same connection can carry different reporting currencies purely because one degraded to the retry path. Defensible — but it's a different trade-off from the one the ADR's Cons section describes, which frames drift as slow historical accumulation, not retry-timing nondeterminism. Either freeze the ladder result per connection once first resolved, or state the retry divergence explicitly. This is exactly the kind of thing a future analyst files as a data bug.

4. Write-once and append-only are conventions with no enforcement. The ADR is admirably plain about it ("append-only by convention — nothing enforces it at the database level"), and the ShipmentRepository.claimWaybillRelay precedent is the right in-repo shape, so not a blocker. But for a table whose entire purpose is that financial history cannot be rewritten, consider a DB-level guard, or at minimum an int-spec asserting no update path exists.

5. The consuming issues may contradict this model. The ADR notes #1987/#1988 currently specify "multiple currencies never sum into a single figure" and defers reconciliation to them. Worth confirming with those owners before this lands as Proposed, or the analytics epic ships two incompatible contracts.

SUGGESTION

• Length: ~1,866 words against the README's current "under 500 words" bar. #2055 proposes raising it to 1,500 — this still exceeds even that. Not worth cutting the load-bearing parts, but note the cross-PR dependency.
• *.provider.ts isn't in the engineering-standards.md suffix table; since ExchangeRateProviderPort exists and these implement it, *.adapter.ts is the documented name.
• The plan leaves one decision open — whether persistOrder's returned OrderRecord mirrors the stamped FX fields or reports null after a successful stamp. Pick it now; a stale returned record is precisely the class of silent-wrong-number this ADR exists to prevent.
• Two adjacent findings worth their own issues: fulfillmentState (#1108) is reset to null by a re-poll on the upsert path, and prestashop-order.mapper.ts:247 hardcodes const conversionRate = 1.0; outbound. Correctly out of scope here.

Verified — the FX-specific failure modes are all properly closed

• Provenance/immutability: stamped at ingestion, never recomputed, written by a narrow conditional UPDATE deliberately outside persistOrder's full-row save(). The JSONB-blob alternative was correctly dropped because re-ingestion would rewrite the snapshot.
• Quote direction is an explicit invariant, not a convention: rate = to units per one from unit, from is always the order's currency, so the stamp is always total × rate, never a division — pinned by a unit test asserting an exact numeric result, with a worked cross-rate including a sanity check for the flipped-divide tell. This is the classic silent-inversion bug and it's genuinely closed.
• As-of instant is unambiguous: placedAt, prev-business-day, resolved in Europe/Warsaw, calendar owning weekends/holidays with the 404 walk-back only as defence — and the ADR states plainly that this is an analytics figure, not a fiscal one, because PL VAT anchors on the tax point rather than placement. That distinction is the thing most likely to be conflated later.
• Precision/rounding: rate at 8 dp as a string; reportingTotalAmount matching #1985's numeric(12,2) via the house round2 idiom; multiply-the-total chosen explicitly over converting lines.
• Missing-rate is never a silent default: terminal (no stamp, no retry) for missing placedAt / unsupported pair / non-404 4xx / divergent pivot legs; transient (unstamped + retry) for 5xx/timeout; plus a reconcile sweep backstopping a dead retry job holding its idempotency key.
• Same-currency: stamps reportingTotalAmount = totals.total with exchangeRateId: null and no I/O, and names reportingCurrency — not exchangeRateId — as the unstamped discriminator. Right call for downstream aggregation.

Merge readiness: ❌ Blocked only on the ADR-040 collision (and the 039 ambiguity). Renumber, refresh the stale body, and settle item 3 in the ADR text — after that this is an approve. The design work is solid and the plan is implementation-ready.

[piotrswierzy]

Product decisions following a design discussion — ADR needs revision before implementation

The FX mechanics in this ADR are good and I don't want them changed: the direction invariant, the shared rate registry, the narrow conditional UPDATE, terminal-vs-transient failure handling, the same-currency short-circuit, and the analytics-not-fiscal framing all stand.

What changes is the reporting-currency model. Five decisions below, with reasoning, since otherwise they'll get re-litigated at implementation time.

---

1. The reporting currency is system-level, not derived per connection

Drop rung 1 (Connection.config.fx.reportingCurrency) and rung 3 (dominant native currency from history). Replace the ladder with a single system-level setting.

Reasoning: reporting currency is a property of the reporting entity — the business — not of a sales channel. A connection is a place orders arrive from; it has no view on what currency we keep books in. Three distinct things were being conflated:

Level	What it is	Kind
Order	the currency the buyer paid in	fact
Connection	the currency that shop prices products in	fact
Business	the currency we report in	choice

The ladder derives a choice from facts. That's the category error. Concretely it caused two problems the ADR already documents as costs: per-connection values produce an estate that by construction cannot be summed (the exact problem this feature exists to fix), and rung 3's history-derived value drifts silently as a connection's order mix changes — the "two reporting-currency eras" consequence. Both disappear when there's one authoritative answer.

Connection.config.currency (#362) stays and keeps its current meaning and readers — it's a genuine fact about a shop. It just stops being an input to "what do we report in."

I understand the original rejection was of a mandatory global setting, and that concern was right — "a configuration step that silently breaks analytics when skipped" is a real failure mode. Optional-with-a-default answers it: skipping it is the normal path.

2. Default is EUR

Resolution chain, mirroring the ai_provider_active_setting precedent exactly:

settings row  →  OL_REPORTING_CURRENCY  →  'EUR'

3. Consequence: ECB becomes the shipped provider, not NBP

This follows from (2) and isn't optional. NBP table A quotes everything against PLN as the target (1 USD = 4.05 PLN), which is uniquely convenient for PLN reporting and structurally wrong for EUR:

Order currency	→ EUR via NBP	Legs
PLN	invert EUR→PLN	1 inversion
USD	rate(USD→PLN) ÷ rate(EUR→PLN)	2-leg pivot

For a PL operator the dominant case is PLN orders, so with NBP every headline figure becomes a derived cross. ECB reference rates collapse every pair to a single documented inversion instead.

The ADR anticipates this as future work — "the provider interface takes a pivotCurrency so a second source (e.g. ECB) is additive" — it just assumed additive-later. With an EUR default it's required in this slice.

To be clear: inversion isn't a correctness problem, it's exact and the ADR already records how a derived rate was obtained. This is about leg count and citability.

4. Providers are adapters in libs/integrations/fx/, not implementations inside core

Decision 5 currently places NbpExchangeRateProvider in libs/core/src/currency/infrastructure/providers/ and concedes it makes NBP "the first outbound HTTP call in libs/core", bounded by this rule:

a rate provider that needs a credential or an SDK ships as libs/integrations/fx-*

That criterion is the problem. It puts two implementations of one port in different packages based on whether the vendor happens to require an API key — an incidental property of someone else's auth policy. It isn't enforceable by check:invariants, and the day NBP or ECB adds a key the adapter has to move packages.

The ADR cites AiCompletionPort + @openlinker/integrations-ai as its precedent, but that split is port in core, every implementation in integrations. This does port in core, some implementations in core. It invokes the precedent to justify departing from it.

The kernel of the argument is right — "a published reference rate is a shared read, not a per-connection integration capability" — an FX rate has no connectionId and must not be a capability plugin resolved via getCapabilityAdapter. But that argues against a capability plugin, not for core. integrations-ai is exactly the third option: an integration package that is not a per-connection capability, implementing a core port, with a router dispatching to per-vendor adapters.

So:

• ExchangeRateProviderPort, the registry, and the rate-date rules stay in libs/core/src/currency/. No HTTP in core.
• libs/integrations/fx/ ships NbpExchangeRateProvider + EcbExchangeRateProvider.
• Drop the credential-based split rule entirely — the boundary becomes the ordinary one, with no exception to memorise.

Resolution is by purpose, not a single active provider. This is a genuine difference from the AI precedent, where the setting picks one winner. Here NBP and ECB must be live simultaneously — see (5). The exchange_rates table is already keyed (source, from, to, rateDate), so multiple concurrent sources are designed into the data model; only the code layering assumed one.

Killing the in-core HTTP precedent matters independently: once it exists, the next person with a "simple unauthenticated GET" has a citable reason to add one.

5. Invoicing must compute its own rate — reverse the current instruction

This line no longer holds and will cause a wrong number on a fiscal document if left as-is:

whoever implements it must consume this stamp rather than compute a second, divergent rate for the same order

A Polish invoice's FA(3) KursWaluty needs an NBP rate to PLN on the statutory date. The analytics stamp will be an ECB rate to EUR on the placement-derived date. Different source, target and date rule — invoicing cannot consume it.

Please reverse the sentence: invoicing computes its own NBP/PLN rate, and the two figures legitimately coexist and differ. State it as deliberate, or the first implementer will follow the current text.

---

Guards to specify while revising

• Label every analytics figure with its currency at the point of consumption ("Revenue — reported in EUR"). With a converting default, this is what makes a wrong setting announce itself on the page instead of being found by an accountant months later.
• Restatement moves into scope, or the setting locks after first stamp. The ADR currently defers backfill entirely, which was fine when the default converted nothing. With a converting default, an operator who changes their mind in month three has three months of EUR stamps and no remedy. Everything needed is retained (native currency, placedAt, the registry) so it's computable — but somebody has to write it. What to avoid is a changeable setting with no restatement path.
• Validate the setting against provider coverage at save time rather than one order at a time.
• Show it as EUR (default) until explicitly set, and put OL_REPORTING_CURRENCY in .env.example so a self-hoster discovers it before their first order.

Still standing from my earlier review

• ADR-040 is claimed twice (this and #2056) — renumber per the coordination table.
• The PR body and issue #2049 both describe the superseded design (baseCurrency, three rungs, migration 1833000000000). Since #2049 carries the acceptance criteria someone implements against, it needs the same refresh — and now a larger one.

Happy to talk any of these through before you revise.

[piotrswierzy]

Correction to decision 3 — NBP ships too

My heading above ("ECB becomes the shipped provider, not NBP") was badly worded and could be read as dropping NBP. It isn't. The body of that section already listed libs/integrations/fx/ shipping both NbpExchangeRateProvider and EcbExchangeRateProvider — the heading should have read "ECB becomes the default provider for the EUR reporting path."

Both providers ship in this slice. Three independent reasons, any one of which is sufficient:

1. Fiscal — FA(3) KursWaluty needs the NBP statutory rate to PLN. Per decision 5, invoicing computes its own rate rather than consuming the analytics stamp, and that rate is NBP's.
2. Demo — the demo deployment reports in PLN, so it resolves rates directly against NBP with no inversion or pivot.
3. It's already specified in the plan, so it isn't additional work.

This makes the demo the first real consumer of the env rung: the demo sets OL_REPORTING_CURRENCY=PLN while the product default stays EUR. Useful side effect — both provider paths get exercised in practice rather than one sitting untested: NBP-direct under the demo, ECB-inverted under the default configuration.

One thing to catch in the demo seed

If the demo reports in PLN and its orders are all PLN, the equal-currency short-circuit fires on every order and no conversion ever happens — the feature would be invisible in the demo despite being fully implemented.

The demo seed should include a handful of foreign-currency orders (EUR and one non-EUR, so the pivot path shows too) so that reportingTotalAmount, exchangeRateId and the joined rate row are actually populated and visible on the analytics surface.

Worth noting the two provider paths differ in shape, so exercising both is genuinely worth something:

Deployment	Reporting	Provider	Rate shape
Demo	PLN	NBP	direct published lookup (X→PLN)
Default	EUR	ECB	single documented inversion

[norbert-kulus-blockydevs]

Reply: taking all five decisions, three need your call before I rewrite

Accepted as-is, not re-litigating: system-level reporting currency and the fact/choice framing, EUR default on the ai_provider_active_setting chain, both providers in libs/integrations/fx/ with no HTTP in core, the credential-split rule dropped, resolution by purpose, Connection.config.currency (#362) unchanged, and all four guards.

Three of the decisions do not survive contact with the code. Each one is a question for you, marked below - the rest of this comment is detail you can skip.

---

❓ Q1 - The system-level setting does not fix your review item 3. Add a first-attempt snapshot?

Your item 3 (retry stamps a different currency than inline would have) and decision 1 are being read as one fix. Decision 1 removes the history-driven drift. It leaves the retry-timing one untouched, because the setting is still read at stamp time - and it widens the blast radius:

• Retry window is ~4.3h (maxAttempts: 10, 30s * 2^(n-1) capped at 6h). An admin edit inside it makes the degraded order stamp under the new value while its same-minute neighbour keeps the old one.
• The reconcile sweep has unbounded lag by design. Under a deployment-wide setting, one admin edit reclassifies an arbitrary slice of history with nothing recording which cohort a row belongs to. Under the ladder that was one connection.

It also does not need rung 3: rung 4 ("no history, no conversion") is time-dependent too, so inline-vs-retry can flip an order between converted and not converted at all.

Proposal: resolve once at the first stamp attempt and persist it (fxIntendedCurrency, one guarded claim write in the proven claimWaybillRelay shape); retry and sweep read that snapshot. Model-agnostic, and it turns the remaining divergence into a time boundary ("we changed the setting on 3 Aug") that an analyst can explain.

The ask: confirm the snapshot is a precondition of decision 1, not an alternative to it. Without it, decision 1 hands an admin edit a deployment-wide reach the ladder never had.

❓ Q2 - Decision 5 is right, but the plan ships NBP-only, so the trap is worse. Fine to state the divergence on three axes, not one?

You reasoned that invoicing cannot consume the stamp because it will be ECB/EUR while KursWaluty needs NBP/PLN. Correct conclusion, wrong premise: the branch ships CHECK ("source" IN ('nbp')) and defaults to source: 'nbp'. So both figures would come out of the same published table, which removes the one cue that would have made a future implementer suspicious.

Three independent divergences, any one fatal: date (placement vs tax point - and under art. 19a ust. 8 that is the payment instant, which no shipped source persists, so placement is structurally never the tax point for a prepaid marketplace order), target (always PLN vs the reporting currency - dimensionally unusable on an EUR deployment), derivation (a statutory rate must be a directly published table-A quote).

FA(3) draws the same line itself: KursWaluty (schemat_fa3_v1-0e.xsd:3199) is scoped to dzial VI (the statutory conversion), while KursUmowny / WalutaUmowna (:3490) is scoped "Nie dotyczy przypadkow, o ktorych mowa w dziale VI". A self-computed rate may only ever occupy KursUmowny.

The ask: OK to write the reversal as per-provider rather than flat? KSeF is the only path where OL builds the document and would own the rate; inFakt and Subiekt compute their own conversion server-side and must not be handed one.

❓ Q3 - With a converting default, does restatement ship here, or do we take the acknowledgement rail?

Lock-after-first-stamp should be rejected, and the reason comes from your own decision 2: the default converts and nobody chose it. Lock makes an unchosen default permanent - a deployment that never opens the settings page takes one foreign-currency order and is locked into a currency it was never asked about. With NBP as the source it stings twice: there is no statutory PLN to EUR rate, so a locked-in EUR default converts a PL estate's PLN orders through an inverted quote that appears in no published table.

Restatement is ~+0.5 to 1 day, because it is a third caller of the existing stamp primitive, not a new subsystem: native currency and placedAt are retained, the registry is keyed (source, from, to, rateDate) so restating N orders costs distinct(pair x rateDate) reads (~65 GETs for three months of one pair, not N), and the job type, handler pattern, idempotency key and ADR-007 split all exist for the retry job. It needs decision 4 refined, not contradicted: the stamp is immutable with respect to ingestion, and the sole writer that may move it is the explicit, audited restatement path.

The ask: pick (a) restatement in this PR, or (b) changeable + a hard rail - the PUT requires an explicit acknowledgement when stamped rows exist, the response reports the count, and the restatement issue is filed and linked from the ADR before merge. Not lock.

---

Detail: 11 smaller decisions I am taking unless you object

• The setting lives in the new currency context, not orders. Save-time coverage validation needs the provider list, which lives in currency; putting it in orders creates an orders -> currency value dependency just for validation and breaks currency's leaf property. The precedent is three identical stacks (ai_provider_active_setting, mailer_settings, posthog_settings), and the AI view's activeUpdatedAt === null is already the (default) discriminator you asked for.
• Save-time validation splits block/warn. Reachability against a shipped table-A membership list blocks (422, pure array test, no I/O). Coverage against currencies OL has actually ingested only warns - blocking on history would let one junk currency in an old snapshot make a legitimate currency unsettable forever.
• Connection.config.fx goes too. Your fact/choice argument applies identically to the FX rule and the rate source. Dropping the key deletes readFxConfig, its types and spec, and the ConnectionPort read in the FX path. The rule stays persisted per stamp for auditability.
• .env.example is two files. apps/api/.env.example is dev; the root ./.env.example is the demo/self-host list docker-compose.demo.yml actually reads. Demo needs the root one plus a compose passthrough, with PLN as the demo compose default, not a product-wide claim.
• Plus apps/worker/.env.example. The worker runs the retry and the sweep, so a divergent fallback between processes stamps differently before the first settings row exists.
• CurrencySettingsController needs a write-guard-coverage.spec.ts entry, or the new write endpoint ships without guard coverage, silently.
• *.adapter.ts, not *.provider.ts. Once they live in an integration package the plan's counter-argument disappears, and integrations-ai already ships non-capability *.adapter.ts files. No standards-table edit needed.
• Append-only: spec pair, no DB trigger - and for a repo-specific reason worth recording so it is not re-proposed. No migration here creates a trigger, rule or function, and decisively the integration harness builds its schema with TypeORM synchronize and never runs a migration, so a migration-only trigger would be absent in dev and CI and would first fire in production. REVOKE UPDATE is a no-op because the app connects as a superuser.
• persistOrder re-reads conditionally, following the recordCancellationIfNeeded precedent, with both post-upsert writers collapsed into one refresh - otherwise the cancellation re-read can run first and the stamp land after, returning a stale record again.
• The direction invariant needs one clause changed. Decision 3 states from/to as a property of the registry; it is a property of the stamp. As written, a future invoicing row (to = 'PLN' on an EUR deployment) is illegal in its own table, though the (source, from, to, rateDate) key was designed to hold it.
• The fiscal rate should be snapshotted onto InvoiceRecord at issue time (issuedLineSnapshot [TECH-DEBT] CORE — Persist issuance-time line snapshot on InvoiceRecord for safe KSeF corrections #1297 precedent). The registry is genuinely shareable as a cache; "append-only by convention" is fine for an analytics figure and not fine under a cleared document.

Detail: consumers, labelling and the demo seed

• The consumer conflict is three issues wide. [IMPL] Backend — order analytics read model (queryable line items, placed-at, currency, tax treatment) #1985 contradicts too ("a query spanning multiple currencies cannot return a single summed figure", plus an Out-of-scope line excluding a single reporting currency). The disagreement is narrow but live: everyone groups by a currency, but [IMPL] Backend — order analytics read model (queryable line items, placed-at, currency, tax treatment) #1985/[IMPL] Backend — sales & channel aggregates endpoint (revenue, orders, AOV + median, units) #1987/[IMPL] Backend — top-products endpoint with inline per-channel split #1988 group by the native column and the ADR by the stamped one, so their ACs as written forbid the exact behaviour this feature enables - an implementer following [IMPL] Backend — sales & channel aggregates endpoint (revenue, orders, AOV + median, units) #1987 literally ships GROUP BY currency and leaves the FX columns unread. Proposal: amend [IMPL] Backend — sales & channel aggregates endpoint (revenue, orders, AOV + median, units) #1987 and [IMPL] Backend — top-products endpoint with inline per-channel split #1988 only (the query issues, and the stamp's consumers), add Blocked by: #2049 to both, leave [IMPL] Backend — order analytics read model (queryable line items, placed-at, currency, tax treatment) #1985's AC alone with one Out-of-scope clarification. Note [IMPL] Backend — order analytics read model (queryable line items, placed-at, currency, tax treatment) #1985 has not landed - the analytics-trust module on this branch is feat(analytics-trust,sync): add analytics data-trust reads #2037 (ingestion freshness, no money), a different feature.
• Currency labelling: per-figure in the response. A header cannot work (the by-channel table can legitimately hold two reporting currencies); a settings query is worse, since it re-derives on the client a value the server stamped per order, so a stale answer mislabels a correct figure - the exact failure the guard exists to prevent. MoneyFigureDto { amount, currency, isReportingCurrency } per figure plus one unstampedOrderCount per response. isReportingCurrency is load-bearing: without it the FE cannot tell a converted EUR figure from one that could not be stamped and is shown natively. Since features/analytics/ does not exist yet, a MoneyKpiCard that requires a MoneyFigure makes a currency-less money tile a type error.
• There is no OL-side order seeder at all - zero INSERT INTO order_records repo-wide, and PS_DEMO_MODE: 0, so PrestaShop's own fixtures are not installed either. The only order-creating seeder is WooCommerce's, and it creates them in WooCommerce.
• So the foreign-currency orders go on PrestaShop. The WC order source sets only createdAt, no placedAt, so a foreign-currency WC order can never be stamped. Relatedly the demo WC shop runs USD (woocommerce_currency is set nowhere), so the demo already carries unstampable non-PLN orders - day one, its first FX-visible artifact would be the failure path.
• The pivot path is not demonstrable by seeding orders alone. With PLN reporting, NBP quotes both EUR and USD directly against PLN. A real pivot needs a non-PLN reporting currency on a source that emits placedAt. Suggest seeding PLN + EUR + USD on PrestaShop and demonstrating inversion/pivot provenance from the exchange_rates row, and saying so plainly rather than implying the demo covers it.
• Seed the rates deterministically, do not fetch. The rate that matters is not today's, it is the one for resolveRateDate(placedAt, ...), so the seed pins placedAt to fixed historical dates first (PS lets the seeder set date_add) - the only way a pre-seeded row is guaranteed to be the one looked up. It also keeps demo boot independent of NBP uptime and revenue figures stable across screenshots. Seeded rows carry real published values with their genuine sourceRef.

---

Filing separately (out of scope here): fulfillmentState (#1108) reset to null by a re-poll on the upsert path; prestashop-order.mapper.ts:247 hardcoding const conversionRate = 1.0; outbound; infakt-invoicing.adapter.ts sending no currency at all (zero .currency references - a EUR order is booked as if the numerals were PLN); docs/dev-environment.md:247 and :375-380 claiming PS_DEMO_MODE=1 with sample orders, contradicting compose.

Fixing on this PR: #2049's body still uses baseCurrency / baseTotalAmount and links a filename that does not exist, so it needs the same rename the ADR applied; and the ADR's ## References forward-references a docs/architecture-overview.md § Currency section the implementation PR has to actually add.

Answer Q1-Q3 and I will rewrite the ADR and plan in one pass.

[norbert-kulus-blockydevs]

Resolved - all five decisions taken, with three corrections. Revising the ADR now.

Q1-Q3 from my previous comment are settled. Summary of what the ADR is being rewritten to, so the diff is not a surprise.

Your decisions, taken

1. Reporting currency is one system-level setting, not derived per connection. Resolution chain mirrors ai_provider_active_setting: settings row → OL_REPORTING_CURRENCY → 'EUR'. Connection.config.currency (Resolve PrestaShop product currency from shop config or connection setting #362) keeps its meaning and readers, and stops being an input.
2. Both providers ship in libs/integrations/fx/. No HTTP in libs/core. The credential-based split rule is dropped entirely.
3. Invoicing computes its own rate. The instruction to consume the analytics stamp is reversed.
4. Guards: label every figure with its currency, validate the setting at save time, show EUR (default) until set, OL_REPORTING_CURRENCY in .env.example.

Three corrections that came out of the discussion

(a) The provider is a function of the reporting currency, not a separate config key. PLN → NBP, EUR → ECB, one call per order. This is simpler than the "resolution by purpose" abstraction I was going to build, and it removes Connection.config.fx entirely (with it: readFxConfig, its types and spec, and the ConnectionPort read in the FX path). The rule is still persisted per stamp for auditability.

Consequence worth stating: because NBP quotes every table-A currency against PLN and ECB quotes EUR against everything, every pair is either direct or a single inversion while the reporting currency is PLN or EUR. The pivot machinery stays in the code (open for extension), but save-time validation accepts only PLN and EUR for now. A third reporting currency needs a third provider anyway.

(b) The resolution is snapshotted at the first stamp attempt. Not a new requirement, just making the plan match the intent: the retry job and the reconcile sweep must stamp as it was at the time of the cron, so a config change mid-retry cannot move an order's reporting currency. One nullable column plus one guarded claim write in the existing ShipmentRepository.claimWaybillRelay shape; retry and sweep read the snapshot instead of re-resolving. Two mechanical follow-ons: the migration's group CHECK has to drop its fxRule clause (an intent row would otherwise violate it), and the plan's test-matrix row asserting "retry stamps a different reporting currency than the inline path would have" inverts into asserting the ladder is never consulted once the intent is persisted.

(c) KursWaluty diverges on the date even when the provider is identical. My earlier framing (ECB/EUR vs NBP/PLN) was not the sharpest version of the problem, because the branch ships NBP-only. The sharp version, on a PLN-reporting install where there is no provider choice at all:

An order of EUR 1,000 placed Fri 7 Aug, goods delivered Mon 10 Aug. Same shop, same currency, same provider, same order.

	Rate date	NBP EUR/PLN (illustrative)
Analytics stamp	day before placement → Thu 6 Aug	4.2810
FA(3) KursWaluty	day before the tax point (art. 19a) → Fri 7 Aug	4.2935

Two legitimate numbers from one published table, differing only by date. Under art. 19a ust. 8 the tax point is the payment instant when the buyer paid first, which is the OL default for a marketplace order and a timestamp no shipped source persists. So placement is not merely a different date, it is structurally never the tax point for the most common order shape.

Worth adding because it settles the argument from the schema side: FA(3) draws this line itself. KursWaluty (schemat_fa3_v1-0e.xsd:3199) is scoped to dział VI (the statutory art. 31a conversion), while the separate KursUmowny / WalutaUmowna pair (:3490) is scoped "Nie dotyczy przypadków, o których mowa w dziale VI". A self-computed rate may only ever occupy KursUmowny.

The reversed sentence will also say that rate ownership is per-provider: KSeF is the only path where OL builds the document and would own the rate; inFakt and Subiekt compute their own conversion server-side and must not be handed one.

On restatement: not shipping it

Your review asked for a restatement path or a lock. Taking neither: the ADR will state plainly that changing the setting does not restate history and that a deployment can therefore carry two reporting-currency eras, the PUT will report how many already-stamped rows exist so the choice is informed, and a restatement issue gets filed and linked from § Migration path. Lock-after-first-stamp is rejected for the reason your own decision 2 implies: the default converts and nobody chose it, so a lock would make an unchosen default permanent.

The WooCommerce caveat in the ADR is wrong, and it is a one-line fix

The ADR and the plan both record that a foreign-currency WooCommerce order can never be stamped because WC emits no order-placed timestamp. The data is there and simply is not mapped: woocommerce-order-source.adapter.ts:178 sets only createdAt: normGmt(order.date_created_gmt, order.date_created), while the PrestaShop adapter maps date_add → placedAt with the comment "PrestaShop date_add is when the customer placed the order" (prestashop-order-source.adapter.ts:233-249). WooCommerce's date_created is the same fact.

So the caveat comes out of the ADR and a one-line placedAt mapping goes into this PR. Useful side effect for the demo: the demo WC shop runs USD (woocommerce_currency is set nowhere in the tree), so its two seeded orders become genuine converted orders and the demo needs no PrestaShop order seeder to show a populated reportingTotalAmount.

Smaller decisions being taken in the revision

Nine items, expand if you want to object to any

• The setting lives in the new currency context, not orders - save-time coverage validation needs the provider list, which lives in currency; putting the setting in orders creates an orders → currency value dependency just for validation and breaks currency's leaf property. The precedent is three identical stacks (ai_provider_active_setting, mailer_settings, posthog_settings), and the AI view's activeUpdatedAt === null is already the (default) discriminator you asked for.
• Save-time validation splits block/warn - reachability against the provider list blocks (422, pure array test, no I/O); coverage against currencies OL has actually ingested only warns, since blocking on history would let one junk currency in an old snapshot make a legitimate currency unsettable forever.
• .env.example is two files - apps/api/.env.example is dev, the root ./.env.example is the demo/self-host list docker-compose.demo.yml actually reads. Plus apps/worker/.env.example, because the worker runs the retry and the sweep and a divergent fallback between processes would stamp differently before the first settings row exists. PLN as the demo compose default, not a product-wide claim.
• CurrencySettingsController gets a write-guard-coverage.spec.ts entry, or the new write endpoint ships without guard coverage silently.
• *.adapter.ts, not *.provider.ts - once they live in an integration package the plan's counter-argument disappears, and integrations-ai already ships non-capability *.adapter.ts files. No standards-table edit needed.
• Append-only: spec pair, no DB trigger. Recording the repo-specific reason so it is not re-proposed: no migration here creates a trigger, rule or function, and decisively the integration harness builds its schema with TypeORM synchronize and never runs a migration, so a migration-only trigger would be absent in dev and CI and would first fire in production. REVOKE UPDATE is a no-op because the app connects as a superuser.
• persistOrder re-reads conditionally, following the recordCancellationIfNeeded precedent, with both post-upsert writers collapsed into one refresh - otherwise the cancellation re-read can run first and the stamp land after, returning a stale record again.
• The direction invariant gets one clause changed - decision 3 states from/to as a property of the registry; it is a property of the stamp. As written, a future invoicing row (to = 'PLN') would be illegal in its own table, though the (source, from, to, rateDate) key was designed to hold it.
• The fiscal rate should be snapshotted onto InvoiceRecord at issue time (issuedLineSnapshot [TECH-DEBT] CORE — Persist issuance-time line snapshot on InvoiceRecord for safe KSeF corrections #1297 precedent). The registry is shareable as a cache; "append-only by convention" is fine for an analytics figure and not fine under a cleared document.

Consumers

The conflict is three issues wide, not two: #1985 contradicts as well ("a query spanning multiple currencies cannot return a single summed figure", plus an Out-of-scope line excluding a single reporting currency). The disagreement is narrow but live - everyone groups by a currency, but #1985/#1987/#1988 group by the native column and this ADR by the stamped one, so their ACs as written forbid the behaviour the feature enables. An implementer following #1987 literally ships GROUP BY currency and leaves the FX columns unread.

Proposal: amend #1987 and #1988 only (they are the query issues and the stamp's consumers), add Blocked by: #2049 to both, and leave #1985's AC alone with one Out-of-scope clarification. That is an AC edit on two unstarted issues, so it belongs to the owner of epic #1976 and is not a blocker for merging this. Note #1985 has not landed - the analytics-trust module on this branch is #2037 (ingestion freshness, no money, no currency), a different feature.

For the labelling guard: currency goes per figure in the response (MoneyFigureDto { amount, currency, isReportingCurrency }) plus one unstampedOrderCount per response. A response-level header cannot work because the by-channel table can legitimately hold two reporting currencies, and a client-side settings query is worse - it re-derives a value the server stamped per order, so a stale answer mislabels a correct figure, which is the exact failure the guard exists to prevent.

Filing separately

fulfillmentState (#1108) reset to null by a re-poll on the upsert path; prestashop-order.mapper.ts:247 hardcoding const conversionRate = 1.0; outbound; infakt-invoicing.adapter.ts sending no currency at all (zero .currency references, so a EUR order is booked as if the numerals were PLN); docs/dev-environment.md:247 and :375-380 claiming PS_DEMO_MODE=1 with sample orders, contradicting docker-compose.yml:229.

Also fixing on this PR: #2049's body still uses baseCurrency / baseTotalAmount and links a filename that does not exist, and the ADR's ## References forward-references a docs/architecture-overview.md § Currency section the implementation PR has to actually add.

ADR revision incoming; the plan follows in the same pass.

[piotrswierzy]

Tech-lead re-review — ❌ Request changes (reasoning settled; docs haven't caught up, plus the renumber)

Your resolution comment lands all three questions and I agree with every one — including where you overturned me. Blocking items are the ADR number and the fact that the revision itself isn't written yet; on the substance we're aligned, so the rewrite should be uncontroversial.

Q1–Q3, settled

Q1 — snapshot at first stamp attempt: yes, precondition, not alternative. You're right that decision 1 doesn't close it. I conflated "removes history-driven drift" with "removes timing drift"; the setting is still read at stamp time, and your point that rung 4 can flip an order between converted and not converted at all is the part I missed entirely. The ~4.3h retry window plus an unbounded reconcile lag makes an admin edit reclassify an arbitrary slice of history — and under a deployment-wide setting that's strictly worse than the per-connection version, which is a real cost of decision 1 that I should have priced. fxIntendedCurrency in the claimWaybillRelay shape is right, and turning the residual into an explainable time boundary is the correct goal. Both mechanical follow-ons you name (group CHECK drops its fxRule clause; the test-matrix row inverts to assert the ladder is never re-consulted) look correct.

Q2 — per-provider, yes. And your version of the argument is better than mine. I reasoned from ECB/EUR vs NBP/PLN; you're right that the branch ships NBP-only, so both figures would come from the same published table — which removes the one cue that would make a future implementer suspicious, making the trap worse rather than better. The date-only divergence on a PLN-reporting install (placement vs art. 19a tax point, EUR 1,000 on 7 Aug giving 4.2810 against 4.2935 from one table) is the sharp version, and the art. 19a ust. 8 observation that the tax point is the payment instant — which no shipped source persists — means placement is structurally never the tax point for a prepaid marketplace order. That's a stronger claim than "different provider" and it survives any provider choice.

The FA(3) schema citation settles it from the other side: KursWaluty scoped to dział VI vs KursUmowny/WalutaUmowna scoped "nie dotyczy przypadków, o których mowa w dziale VI". Please keep that in the ADR — it's the detail that stops this being re-argued.

Q3 — third option accepted, and my lock suggestion was wrong. Your objection kills it: the default converts and nobody chose it, so a lock makes an unchosen default permanent, and on a PL estate an unchosen EUR lock converts PLN orders through an inverted quote that appears in no published table. Changeable + PUT reporting the stamped-row count + the two-era consequence stated plainly is the right shape.

One condition on that: the restatement issue must be filed and linked from § Migration path before this merges, not after. A filed-and-linked issue is what separates a documented trade-off from a silent one, and this is precisely the kind of follow-up that evaporates once the feature ships and looks finished.

Corrections you took that I want to endorse explicitly

Provider as a function of reporting currency is simpler than the "resolution by purpose" I proposed, and it's correct once invoicing owns its own rate (Q2) — "by purpose" collapses to analytics resolves by currency, invoicing computes its own. Dropping Connection.config.fx, readFxConfig and the ConnectionPort read in the FX path is real deletion, not just relocation.

I checked the coverage claim and it holds: with NBP quoting X→PLN and ECB quoting EUR→X, PLN reporting gives direct lookups on every pair and EUR reporting gives exactly one inversion on every pair. No pivots either way.

State the consequence as a limitation, though: save-time validation accepting only PLN and EUR makes the reporting currency an effective two-value set at launch. That's defensible and follows from provider coverage — it just needs saying in the ADR rather than being discovered by the first operator who wants GBP.

Direction invariant as a property of the stamp, not the registry — agreed, and it's the right catch. A future to = 'PLN' invoicing row must be legal in a table whose key was designed to hold it.

One thing to check before folding it in

The WooCommerce placedAt mapping has a side effect the ADR itself flagged. You're right that the data is there and unmapped — woocommerce-order-source.adapter.ts:178 sets only createdAt, while PrestaShop maps date_add → placedAt with a comment saying it's when the customer placed the order, and WC's date_created is the same fact. Good catch, and it does fix the demo for free.

But the current ADR gives a reason for keeping it separate: "Populating that field is a one-line adapter change but it also moves invoicing's saleDate for every WooCommerce order, so it is tracked separately rather than folded in here." Folding it in now silently changes saleDate on every existing WooCommerce order — a fiscal-document field, on already-issued documents. Either address that consequence in this PR, or keep the split the ADR argued for. What I'd avoid is deleting the caveat without answering the objection it recorded.

Blocking

1. ADR number. Still 040. #2056 vacated it, but #2066 has since claimed 039, 040 and 041, so this now collides with #2066 rather than #2056 — next free is 043. The renumber has second-order reach: the filename, the in-file # ADR-043: heading, the README row, ~8 ADR-040 references in the plan (§5, §8, checklist), and the PR body. Worth one mechanical pass so no stale ADR-040 survives.

I've filed #2082 for a check-adr-numbers.mjs invariant so this stops costing review rounds — it's now bitten three times in two days.

2. The PR body still describes the superseded design and is wrong on five points: "three-step ladder", baseCurrency rungs, "Four nullable columns", "pinned to 1833000000000", and a link to 040-…-base-currency.md — a filename that no longer exists in the branch. It also still says the stamp uses "the rate that applied on the day the buyer paid", which the ADR now explicitly disowns.

3. The design revision isn't in the documents yet. Decisions 1–5 and all four guards are absent, and on 1, 4 and 5 the ADR currently argues the opposite position — Context still says a global setting was "explicitly rejected by the operator", Alternatives still rejects it, decision 5 still says NBP is "the first outbound HTTP call in libs/core" with the credential-split rule intact, and both documents still say invoicing "must consume this stamp". Merging now would enshrine a rejected design as a Proposed ADR. Your sequencing is fine — I'm noting it so the state is on the record.

Smaller items

• The append-only reasoning (harness builds schema via TypeORM synchronize and never runs migrations, so a migration-only trigger would first fire in production; REVOKE UPDATE is a no-op under a superuser connection) belongs in the ADR, not only in a PR comment. It's exactly the reasoning someone will otherwise re-propose.
• ADR § References forward-references a docs/architecture-overview.md § Currency section this PR doesn't add. Fine for the implementation PR to add — the ADR just shouldn't assert it exists.
• Your amend-#1987-and-#1988-only proposal (leaving #1985's AC with an Out-of-scope clarification) is the right split, and the observation that an implementer following #1987 literally ships GROUP BY currency and never reads the FX columns is the concrete reason it matters.
• MoneyFigureDto { amount, currency, isReportingCurrency } per figure is better than the response header I suggested — the by-channel table can legitimately hold two reporting currencies, and isReportingCurrency carrying the converted-vs-shown-natively distinction is the part a header couldn't express.
• All nine smaller decisions in your details block look right to me; no objections.

Merge readiness: ❌ Blocked on the renumber to 043, the PR body refresh, and the rewrite itself. The reasoning is settled and I don't expect to re-open any of it — this is now mechanical. Please confirm the WooCommerce saleDate question either way before folding that mapping in.

[norbert-kulus-blockydevs]

ECB research done — the Phase 1b blocker is cleared, and it found a bug in the plan's rate-date rule

Ran against the live API today. Every command below is reproducible as-is; outputs are verbatim.

Host: https://data-api.ecb.europa.eu/service/data/EXR/D.{CUR}.EUR.SP00.A. The legacy sdw-wsrest.ecb.europa.eu that most tutorials cite no longer resolves at all (curl: (6) Could not resolve host), so anything written against it is dead.

1. The daily feed is unusable, as assumed

$ curl -s 'https://www.ecb.europa.eu/stats/eurofxref/eurofxref-daily.xml'
<Cube time='2026-08-13'>
  <Cube currency='USD' rate='1.1534'/>
  <Cube currency='PLN' rate='4.3048'/>
  ...

One day per document. Confirmed — a prev-business-day rule cannot use it.

2. endPeriod + lastNObservations=1 removes the walk-back entirely

This is the finding that matters. The API itself resolves "the last publication on or before date X":

$ curl -s '…/D.PLN.EUR.SP00.A?endPeriod=2026-08-08&lastNObservations=1&format=csvdata&detail=dataonly'
KEY,FREQ,CURRENCY,CURRENCY_DENOM,EXR_TYPE,EXR_SUFFIX,TIME_PERIOD,OBS_VALUE
EXR.D.PLN.EUR.SP00.A,D,PLN,EUR,SP00,A,2026-08-07,4.2983

2026-08-08 is a Saturday; it answered with Friday the 7th. One request, no loop, no calendar knowledge in the adapter. TIME_PERIOD is the actual rate date, so it is what gets persisted as rateDate — exactly how the NBP adapter treats effectiveDate.

It handles clusters a walk-back-by-one would get wrong:

endPeriod	What it is	Resolved TIME_PERIOD	Rate
2026-04-03	Good Friday	2026-04-02	4.2855
2026-04-06	Easter Monday	2026-04-02	4.2855
2026-05-01	Labour Day	2026-04-30	4.2605
2026-01-01	New Year	2025-12-31	4.2210

The Easter case skips four days and crosses a year boundary in one call.

3. A non-publication day is HTTP 200 with an empty body, not 404

$ curl -s -o /dev/null -w '%{http_code} bytes=%{size_download}' '…?startPeriod=2026-08-08&endPeriod=2026-08-08&format=csvdata'
200 bytes=0

Different from NBP, which 404s. So an adapter written on the NBP shape would treat "no publication" as success-with-no-data and fall through to whatever its parser does with an empty string. Worth pinning in a spec.

4. An unknown currency is a real 404, with a distinguishable body

$ curl -s '…/D.XYZ.EUR.SP00.A?endPeriod=2026-08-13&lastNObservations=1&format=csvdata'
{"type":"…","title":"Not Found","status":404,"detail":"No Series was returned for the query: …"}

So supports() has a clean discriminator, and it is not the same signal as (3): 404 = unsupported pair (terminal), empty 200 = no observation at or before that date (also terminal, but a different reason).

5. ⚠️ This invalidates the plan's shared-calendar rate-date rule

The plan has resolveRateDate derive the candidate day from the Polish working-day calendar and treats each provider's walk-back as mere defence. For ECB that produces a silently stale rate, because the two calendars genuinely diverge — proven twice:

$ …?startPeriod=2026-06-04&endPeriod=2026-06-04   ->  2026-06-04,4.2368   # Corpus Christi (PL holiday, Thu)
$ …?startPeriod=2026-01-06&endPeriod=2026-01-06   ->  2026-01-06,4.2105   # Epiphany (PL holiday, Tue)

ECB publishes on both. Concrete failure — an order placed Friday 2026-06-05, rule prev-business-day:

$ …?startPeriod=2026-06-03&endPeriod=2026-06-05&format=csvdata&detail=dataonly
…,2026-06-03,4.2383
…,2026-06-04,4.2368   <- correct: ECB's last publication before Fri 05 Jun
…,2026-06-05,4.2338

The PL calendar skips Thursday the 4th (Corpus Christi) and lands on Wednesday the 3rd → 4.2383 instead of 4.2368. Never throws, off by 0.0015 PLN per EUR (~0.035%), and wrong in the same direction every year on every PL-only holiday.

Fix, and it simplifies things: the rule yields a plain previous calendar day, and each adapter resolves the actual published day with its own mechanism — NBP keeps the PL calendar plus its 404 walk-back (so its common case stays one request), ECB uses lastNObservations=1 and needs no calendar at all. That deletes the "shared isPlWorkingDay helper serves both" assumption rather than papering over it.

6. The future-date clamp is load-bearing, not defensive

$ …?endPeriod=2026-12-24&lastNObservations=1   ->  2026-08-13,4.3048

A future endPeriod returns HTTP 200 with a rate four months stale. No error, no signal. The plan's min(resolved, todayInWarsaw) clamp is what stands between a source reporting a future date_add and a confidently wrong stamp.

7. Decimal places vary per currency — NBP's fixed 4 dp does not transfer

  PLN  4.3048   DECIMALS=4
  GBP  0.8549   DECIMALS=5
  JPY  183.77   DECIMALS=2
  HUF  362.85   DECIMALS=2

The series metadata carries its own DECIMALS column. numeric(18,8) holds all of them, so nothing changes in storage — but the plan's rounding-bounds paragraph is argued from "NBP publishes mid to 4 dp" and needs an ECB variant: a 2-dp JPY quote carries ~100× the relative error of a 4-dp one.

8. Smaller operational facts

• Reference time is 14:15 CET, not 16:00 — straight from the series metadata: "ECB reference exchange rate, Polish zloty/Euro, 2.15 pm (C.E.T.)". Only matters if a same-day rule is ever added.
• No EUR/EUR identity series — D.EUR.EUR.SP00.A is a 404. The same-currency short-circuit must precede any provider call, which the plan already requires.
• Multi-currency batching works: D.PLN+USD+GBP+JPY.EUR.SP00.A?…&lastNObservations=1 returns all four in one response. Not needed per-order, but it is what makes a restatement wave ([TASK] CORE - Restate already-stamped orders when the reporting currency changes #2096) cheap.
• detail=dataonly trims the response from 32 columns to 8. format=jsondata is also available; CSV is simpler to parse and has no ambiguity here.
• Headers: content-type: text/csv, cache-control: max-age=30, no advertised rate-limit headers, no auth.

Plan updates pushed with this

§ 4 now carries a full ECB section on the same footing as NBP (verified, not flagged), § 6 step 2 and step 3b carry the split-calendar rule, the § 8 rounding paragraph gets its ECB variant, and § 10's "ECB research blocks Phase 1b" item is closed. Phase 1b is unblocked.

The one thing I have not verified is how far back the daily series goes (it clearly covers 2025-12-31, and EUR reference rates start 1999-01-04) — irrelevant for stamping at ingestion, but a restatement or backfill wave should confirm it rather than assume.

[norbert-kulus-blockydevs]

ADR-040 confirmed free — checked against every open PR, not from memory

You asked me to settle the number rather than assert it, so I enumerated all 14 open PRs and diffed each one's file list for docs/architecture/adrs/NNN-. Commands are reproducible as-is; run date 2026-08-14.

What main carries today

$ git ls-tree --name-only origin/main docs/architecture/adrs/ | grep -oE '/[0-9]{3}-' | tr -d '/-'
001 … 038  041

Contiguous 001-038, then 041. 039, 040 and 042 are gaps on main, and main's own reserved-numbers note names their claimants.

Every open PR that touches an ADR file

PR	New ADR number(s)	Note
#2014	039-order-analytics-read-model-persistence-strategy.md	new
#2050 (this)	040-order-time-fx-stamping-against-a-system-reporting-currency.md	new
#2056	042-fiscalisation-capability.md	new; also edits 026 (VAT-rate annex)
#2058	—	edits 014 only, no new number
#2066	043 / 044 / 045	new

The other nine open PRs (#2099, #2098, #2060, #2057, #2048, #2018, #2008, #1769, #1396) touch no ADR file at all.

Post-merge union, computed rather than eyeballed

$ { git ls-tree --name-only origin/main docs/architecture/adrs/ | grep -oE '/[0-9]{3}-' | tr -d '/-';
    for n in 2014 2050 2056 2058 2066; do
      gh pr view $n --json files --jq '.files[].path' | grep -oE 'adrs/[0-9]{3}-' | grep -oE '[0-9]{3}';
    done; } | sort | uniq -c | awk '$1>1'
      2 014
      2 026

The only two numbers appearing twice are 014 and 026, and in both cases the second occurrence is an edit to an ADR that already exists on main (#2058 amends 014, #2056 annexes 026) — not a second claim on a free number.

Every new number is claimed exactly once. After all 14 open PRs merge the sequence is 001-045 with no gap and no duplicate.

Why 043 would have been the collision

Your re-review asked for 043 on the basis that #2066 had taken 039/040/041. It hasn't:

$ gh pr view 2066 --json files --jq '[.files[].path] | map(select(test("adrs/0")))'
["docs/architecture/adrs/043-order-lifecycle-derived-from-fact-ledger.md",
 "docs/architecture/adrs/044-order-changeset-proposed-then-confirmed.md",
 "docs/architecture/adrs/045-pack-policy-per-connection-config.md"]

041 is already merged (#2055), and #2056 vacated 040 for 042 after your first review. So renumbering this PR to 043 would land on top of #2066 — creating exactly the collision the renumber was meant to prevent.

Keeping 040. The README row and the reserved-numbers note in this branch are already updated to match the table above (039 -> #2014, 042 -> #2056, 043/044/045 -> #2066, 040 no longer reserved because this PR now carries its row).

Note this is a point-in-time check: it is only stable until someone opens a new PR claiming a number. That is precisely the hole #2082 closes, and it is the argument for landing that invariant rather than repeating this by hand.

[norbert-kulus-blockydevs]

Adjacent findings now actually filed

My earlier comment listed these under "Filed separately", which overstated it — they were identified, not filed. They are now, one issue each, so nothing depends on a PR comment surviving:

Issue	Finding
#2101	persistOrder never passes fulfillmentState (12th ctor param, defaults null), and upsert is a full-row save() — so any re-poll clobbers the value written by updateFulfillmentState. A dispatched order reappears as not-shipped and re-enters its ship-by SLA bucket. This is the one you flagged as adjacent to #1108.
#2102	prestashop-order.mapper.ts:247 const conversionRate = 1.0;, sent as conversion_rate. Cross-currency orders book at parity while lines stay pinned at the source price via cart-scoped specific_prices (#895), so header and lines can disagree on one document.
#2103	InfaktInvoice (infakt.types.ts:50-81) has no currency field and the adapter has zero .currency references, so IssueInvoiceCommand.currency is dropped — a EUR invoice books as PLN, silently, and relays to KSeF.
#2104	docs/dev-environment.md:247,375-380 promises PS_DEMO_MODE=1 demo data; docker-compose.yml:229 sets 0. This one cost time during this PR's own demo analysis.

All four carry the file:line evidence, a proposed shape, and acceptance criteria; all are explicitly independent of #2049 (#2102 and #2103 each state why they must not consume the ADR-040 stamp, matching § Consequences).

Also recorded while tidying up: the MoneyFigureDto { amount, currency, isReportingCurrency } shape you endorsed over a response-level currency header existed only in this thread. It is now written into the note on #1976, with the two reasons it has to be per-figure — a by-channel breakdown can legitimately carry both a stamped reporting-currency group and an unstamped native-currency group at once, and isReportingCurrency carries the converted-vs-shown-natively distinction that a bare currency code cannot (PLN means opposite things on a PLN-reporting and a EUR-reporting deployment).

That leaves the ADR number as the only open item from your re-review — evidence in the comment above.

[piotrswierzy]

Tech-lead re-review — ✅ Approve

The rewrite landed as one pass, not a patch. ADR-040 (249 lines) argues the system-level setting from Context through Alternatives, and the plan deletes the ladder step outright rather than editing around it.

First: I was wrong about the number, and you were right to push back

My instruction to renumber to 043 was incorrect and would have created a collision. Verified directly against every branch:

Number	Holder
039	#2014
040	#2050 — free, correctly held
041	merged to main via #2055
042	#2056
043 / 044 / 045	#2066 (renumbered from its earlier 039/040/041)

The "ADR-040" in #2066's body is the pre-renumber name of what is now its 043. So there was nothing to renumber here and no half-rename to find. Apologies for the churn — and your README reservation note is now more accurate than main's, which still lists the stale claims.

Checklist

#	Item
1	System-level currency; per-connection + history rungs dropped	✅
2	Default EUR on the ai_provider_active_setting chain	✅
3	Provider = f(reporting currency); both ship; config.fx gone; limitation stated	✅
4	Providers in libs/integrations/fx/, no HTTP in core, split rule dropped, *.adapter.ts	✅
5	Invoicing instruction reversed, per-provider, date + FA(3) argument	✅
6	First-attempt snapshot + both mechanical follow-ons	✅
7	Restatement stated, PUT count, issue filed and linked	✅
8	Guards	⚠️
9	Direction invariant as a property of the stamp	✅
10	WooCommerce placedAt kept split, saleDate addressed	✅
11	ADR number	✅
12	PR body refreshed	✅

1. Context frames the fact/fact/choice table and concludes "one system-level answer" (:18-29); Decision 1 says config.currency "keeps its meaning and readers … and is explicitly not consulted" (:44-45); the ladder is the first rejected alternative with all four objections (:112-116). The defect I flagged as most likely — Decision 1 updated while Context/Alternatives still argue the opposite — did not occur. No surviving "explicitly rejected by the operator" text anywhere.

3. :47-54, and the two-value limitation is stated as a limitation in its own Cons bullet — "a real limitation and not a temporary omission" (:173-177). That's the honest framing.

4. :86-95 and :121-125. Notably plan :172 adds libs/integrations/fx to check-outbound-http.mjs SCAN_ROOTS — the no-HTTP-in-core rule is now enforced, not just asserted. That's better than what I asked for.

5. Per-provider at :161-163, three-axis argument with art. 19a ust. 8 at :150-156, and the FA(3) citation exact: KursWaluty at schemat_fa3_v1-0e.xsd:3199 scoped to dział VI, KursUmowny/WalutaUmowna at :3490 scoped "Nie dotyczy przypadków, o których mowa w dziale VI".

6. Both mechanical follow-ons landed, and the second in a stronger form than proposed: the test row now asserts a populated fxIntendedCurrency means the settings service is never called and the stamped currency equals the intent even after the live setting changed (:1641). That tests the property rather than the absence.

7. #2096 exists, is open, is linked by full URL from § Migration path, and its own AC includes amending § Decision 4 to the refined invariant. The condition I set is met.

10. Kept split, and the consequence is addressed head-on rather than dropped: saleDate is gated on placedAt, so a WC invoice carries none today and the provider substitutes; populating it moves saleDate empty → placement date and "can move an invoice into a different tax period". Already-issued documents are shown safe via InvoiceRecord + issuedLineSnapshot (#1297), narrowing exposure to a future invoice on an existing order. #2097 carries the VAT-period cutoff as an explicit AC. That's the right answer to the objection rather than a deletion of it.

IMPORTANT — the one partial

8 — the per-figure labelling shape is not specified anywhere. No MoneyFigureDto { amount, currency, isReportingCurrency }, no unstampedOrderCount in either document. What exists is the principle ("every analytics figure is labelled with its currency where it is consumed", :41-42) and a recommendation to amend #1987/#1988 to expose an unstamped count.

Since the consuming DTOs live in those issues and the body correctly defers the AC edit to the epic owner, this is placement rather than omission — but name the concrete shape on #1987/#1988 before they're implemented, or each will invent its own. isReportingCurrency is the load-bearing part: without it the FE cannot distinguish a converted figure from one that couldn't be stamped and is shown natively.

Save-time validation, EUR (default), and OL_REPORTING_CURRENCY across all three .env.example files plus the demo compose passthrough all landed, with the worker file's rationale spelled out.

Suggestion

Plan :1705-1706 notes #2049's issue body still describes the superseded design (baseCurrency, three rungs, migration 1833000000000, dead ADR filename). It carries the criteria someone implements against, and the plan flags it but nothing closes it. Worth editing before the implementation PR opens.

CI

7/8 green. Lint is red and inherited — main itself carries two migrations at 1833000000002 (add-identifier-mappings-offer-created-index and create-refund-records), so check:invariants fails on the trunk. This PR touches three files under docs/, so it cannot be the cause; your plan even documents the duplicate at :968. Being fixed separately.

Merge readiness: ✅ Approve — every committed item is in the documents, both follow-up issues exist and are linked, and the number is free. Merge resolves the README conflict by keeping both the 040 and 041 rows.