Skip to content

fix(octopus): review follow-ups for #4849, #4850 and #4852 - #4873

Open
mgazza wants to merge 3 commits into
mainfrom
fix/octopus-power-up-review-follow-ups
Open

fix(octopus): review follow-ups for #4849, #4850 and #4852#4873
mgazza wants to merge 3 commits into
mainfrom
fix/octopus-power-up-review-follow-ups

Conversation

@mgazza

@mgazza mgazza commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator

Follow-ups to #4849, #4850 and #4852. All three merged quickly — thank you — but these landed on the branches just after the merges, so they need a fresh PR.

1. fix(octopus): stop offering Weekend Happy Hours as available to join@gcoan's request on #4851:

The BottlecapDave integration has (in v19.0.1) decided to NOT pass through these zero valued events in the power down sensor since they cannot be joined by API. […] suggest we take the same approach of excluding them from available events.

Now that eventType is being requested (#4852) these are identifiable precisely, so Octopus Direct can match BCD. Listing them only produces the join attempts the API rejects (#4593/#4595) and fills the join selector with options that cannot be selected.

Ordering trap worth a reviewer's eye, because it is invisible in the diff. The reward/code/type lookup maps are built from the same events list, and a joined Happy Hour reads its type out of those maps. Skipping before they are populated leaves the joined event with event_type: None and the injected default reward — so a free hour is priced as an 80p/kWh saving session, which is worse than the bug #4852 fixed. The skip therefore sits after the maps are filled, and test_octopus_saving_event_type fails if it is moved above them.

2. style(kraken): use the Kraken: log prefix — Copilot on #4850. The new backoff and token-request warnings read Warn: Kraken token … while the rest of the component uses Warn: Kraken: …; the odd one out breaks grep and log parsing on the usual prefix.

3. docs(octopus): test list numbering and docstring phrasing — Copilot on #4849. The test docstring skipped Test 7 → Test 9 despite Test 8 existing, and "block_count'th" read awkwardly. Note the docstring moved to utils.py in #4850, so the fix is applied there.

Copilot's third point on #4849 was that the PR description undercounted the tests — that was a description issue on a now-merged PR, so there is nothing to change in the code.

Tests

New test_octopus_saving_event_type, registered in unit_test.py:

  • a Happy Hour the account was allocated is withheld from available_events but still reaches joined_events carrying event_type: WEEKEND_HAPPY_HOUR
  • a TURN_DOWN alongside it is still offered, with its type
  • an event with no eventType is unaffected

unit_test.py -k octopus (72), -k saving and -k kraken all pass.

mgazza and others added 3 commits August 30, 2026 21:30
Review nits: the test docstring skipped from Test 7 to Test 9 despite Test 8
existing, and "block_count'th" read awkwardly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NnzhfR8uDgGUtYLNpDA4ka
Review nit: the new backoff/token-request logs read "Warn: Kraken token ..."
while the rest of the component uses "Warn: Kraken: ...". The odd one out
breaks grep and log-parsing on the usual prefix.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NnzhfR8uDgGUtYLNpDA4ka
Review follow-up (@gcoan). A Weekend Happy Hour cannot be joined through the
API - Octopus either allocates one or the user books it on the website - so
listing it in available_events only produces join attempts the API rejects
(#4593/#4595) and fills the join selector with options that cannot be selected.
BottleCapDave's integration stopped passing these through in v19.0.1 for the
same reason; match that now that eventType makes them identifiable.

Ordering matters and is easy to get wrong: the reward/code/type maps are built
from this same events list, and a joined Happy Hour looks its event type up
there. Skipping before those maps are filled leaves the joined event with no
type and the injected default reward, so the planner prices a free hour as an
80p/kWh saving session. The skip therefore sits after the maps are populated,
and the test fails if it is moved above them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NnzhfR8uDgGUtYLNpDA4ka
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.

1 participant