Skip to content

fix(octopus): prefer the direct debit rate when both payment methods overlap - #5145

Merged
springfall2008 merged 2 commits into
springfall2008:mainfrom
Navesz:fix-octopus-payment-method
Sep 19, 2026
Merged

springfall2008 merged 2 commits into
springfall2008:mainfrom
Navesz:fix-octopus-payment-method

Conversation

@Navesz

@Navesz Navesz commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Closes #5144

The bug

The REST tariff endpoints return two overlapping rows per validity window, one DIRECT_DEBIT and one NON_DIRECT_DEBIT. Both go straight into minute_data(), which writes each row over its range, so whichever row comes last in the response wins.

Reproduced against the live public API (E-1R-VAR-22-11-01-A, no auth), using the repository's own minute_data:

valor que o predbat usa agora : 27.8488
   API DIRECT_DEBIT      = 26.381355
   API NON_DIRECT_DEBIT  = 27.848835
janelas: 17 · com DIRECT_DEBIT por último: 5

So a direct debit customer is shown the higher rate — and the last line is the part that is easy to miss: 5 of the 17 windows the API returns put DIRECT_DEBIT last, so the displayed rate silently flips variant from one period to the next rather than always being the higher one.

payment_method appeared nowhere in the production code before this change — only in test fixtures.

The fix

A module-level filter_payment_method() in octopus.py, applied at both parse points, as suggested in the triage on the issue:

  • get_octopus_rates_direct() — the OctopusAPI component path, for unit rates and standing charges;
  • download_octopus_rates_func() — the legacy import_octopus_url / compare path.

It is deliberately conservative:

  • rows with payment_method: None (Agile, day/night) pass through untouched;
  • rows without the key pass through untouched;
  • a response that never mentions DIRECT_DEBIT is returned unchanged, so a non-direct-debit-only feed keeps its rate rather than becoming empty.

Filtering at the parse point rather than at download also covers data already sitting in the URL and storage caches.

Tests

A seventh sub-test in tests/test_octopus_url.py (payment_method), covering the five cases:

Case Asserts
unit rates, non-direct-debit row last 26.3814 (the order the live API returns today)
unit rates, direct debit row last 26.3814 (the order returned for older periods)
standing charges, same overlap 50.6564
payment_method: None 16.5, untouched
only NON_DIRECT_DEBIT rows 27.8488, untouched

Run with the fix: RESULTS: 7 passed, 0 failed out of 7 tests.

Run with octopus.py reverted to main, to show the test would have caught this:

ERROR: Expected 26.3814 at minute 0, got 27.8488
ERROR: Expected 50.6564 at minute 0, got 59.3136
RESULTS: 6 passed, 1 failed out of 7 tests

Note that the "direct debit row last" case passes even without the fix — that is the order-dependence itself.

Other checks

  • ./run_all --quick: the only failure is predheat ("Predheat did not run promptly after being re-enabled"), which fails the same way on a clean checkout of main on this machine (Windows), so it is unrelated to this change.
  • black --check on both files: unchanged.
  • flake8: same number of findings as main on octopus.py (all pre-existing).
  • interrogate: 90.4% on main → 90.6% with this change, for these two files.

🤖 Generated with Claude Code

…overlap

The REST tariff endpoints return a DIRECT_DEBIT row and a NON_DIRECT_DEBIT row
over the same validity window. Both went unfiltered into minute_data(), which
writes each row over its range, so the row appearing last in the response won.

Measured against the live public API for E-1R-VAR-22-11-01-A: predbat used
27.8488 where the direct debit rate is 26.381355, and 5 of the 17 windows
returned put DIRECT_DEBIT last, so the displayed variant was not even stable
between periods.

filter_payment_method() now runs at both parse points, the OctopusAPI component
path and the legacy import_octopus_url path. Rows with payment_method None
(Agile, day/night), rows without the key, and responses that never mention
DIRECT_DEBIT are all left untouched.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The change is small, localized to the intended parse points, and includes targeted regression coverage for the reported order-dependence bug and its edge cases.

Review effort: Lite
Findings: None

What changed in this PR

This PR fixes an Octopus REST tariff parsing bug where overlapping DIRECT_DEBIT and NON_DIRECT_DEBIT rows for the same validity window could overwrite each other in minute_data() depending on API response order, causing direct-debit customers to see the higher non-direct-debit rate. It adds a conservative parse-time filter that prefers DIRECT_DEBIT when present, while preserving existing behaviour for tariffs that don’t provide (or don’t include) that payment-method variant.

Changes:

  • Add filter_payment_method() in octopus.py and apply it at both Octopus REST parse points (get_octopus_rates_direct() and download_octopus_rates_func()).
  • Add a new payment_method sub-test to verify correct selection for unit rates and standing charges across multiple overlap/edge cases.
File Description
apps/​predbat/​octopus.py Introduces filter_payment_method() and applies it before minute_data() to make payment-method overlaps deterministic (prefer direct debit).
apps/​predbat/​tests/​test_octopus_url.py Adds a new sub-test covering overlapping payment-method rows and edge cases (None / single-variant).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…nual paths

The filter added at the two parse points does not reach two other consumers of
the same overlapping DIRECT_DEBIT / NON_DIRECT_DEBIT rows:

- async_get_day_night_rates builds its own schedule rows, without a
  payment_method, before get_octopus_rates_direct ever runs, so a Flexible
  dual-register tariff (E-2R-*, which has no standard-unit-rates endpoint and
  so always takes this path) still used whichever variant _get_rate_for_time
  saw first. Against the live E-2R-VAR-22-11-01-A feed, 4 of its 17 windows
  return NON_DIRECT_DEBIT first.
- annual_tariff._download_rows feeds the same raw rows to minute_data and to
  _rows_to_local_pattern. Live January 2025 rates for E-1R-VAR-22-11-01-A
  resolved to 26.3156p (non-direct-debit) rather than 25.5749p.

filter_payment_method moves to utils.py, next to the minute_data whose
last-row-wins behaviour causes this, so annual_tariff does not have to import
the Octopus component module - components.py loads those lazily.

The annual rows are filtered on the way out of _download_rows rather than
before the cache write, so the cache keeps Octopus's own rows and a year of
cached history would not need re-downloading if the preferred payment method
ever became configurable; the cached branch is filtered on read.

Tests cover both response orders on each path, because the three consumers
resolve an overlap in opposite directions: minute_data keeps the last row of
an overlap, while _get_rate_for_time and _rows_to_local_pattern keep the first.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@springfall2008
springfall2008 merged commit b4edced into springfall2008:main Sep 19, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flexible Octopus tariff parsing selects non-direct-debit rates

3 participants