[Venius24]

Fixes #5573

The Kraken REST path (async_fetch_standing_charges) took the first row after the payment-method filter, and the GraphQL fallback took applicable_charges[0] from a 3-day window, neither checking that the row covers the current time. A future-dated or expired row listed first was used instead of the current charge.

Both paths now go through a small helper, KrakenAPI._standing_charge_covering(): rows whose valid_from is in the future or whose valid_to has passed are skipped, null bounds are open, and the latest valid_from wins among the covering rows. If no row covers now, a warning is logged and None is returned (the caller already keeps its previous value on None).

Tests / checks run (from coverage/):

• unit_test.py --test kraken: all pass, including 4 new tests covering a future-dated row listed first and last (REST), an expired row listed first, no covering row, and the GraphQL window with a price change in both orders.
• With the kraken.py change stashed, the new tests fail.
• black --check on the two touched files: clean.

Not run: the rest of ./run_all and pre-commit; no live Kraken account.

🤖 Generated with Claude Code

[springfall2008]

Automated comment from the triage bot.

Reviewed at 59fdc58: 2 findings, both low — 1 inline, 1 below. The fix itself is correct against issue #5573: it covers both the REST results[0] and the GraphQL applicable_charges[0] pick, the half-open boundary (valid_from > now skips ahead of the change, valid_to <= now marks expired) gives continuous coverage at a period change with no gap or double-pick at the boundary minute, and str2time yields timezone-aware datetimes for both the REST ...Z strings and GraphQL +00:00 offsets so the comparisons are aware-vs-aware. I verified the caller claim from the PR body at apps/predbat/kraken.py:1539: run() assigns self.import_standing_charge only when the return is not None, so a covering-row miss keeps the previous value rather than blanking the sensor, and _vat_multiplier() (kraken.py:421) is unaffected — it reads the standing charge off the already validity-checked agreement, not an unfiltered row list. The 4 new tests are picked up by run_kraken_tests()'s discovery, so no TEST_REGISTRY change is needed, and no user-facing surface (setting, entity or docs) changes.

• apps/predbat/utils.py:1378 — filter_payment_method's docstring still ends "The Kraken component (EDF, E.ON) also relies on this to pick its standing charge from the first remaining row, because EDF lists the NON_DIRECT_DEBIT row first (GH#5502)". That first-remaining-row mechanism is exactly what this PR replaces: the pick now goes through _standing_charge_covering, so the sentence misdescribes kraken.py from this commit on. The filter itself is still essential — two same-window rows of the same period both cover now and the helper keeps the first of equal valid_froms, so without it EDF's NON_DIRECT-first ordering would leak back in — it is only the "first remaining row" description that is now stale, e.g. "…relies on this so that one row per validity window remains when it picks the row covering now".

[springfall2008]

Automated comment from the triage bot.

Low: this tie-break — "the latest start wins among the rows that do cover now" (docstring, lines 1103-1104) — is not exercised by any test. Every new test leaves exactly one covering row: the future/expired/none-covers tests have one covering row after the skips, and test_fetch_standing_charges_graphql_picks_row_covering_now flips the order of a future row and a covering row but never ends up with two covering rows. So a regression in the valid_from > best_from comparison, or in the best_from is None arm (undated listed before dated, lines 1118-1120), would pass CI — e.g. an accidentally inverted comparison would silently keep the first covering row instead of the latest, which is the price-change scenario this helper exists for. A row list with two covering rows (say an older row that overlaps the newer window, or an undated row listed before a dated one) asserted in both orders would pin the branch. Not blocking — the skip logic the tests do cover is the substance of the fix.

[springfall2008]

Automated comment from the triage bot.

Reviewed head 59fdc58 (1 inline, 1 body finding). The PR is sound and fixes GH#5573 as described: both the REST and GraphQL standing-charge paths now select a row whose validity covers now instead of trusting row order, and the semantics match the Octopus precedent the issue cites (_get_rate_for_time, octopus.py:1851-1883 — half-open [valid_from, valid_to), null bounds open, latest valid_from wins). I verified str2time returns timezone-aware datetimes for these strings so the comparisons against datetime.now(timezone.utc) are safe in both the fromisoformat fast path and the strptime fallback, that the GraphQL query already requests validFrom/validTo (kraken.py:153-154), and that the caller keeps its previous value on None (kraken.py:1539-1542) so the new warn-and-None paths degrade to exactly the failure behaviour described in the PR body. The four new tests would fail without the kraken.py change (the old results[0]/[0] selection returns the future-dated row's value in the listed-first orderings the tests include), tests stay in the already-registered kraken module, and no docs/config changes are needed for this behaviour fix. No main-side commits have touched kraken.py or test_kraken.py since the PR branched (merge base d17f0e0), so nothing is superseded.

The two findings below are both low severity; nothing blocks merge.

apps/predbat/utils.py:1378-1379

The filter_payment_method() docstring still says the Kraken component "relies on this to pick its standing charge from the first remaining row" — that selection rule is exactly what this PR replaces. The filter itself is still load-bearing for the standing charge (it removes the NON_DIRECT_DEBIT rows before the covering-row pick), but a reader debugging a GH#5502-style issue on utils.py would be pointed at a results[0] selection that no longer exists and could "restore" the wrong behaviour. Worth a one-line docstring update while this is fresh (the PR does not touch utils.py, so the line sits outside this diff).

[springfall2008]

Automated comment from the triage bot.

A row that is not a dict raises AttributeError here rather than being ignored: row.get(from_key) on None/a non-dict row is an AttributeError, which the surrounding except (ValueError, TypeError) does not catch, nor the per-row continue intends. The neighbouring filter that feeds this helper explicitly tolerates non-dict rows — filter_payment_method() keeps rows where not isinstance(rate, dict) (utils.py:1386) — so on main a junk row only crashed if it happened to be results[0], while with this helper one junk row anywhere in the list aborts the standing-charge call into the component catch-all (kraken.py run()), skipping the rest of that cycle every retry.

I could not confirm Kraken actually emits non-object rows in results/edges (no live account; _connection_nodes also only filters falsy nodes, not non-dict ones), so this is robustness parity with the defensive pattern already used for the same endpoints, not a reproduced bug. If you want the parity, one isinstance(row, dict) guard (or a AttributeError catch, though the isinstance check is the in-repo precedent) before the parse makes the helper as tolerant as the filter feeding it.