fix: read price labels instead of casting them, and require currency (#21)

[karlwaldman]

Closes #21.

The defect

src/Price.php cast six fields with an unchecked (string). Reproduced on
origin/main @ 21339d43e:

PHP Warning:  Array to string conversion in src/Price.php on line 96
PHP Warning:  Array to string conversion in src/Price.php on line 101

currency: ["EUR"]      -> string(5) "Array"
unit:     true         -> string(1) "1"
name:     978          -> string(3) "978"
source:   ['a'=>'b']   -> string(5) "Array"
type:     1.5          -> string(3) "1.5"

A mislabelled unit — barrel where the payload said tonne — is the same harm
class as a wrong number, and harder to spot, because the number beside it is
right. PHPStan level 9 flags all six casts.

The fix

Present-but-non-string values raise ApiException naming the field and its
actual type (get_debug_type). Absent and explicitly null still mean "not
provided" and still give null. No PHP warning is emitted on any path — there
is a test asserting that, because a warning from library internals is either an
exception under a strict handler or a log line nobody reads.

currency is now required

Price::fromArray(['code' => 'EU_CARBON_EUR', 'price' => 78.40])
// before: currency 'USD'
// now:    ApiException

The ?? 'USD' default labelled a euro-denominated carbon price as dollars. A
$0.00 Brent quote is obviously broken and a human catches it; 78.40 USD on
an EUA contract is plausible, roughly 8% wrong, and flows into a model
undetected. The repo's own fixtures carry EU_CARBON_EUR, so the catalogue is
not USD-only. #18 already broke BC on this method, so the strictness costs
nothing extra.

The ISO numeric spelling 978 is rejected rather than accepted as '978' —
it is a currency identifier, but not the one $price->currency is documented
to hold, and silently accepting it would put a number where callers compare a
code.

One value the DTO adjusts: surrounding whitespace on currency is trimmed, so
$price->currency === 'EUR' behaves for " EUR\n". Nothing about the label
changes; it is the alternative to shipping a value that fails every comparison
a caller writes.

TDD evidence

Test written first, against unmodified origin/main source.

RED (./vendor/bin/phpunit --filter PriceFieldCoercionTest, source at origin/main):

FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF......FFF              52 / 52 (100%)

There were 46 failures:

1) PriceFieldCoercionTest::testNonStringFieldIsRejectedRatherThanCoerced@currency as array with data ('currency', ['EUR'])
Field currency was coerced to 'Array' instead of being rejected.

3) ...@currency as true with data ('currency', true)
Field currency was coerced to '1' instead of being rejected.

5) ...@currency as int with data ('currency', 978)
Field currency was coerced to '978' instead of being rejected.

11) ...@unit as int with data ('unit', 978)
Field unit was coerced to '978' instead of being rejected.

37) PriceFieldCoercionTest::testArrayCurrencyDoesNotBecomeTheLiteralStringArray
currency ["EUR"] became 'Array'

38) PriceFieldCoercionTest::testCurrencyIsRequiredRatherThanDefaultedToUsd@absent with data (['EU_CARBON_EUR', 78.4])
A row with no usable currency was labelled 'USD'.

42) ...@iso numeric code with data (['EU_CARBON_EUR', 78.4, 978])
A row with no usable currency was labelled '978'.

43) PriceFieldCoercionTest::testEuroCarbonPriceIsNeverLabelledUsd
EU_CARBON_EUR at 78.4 came back as 'USD'.

46) PriceFieldCoercionTest::testNoArrayToStringWarningIsEmitted
PHP warnings were raised: Array to string conversion

FAILURES!
Tests: 52, Assertions: 62, Failures: 46, Warnings: 6.

(All 36 field × type combinations are covered; the list above is trimmed.)

Re-proved red-capable after the fix by git checkout origin/main -- src/Price.php:
Tests: 52, Assertions: 62, Failures: 46, Warnings: 6. — then restored.

GREEN (this branch):

....................................................              52 / 52 (100%)
OK (52 tests, 96 assertions)

Full suite, PHP 8.5.8 / PHPUnit 13.3.3:

• baseline on clean main @ 21339d43e: OK (108 tests, 478 assertions)
• this branch: OK (160 tests, 574 assertions)

14 existing tests failed on the required-currency change because their fixture
rows omitted it. Those rows gained 'currency' => 'USD'; no assertion was
weakened or removed — the diff on those files is additive only
(RetryPolicyTest, RawPathOriginTest, ClientTest, MalformedPriceRowTest,
tests/fixtures/capture.php).

Compatibility

Price::fromArray() throws for two input shapes it previously accepted: a row
with no currency, and a row with a non-string label field. The version bump
and the written-out BC note are handled separately in the versioning PR rather
than bundled here. README now states the required fields.

🤖 Generated with Claude Code

https://claude.ai/code/session_015ao5paex73xXvuM424Libo

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: acaa7336-9d5f-44bb-a6e8-2ae2005cb145

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[karlwaldman]

Rebased onto 056669c — both #25 and #26 are in, and both survive

Resolved as a merge rather than a rebase (beff13c). I first did the rebase, verified it fully, then found the push path needs --force-with-lease, which a local guard blocks. The merge tree is byte-identical to the verified rebase — git diff <rebase-result> -- src/Price.php CHANGELOG.md README.md tests/ composer.json src/Http/ is empty — so nothing about the resolution changed, only how it reaches the branch. MERGEABLE / CLEAN now.

What conflicted, and how

Both conflicts were co-located additions, not competing edits:

• src/Price.php — fix: reject fabricated observation timestamps (#20) #25 inserted the private parseTimestamp() helper at the same offset this branch inserted optionalString(). Both methods are kept. fromArray() calls both: the currency guard runs, then the timestamp is parsed, then the label fields are read.
• CHANGELOG.md — fix: reject fabricated observation timestamps (#20) #25's timestamp bullet and this branch's label bullet are separate fixes. Both stay under ### Fixed.

Nothing from #26 conflicted.

Both behaviours proven coexisting

#25 fabricated timestamp   -> REJECTED: ... unparseable timestamp ('2026-13-45T99:99:99Z')
#25 relative timestamp     -> REJECTED: ... unparseable timestamp ('next friday')
#25 leap second            -> REJECTED: ... unparseable timestamp ('2026-06-30T23:59:60Z')
#27 array currency         -> REJECTED: ... missing a usable currency (got array)
#27 missing currency       -> REJECTED: ... missing a usable currency (got null)
#27 array unit             -> REJECTED: ... non-string unit (array)
BOTH together              -> REJECTED: ... missing a usable currency (got int)
GOOD row (must pass)       -> OK currency='EUR' unit='tonne' at=2026-07-19T12:00:00+00:00

Red re-proved against the NEW main, and the failure set is identical

git checkout origin/main -- src/Price.php, then --filter PriceFieldCoercionTest:

Tests: 52, Assertions: 62, Failures: 46, Warnings: 6.

I did not take the matching count on trust. I diffed the failure names from old main (21339d4) against new main (056669c): identical, all 46. The only textual delta in the whole red output is the line numbers on the six Array to string conversion warnings — 96,99,100,101,102,103 became 112,115,116,117,118,119, because #25 added the TIMESTAMP_FORMATS constant and shifted the file down 16 lines. Nothing about which defects reproduce changed.

Borrowed-red check

With src/Price.php reverted to origin/main and every other file on this branch left in place, only this PR's own suite goes red:

Suite	Result with src/Price.php reverted
FabricatedTimestampTest (#25)	OK (41 tests, 56 assertions)
RedirectOriginTest (#26)	OK (11 tests, 59 assertions)
MalformedPriceRowTest	OK (41 tests, 46 assertions)
RetryPolicyTest	OK (17 tests, 52 assertions)
RawPathOriginTest	OK (24 tests, 57 assertions)
ClientTest	OK (18 tests, 85 assertions)
PublicClaimsTest	OK (8 tests, 238 assertions)
PriceFieldCoercionTest	Failures: 46

The red is this defect's own. It also shows the fixture edits on this branch (adding 'currency' => 'USD' to rows that omitted it) weakened nothing: those tests pass against the old lenient Price.php too.

Baseline

	Tests	Assertions
clean main @ 056669c, pristine clone	160	593
this branch	212	689

(The earlier 108/478 figure was main @ 21339d4; #25 and #26 moved it.)

Green: OK (52 tests, 96 assertions) for PriceFieldCoercionTest, OK (212 tests, 689 assertions) for the suite.

The two judgement calls flagged for Karl are untouched by the rebase: leap-second rejection, and naive timestamps read as UTC. Both live in #25's parseTimestamp(), which this merge did not modify.

🤖 Generated with Claude Code

https://claude.ai/code/session_015ao5paex73xXvuM424Libo